Smatch and sparse errors

Smatch and sparse errors

[Mauro Carvalho Chehab]

Hi Hans/Jasmin,

There is a regression with sparse and upstream Kernels, with also affect
smatch. Due to that, both will produce hundreds of new errors on all places
that directly or indirectly use min() or max().

Those were caused by those upstream patches:

	3c8ba0d61d04 ("kernel.h: Retain constant expression output for max()/min()")
 	e9092d0d9796 ("Fix subtle macro variable shadowing in min_not_zero()")

There is already an upstream patch for hidding it:
	https://patchwork.kernel.org/patch/10334353/

While upstream doesn't fix it (either by applying this patch or with some
other fixup patch), it should be applied on both sparse and smatch trees, 
in order to get rid of those false positives.

I suggest applying it to the logic with does daily compilations,
in order to avoid generating useless reports for smatch/sparse.

Thanks,
Mauro

Re: Smatch and sparse errors

[Jasmin J.]

[-- Attachment #1: Type: text/plain, Size: 3374 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v4-smatch-add--Wpointer-arith-flag-to-toggle-sizeof-void-warnings.patch --]
[-- Type: text/x-patch; name="v4-smatch-add--Wpointer-arith-flag-to-toggle-sizeof-void-warnings.patch", Size: 2692 bytes --]

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.
 

Re: Smatch and sparse errors

[Mauro Carvalho Chehab]

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

Re: Smatch and sparse errors

[Jasmin J.]

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

Re: Smatch and sparse errors

[Mauro Carvalho Chehab]

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

Re: Smatch and sparse errors

[Hans Verkuil]

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