2018-04-10 01:06:25

by Joey Pabalinas

[permalink] [raw]
Subject: [PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings

Recent changes to the min()/max() macros in include/linux/kernel.h
have added a lot of noise when compiling the kernel with Sparse checking
enabled. This mostly is due to the *huge* increase in the number of
sizeof(void) warnings, a larger number of which can safely be ignored.

Add the -Wpointer-arith flag to enable/disable these warnings (along
with the warning when applying sizeof to function types as well as
warning about pointer arithmetic on these types exactly like the
GCC -Wpointer-arith flag) on demand; the warning itself has been disabled
by default to reduce the large influx of noise which was inadvertently
added by commit 3c8ba0d61d04ced9f8 (kernel.h: Retain constant expression
output for max()/min()).

Update the manpage to document the new flag and add/update validation
cases regarding the application of sizeof to void and function types.

CC: Kees Cook <[email protected]>
CC: Linus Torvalds <[email protected]>
CC: Martin Uecker <[email protected]>
CC: Al Viro <[email protected]>
CC: Christopher Li <[email protected]>
CC: Joey Pabalinas <[email protected]>
CC: Luc Van Oostenryck <[email protected]>
Signed-off-by: Joey Pabalinas <[email protected]>
Signed-off-by: Luc Van Oostenryck <[email protected]>

evaluate.c | 6 ++++--
lib.c | 2 ++
lib.h | 1 +
sparse.1 | 13 ++++++++++++
validation/sizeof-function.c | 2 +-
validation/sizeof-void.c | 39 ++++++++++++++++++++++++++++++++++++
6 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 validation/sizeof-void.c

6 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 4e1dffe9c5416380df..f828da37df8e686623 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2193,7 +2193,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;
}

@@ -2204,7 +2205,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 73d372c36626538bab..f7fdac96674aec4c24 100644
--- a/lib.c
+++ b/lib.c
@@ -242,6 +242,7 @@ int Woverride_init = 1;
int Woverride_init_all = 0;
int Woverride_init_whole_range = 0;
int Wparen_string = 0;
+int Wpointer_arith = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
int Wshadow = 0;
@@ -654,6 +655,7 @@ static const struct flag warnings[] = {
{ "return-void", &Wreturn_void },
{ "shadow", &Wshadow },
{ "sizeof-bool", &Wsizeof_bool },
+ { "pointer-arith", &Wpointer_arith },
{ "sparse-error", &Wsparse_error },
{ "tautological-compare", &Wtautological_compare },
{ "transparent-union", &Wtransparent_union },
diff --git a/lib.h b/lib.h
index 3050b5577ba4d42e97..e34bb9a02ebac03f52 100644
--- a/lib.h
+++ b/lib.h
@@ -151,6 +151,7 @@ extern int Woverride_init;
extern int Woverride_init_all;
extern int Woverride_init_whole_range;
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 88343f3170f195297a..4e1f2385fea7ec9296 100644
--- a/sparse.1
+++ b/sparse.1
@@ -288,6 +288,19 @@ Standard C syntax does not permit a parenthesized string as an array
initializer. GCC allows this syntax as an extension. With
\fB\-Wparen\-string\fR, Sparse will warn about this syntax.

+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
diff --git a/validation/sizeof-function.c b/validation/sizeof-function.c
index 27d535d4ebda79ef0c..8ff67f2149981de7a9 100644
--- a/validation/sizeof-function.c
+++ b/validation/sizeof-function.c
@@ -35,7 +35,7 @@ int test(void)

/*
* check-name: sizeof-function
- * check-command: sparse -Wno-decl $file
+ * check-command: sparse -Wpointer-arith -Wno-decl $file
*
* check-error-start
sizeof-function.c:22:14: warning: expression using sizeof on a function
diff --git a/validation/sizeof-void.c b/validation/sizeof-void.c
new file mode 100644
index 000000000000000000..bc58c98d18d5fa3cda
--- /dev/null
+++ b/validation/sizeof-void.c
@@ -0,0 +1,39 @@
+#define is_constexpr(x) \
+ (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+
+int test(void)
+{
+ unsigned int s = 0, i = 0;
+ void *ptr = &i;
+
+ // OK
+ s += sizeof i;
+ s += sizeof &i;
+ s += sizeof ptr;
+ s += sizeof &ptr;
+
+ // KO
+ s += sizeof(void);
+ s += sizeof *ptr;
+ s += is_constexpr(1 + 1);
+ s += is_constexpr((1, 1));
+ s += is_constexpr(ptr + 1);
+ s += is_constexpr(&ptr + 1);
+ s += is_constexpr(*(((char *)&ptr) + 1));
+
+ return s;
+}
+
+/*
+ * check-name: sizeof-void
+ * check-command: sparse -Wpointer-arith -Wno-decl -Wno-unused-value $file
+ *
+ * check-error-start
+sizeof-void.c:16:14: warning: expression using sizeof(void)
+sizeof-void.c:17:14: warning: expression using sizeof(void)
+sizeof-void.c:19:14: warning: expression using sizeof(void)
+sizeof-void.c:20:14: warning: expression using sizeof(void)
+sizeof-void.c:21:14: warning: expression using sizeof(void)
+sizeof-void.c:22:14: warning: expression using sizeof(void)
+ * check-error-end
+ */
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty


Attachments:
(No filename) (6.24 kB)
signature.asc (849.00 B)
Download all attachments

2018-04-10 19:46:30

by Luc Van Oostenryck

[permalink] [raw]
Subject: Re: [PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings

On Mon, Apr 09, 2018 at 03:02:04PM -1000, Joey Pabalinas wrote:
> Recent changes to the min()/max() macros in include/linux/kernel.h
> have added a lot of noise when compiling the kernel with Sparse checking
> enabled. This mostly is due to the *huge* increase in the number of
> sizeof(void) warnings, a larger number of which can safely be ignored.
>

Thank you very much.

There is just a problem with the test but it's my fault as I
pointed to you to my tree but the master tree lack a lot of fixes
and have problems when dereferencing function pointers.
So, for the master tree, I propose to use a version with these
tests removed:
git://github.com/lucvoo/sparse-dev.git pointer-arith-v3

Cheers,
-- Luc

2018-04-10 22:17:04

by Joey Pabalinas

[permalink] [raw]
Subject: Re: [PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings

On Tue, Apr 10, 2018 at 09:41:19PM +0200, Luc Van Oostenryck wrote:
> Thank you very much.
>
> There is just a problem with the test but it's my fault as I
> pointed to you to my tree but the master tree lack a lot of fixes
> and have problems when dereferencing function pointers.
> So, for the master tree, I propose to use a version with these
> tests removed:
> git://github.com/lucvoo/sparse-dev.git pointer-arith-v3

Not a problem; I will rebase it on the v0.5.2-rc1 tag and
take out the function pointer test-case.

--
Cheers,
Joey Pabalinas


Attachments:
(No filename) (568.00 B)
signature.asc (849.00 B)
Download all attachments

2018-04-10 22:29:05

by Joey Pabalinas

[permalink] [raw]
Subject: Re: [PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings

On Tue, Apr 10, 2018 at 12:09:44PM -1000, Joey Pabalinas wrote:
> Not a problem; I will rebase it on the v0.5.2-rc1 tag and
> take out the function pointer test-case.

Sorry, I meant v0.5.2, heh. Sending the v4 now.

--
Cheers,
Joey Pabalinas


Attachments:
(No filename) (253.00 B)
signature.asc (849.00 B)
Download all attachments