[PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

The macro __is_constexpr() causes sparse to report the following:

    warning: expression using sizeof(void)

Avoid this by using __builtin_constant_p() instead.

Fixes: 3c8ba0d61d04 ("kernel.h: Retain constant expression output for max()/min()")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: <stable@vger.kernel.org>
---
 include/linux/kernel.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d23123238534..a9f0d0d48971 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -811,13 +811,19 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #define __typecheck(x, y) \
 		(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
 
+#ifndef __CHECKER__
 /*
  * This returns a constant expression while determining if an argument is
  * a constant expression, most importantly without evaluating the argument.
- * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
+ * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>. However, this
+ * macro causes sparse to report the warning "expression using sizeof(void)".
+ * Hence use __builtin_constant_p() instead when using sparse.
  */
 #define __is_constexpr(x) \
 	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+#else
+#define __is_constexpr(x) __builtin_constant_p((x))
+#endif
 
 #define __no_side_effects(x, y) \
 		(__is_constexpr(x) && __is_constexpr(y))
-- 
2.18.0

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Kees Cook]

On Thu, Jul 5, 2018 at 9:17 AM, Bart Van Assche <bart.vanassche@wdc.com> wrote:
> The macro __is_constexpr() causes sparse to report the following:
>
>     warning: expression using sizeof(void)
>
> Avoid this by using __builtin_constant_p() instead.
>
> Fixes: 3c8ba0d61d04 ("kernel.h: Retain constant expression output for max()/min()")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Cc: <stable@vger.kernel.org>
> ---
>  include/linux/kernel.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d23123238534..a9f0d0d48971 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -811,13 +811,19 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>  #define __typecheck(x, y) \
>                 (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>
> +#ifndef __CHECKER__
>  /*
>   * This returns a constant expression while determining if an argument is
>   * a constant expression, most importantly without evaluating the argument.
> - * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
> + * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>. However, this
> + * macro causes sparse to report the warning "expression using sizeof(void)".
> + * Hence use __builtin_constant_p() instead when using sparse.
>   */
>  #define __is_constexpr(x) \
>         (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
> +#else
> +#define __is_constexpr(x) __builtin_constant_p((x))
> +#endif
>
>  #define __no_side_effects(x, y) \
>                 (__is_constexpr(x) && __is_constexpr(y))

I'm fine with this; it'll only activate for sparse. I'd like to get
Linus's eyes on it, though, since this macro caused us SO much pain
that I'm nervous to change it without some greater level of review. :)

Acked-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Linus Torvalds]

On Sat, Jul 14, 2018 at 6:49 PM Kees Cook <keescook@chromium.org> wrote:
>
> I'm fine with this; it'll only activate for sparse. I'd like to get
> Linus's eyes on it, though, since this macro caused us SO much pain
> that I'm nervous to change it without some greater level of review. :)

Honestly, I'd like to just encourage people to get the sparse update
from Luc Van Oostenryck instead.

For a while there it looked like Chris Li would just pull from Luc,
and we'd have timely releases, but that really doesn't seem to have
ended up happening after all. So right now it's probably just best to
get Luc's tree instead from

    https://github.com/lucvoo/sparse-dev

which also ends up fixing a lot of other issues.

                Linus

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Linus Torvalds]

Oh,
 and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.

            Linus

On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Honestly, I'd like to just encourage people to get the sparse update
> from Luc Van Oostenryck instead.
>
> For a while there it looked like Chris Li would just pull from Luc,
> and we'd have timely releases, but that really doesn't seem to have
> ended up happening after all. So right now it's probably just best to
> get Luc's tree instead from
>
>     https://github.com/lucvoo/sparse-dev
>
> which also ends up fixing a lot of other issues.
>
>                 Linus

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

On Sat, 2018-07-14 at 18:59 -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > 
> > Honestly, I'd like to just encourage people to get the sparse update
> > from Luc Van Oostenryck instead.
> > 
> > For a while there it looked like Chris Li would just pull from Luc,
> > and we'd have timely releases, but that really doesn't seem to have
> > ended up happening after all. So right now it's probably just best to
> > get Luc's tree instead from
> > 
> >     https://github.com/lucvoo/sparse-dev
> > 
> > which also ends up fixing a lot of other issues.
> > 
> >                 Linus
>
> Oh,
>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.
> 
>             Linus

Hello everyone,

I will switch to Luc's sparse tree.

Thank you for the feedback.

Bart.

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Bart Van Assche]

On Sat, 2018-07-14 at 18:59 -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:57 PM Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > Honestly, I'd like to just encourage people to get the sparse update
> > from Luc Van Oostenryck instead.
> > 
> > For a while there it looked like Chris Li would just pull from Luc,
> > and we'd have timely releases, but that really doesn't seem to have
> > ended up happening after all. So right now it's probably just best to
> > get Luc's tree instead from
> > 
> >     https://github.com/lucvoo/sparse-dev
> > 
> > which also ends up fixing a lot of other issues.
> 
> Oh,
>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.

(+ Konstantin)

Something that is confusing for kernel developers is that the sparse wiki
(https://sparse.wiki.kernel.org/index.php/Main_Page) refers kernel developers
to an outdated sparse tree. Should something be done about this inconsistency?
Should the sparse wiki point at one of Luc's sparse trees or should perhaps
Luc be granted access to the tree that wiki points at
(https://git.kernel.org/pub/scm/devel/sparse/sparse.git)?

Thanks,

Bart.

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[konstantin]

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

On Sun, Jul 15, 2018 at 04:07:39AM +0000, Bart Van Assche wrote:
>> Oh,
>>  and I meant to cc Luc on that email, but then forgot. So here he is cc'd now.
>
>(+ Konstantin)
>
>Something that is confusing for kernel developers is that the sparse wiki
>(https://sparse.wiki.kernel.org/index.php/Main_Page) refers kernel developers
>to an outdated sparse tree. Should something be done about this inconsistency?
>Should the sparse wiki point at one of Luc's sparse trees or should perhaps
>Luc be granted access to the tree that wiki points at
>(https://git.kernel.org/pub/scm/devel/sparse/sparse.git)?

I believe Luc is working to get the necessary signatures on his PGP key 
so we can grant him access to the repo at kernel.org. I believe this 
will be less confusing than modifying the wiki to point to the github 
fork of the tree.

Best,
Konstantin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)

[Luc Van Oostenryck]

On Sat, Jul 14, 2018 at 06:57:57PM -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:49 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > I'm fine with this; it'll only activate for sparse. I'd like to get
> > Linus's eyes on it, though, since this macro caused us SO much pain
> > that I'm nervous to change it without some greater level of review. :)
> 
> Honestly, I'd like to just encourage people to get the sparse update
> from Luc Van Oostenryck instead.
> 
> For a while there it looked like Chris Li would just pull from Luc,
> and we'd have timely releases, but that really doesn't seem to have
> ended up happening after all. So right now it's probably just best to
> get Luc's tree instead from
> 
>     https://github.com/lucvoo/sparse-dev
> 
> which also ends up fixing a lot of other issues.

Thank you.

I'll try to move my trees to kernel.org in the coming days or weeks,
it will be better for everyone, I think (I just need one more
signature on my gpg key).

Meanwhile, I prefer that people use my 'stable' tree:
	https://github.com/lucvoo/sparse
(never rebased, only contains the master branch and one backport)
while my sparse-dev repository is mainly a bunch of topic branches
in diverse state of development (but the master branch is the same
so it doesn't matter much).

-- Luc