Smatch and sparse errors

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

Hello Mauro/Hans!

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

index 1533730..815e7e1 100644
@@ -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;
 	}
 

index 69b5ab8..ed4a74f 100644
@@ -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 },

index 0838342..a86615b 100644
@@ -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;

index acdce53..53eff87 100644
@@ -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.
 

Em Sat, 14 Apr 2018 03:18:20 +0200
"Jasmin J." <[email protected]> escreveu:

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

Hello Mauro/Hans!

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

Maybe we should use your version in the build script. Hans?

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

Em Sat, 14 Apr 2018 12:06:34 +0200
"Jasmin J." <[email protected]> escreveu:

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.

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/*

Thanks,
Mauro

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

Let's see what happens tonight.

Regards,

Hans