diff mbox

Smatch and sparse errors

Message ID fc6e68a3-817b-8caf-ba4f-dd2ed76d2a52@anw.at (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Jasmin J. April 14, 2018, 1:18 a.m. UTC 
  Hello Mauro/Hans!

> There is already an upstream patch for hidding it:
The patch from https://patchwork.kernel.org/patch/10334353 will not
apply at the smatch tree.

Attached is an updated version for smatch.

Even with the patched tools, sparse still complains:
 CC      drivers/media/cec/cec-core.o
/opt/media_test/media-git/include/linux/mm.h:533:24: warning: constant 0xffffc90000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:533:48: warning: constant 0xffffc90000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:624:29: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:1098:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:1795:27: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:1887:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:151:25: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:236:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/dma-mapping.h:235:35: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/dma-mapping.h:238:33: warning: constant 0xffffea0000000000 is so big it is unsigned long

 CC      drivers/media/usb/gspca/gl860/gl860-mi2020.o
/opt/media_test/media-git/include/linux/mm.h:533:24: warning: constant 0xffffc90000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:533:48: warning: constant 0xffffc90000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:624:29: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:1098:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:1795:27: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/mm.h:1887:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:151:25: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:236:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
/opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long

But other warnings are gone with the patch.

The daily build is running on a machine of Hans. He need to update the
tools there.

@Hans:
Until this patches are in upstream, we need to patch smatch/sparse on the fly
in build.sh after pulling the last tools version.

BR,
   Jasmin

Comments

Mauro Carvalho Chehab April 14, 2018, 9:46 a.m. UTC | #1 
Em Sat, 14 Apr 2018 03:18:20 +0200
"Jasmin J." <jasmin@anw.at> escreveu:

> Hello Mauro/Hans!
> 
> > There is already an upstream patch for hidding it:  
> The patch from https://patchwork.kernel.org/patch/10334353 will not
> apply at the smatch tree.
> 
> Attached is an updated version for smatch.

Then you're probably not using the right version (or Dan applied some
other stuff yesterday).

Yesterday, I added both trees I'm using here at:

	https://git.linuxtv.org/mchehab/sparse.git/
	https://git.linuxtv.org/mchehab/smatch.git/

My sparse tree has just one extra patch over upstream.
That's needed after a change at max()/min() macros upstream.

At smatch, my tree has 4 extra patches:
	https://git.linuxtv.org/mchehab/smatch.git/

They basically do:
	1) rise the execution time/memory usage of sparse;
	2) mask errors like "missing break", as gcc checks it already;
	3) the same patch for sparse is needed on smatch;
	4) disable this warning:
			drivers/media/platform/sti/bdisp/bdisp-debug.c:594 bdisp_dbg_perf() debug: sval_binop_signed: invalid divide LLONG_MIN/-1
	   with is produced every time do_div64() & friends are called.

IMHO, all 4 patches are disabling false-positive only warnings,
although the 4th patch might have something useful, if fixed to
properly handle the 64-bit compat macros.

Thanks,
Mauro
Jasmin J. April 14, 2018, 10:06 a.m. UTC | #2 
Hello Mauro/Hans!

> Then you're probably not using the right version
Might be ...
The build script from Hans uses the Versions from here:
   git://repo.or.cz/smatch.git
   git://git.kernel.org/pub/scm/devel/sparse/sparse.git

> Yesterday, I added both trees I'm using here at:
> 	https://git.linuxtv.org/mchehab/sparse.git/
> 	https://git.linuxtv.org/mchehab/smatch.git/
Maybe we should use your version in the build script.
Hans?

> IMHO, all 4 patches are disabling false-positive only warnings,
> although the 4th patch might have something useful, if fixed to
> properly handle the 64-bit compat macros.
Another good reason for using your version. Doing so, you can fix/extend
sparse/smatch and the daily build will automatically use that.

BR,
   Jasmin
Mauro Carvalho Chehab April 14, 2018, 10:51 a.m. UTC | #3 
Em Sat, 14 Apr 2018 12:06:34 +0200
"Jasmin J." <jasmin@anw.at> escreveu:

> Hello Mauro/Hans!
> 
> > Then you're probably not using the right version  
> Might be ...
> The build script from Hans uses the Versions from here:
>    git://repo.or.cz/smatch.git

That's right. The last patch on this repo is:

	53b881888d7b (origin/master, origin/HEAD) check_or_vs_and: ignore the kernel's min/max macros

And the patch that adds -Wpointer-arith applies cleanly at the top of
it.

>    git://git.kernel.org/pub/scm/devel/sparse/sparse.git

That's wrong.

Sparse nowadays are getting updates on this dir:

		url = git://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git

I still track the old repo. My config for it is:

[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
[remote "origin"]
	url = git://git.kernel.org/pub/scm/devel/sparse/sparse.git
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
	remote = origin
	merge = refs/heads/master
[remote "sparse-chris"]
	url = git://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git
	fetch = +refs/heads/*:refs/remotes/sparse-chris/*

> 
> > Yesterday, I added both trees I'm using here at:
> > 	https://git.linuxtv.org/mchehab/sparse.git/
> > 	https://git.linuxtv.org/mchehab/smatch.git/  
> Maybe we should use your version in the build script.
> Hans?
> 
> > IMHO, all 4 patches are disabling false-positive only warnings,
> > although the 4th patch might have something useful, if fixed to
> > properly handle the 64-bit compat macros.  
> Another good reason for using your version. Doing so, you can fix/extend
> sparse/smatch and the daily build will automatically use that.
> 
> BR,
>    Jasmin



Thanks,
Mauro
Hans Verkuil April 16, 2018, 12:14 p.m. UTC | #4 
On 04/14/2018 03:18 AM, Jasmin J. wrote:
> Hello Mauro/Hans!
> 
>> There is already an upstream patch for hidding it:
> The patch from https://patchwork.kernel.org/patch/10334353 will not
> apply at the smatch tree.
> 
> Attached is an updated version for smatch.
> 
> Even with the patched tools, sparse still complains:
>  CC      drivers/media/cec/cec-core.o
> /opt/media_test/media-git/include/linux/mm.h:533:24: warning: constant 0xffffc90000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:533:48: warning: constant 0xffffc90000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:624:29: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:1098:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:1795:27: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:1887:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:151:25: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:236:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/dma-mapping.h:235:35: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/dma-mapping.h:238:33: warning: constant 0xffffea0000000000 is so big it is unsigned long
> 
>  CC      drivers/media/usb/gspca/gl860/gl860-mi2020.o
> /opt/media_test/media-git/include/linux/mm.h:533:24: warning: constant 0xffffc90000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:533:48: warning: constant 0xffffc90000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:624:29: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:1098:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:1795:27: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/mm.h:1887:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:151:25: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:236:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> /opt/media_test/media-git/include/linux/scatterlist.h:387:16: warning: constant 0xffffea0000000000 is so big it is unsigned long
> 
> But other warnings are gone with the patch.
> 
> The daily build is running on a machine of Hans. He need to update the
> tools there.
> 
> @Hans:
> Until this patches are in upstream, we need to patch smatch/sparse on the fly
> in build.sh after pulling the last tools version.

I've switched my sparse/smatch to Mauro's repositories for those utilities.

Let's see what happens tonight.

Regards,

	Hans
diff mbox

Patch

diff --git a/evaluate.c b/evaluate.c
index 1533730..815e7e1 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2090,7 +2090,8 @@  static struct symbol *evaluate_sizeof(struct expression *expr)
 	size = type->bit_size;
 
 	if (size < 0 && is_void_type(type)) {
-		warning(expr->pos, "expression using sizeof(void)");
+		if (Wpointer_arith)
+			warning(expr->pos, "expression using sizeof(void)");
 		size = bits_in_char;
 	}
 
@@ -2101,7 +2102,8 @@  static struct symbol *evaluate_sizeof(struct expression *expr)
 	}
 
 	if (is_function(type->ctype.base_type)) {
-		warning(expr->pos, "expression using sizeof on a function");
+		if (Wpointer_arith)
+			warning(expr->pos, "expression using sizeof on a function");
 		size = bits_in_char;
 	}
 
diff --git a/lib.c b/lib.c
index 69b5ab8..ed4a74f 100644
--- a/lib.c
+++ b/lib.c
@@ -234,6 +234,7 @@  int Wnon_pointer_null = 1;
 int Wold_initializer = 1;
 int Wone_bit_signed_bitfield = 1;
 int Wparen_string = 0;
+int Wpointer_arith = 0;
 int Wptr_subtraction_blows = 0;
 int Wreturn_void = 0;
 int Wshadow = 0;
@@ -453,6 +454,7 @@  static const struct warning {
 	{ "return-void", &Wreturn_void },
 	{ "shadow", &Wshadow },
 	{ "sizeof-bool", &Wsizeof_bool },
+	{ "pointer-arith", &Wpointer_arith },
 	{ "transparent-union", &Wtransparent_union },
 	{ "typesign", &Wtypesign },
 	{ "undef", &Wundef },
diff --git a/lib.h b/lib.h
index 0838342..a86615b 100644
--- a/lib.h
+++ b/lib.h
@@ -120,6 +120,7 @@  extern int Wnon_pointer_null;
 extern int Wold_initializer;
 extern int Wone_bit_signed_bitfield;
 extern int Wparen_string;
+extern int Wpointer_arith;
 extern int Wptr_subtraction_blows;
 extern int Wreturn_void;
 extern int Wshadow;
diff --git a/sparse.1 b/sparse.1
index acdce53..53eff87 100644
--- a/sparse.1
+++ b/sparse.1
@@ -265,6 +265,19 @@  initializer.  GCC allows this syntax as an extension.  With
 Sparse does not issue these warnings by default.
 .
 .TP
+.B \-Wpointer\-arith
+Warn about anything that depends on the \fBsizeof\fR a void or function type.
+
+C99 does not allow the \fBsizeof\fR operator to be applied to function types
+or to incomplete types such as void. GCC allows \fBsizeof\fR to be applied to
+these types as an extension and assigns these types a size of \fI1\fR. With
+\fB\-pointer\-arith\fR, Sparse will warn about pointer arithmetic on void
+or function pointers, as well as expressions which directly apply the
+\fBsizeof\fR operator to void or function types.
+
+Sparse does not issue these warnings by default.
+.
+.TP
 .B \-Wptr\-subtraction\-blows
 Warn when subtracting two pointers to a type with a non-power-of-two size.