2018-07-05 16:19:33

by Bart Van Assche

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

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 <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Martin Uecker <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: <[email protected]>
---
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 <[email protected]>
+ * Glory to Martin Uecker <[email protected]>. 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



2018-07-15 01:51:20

by Kees Cook

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

On Thu, Jul 5, 2018 at 9:17 AM, Bart Van Assche <[email protected]> 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 <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Martin Uecker <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Miguel Ojeda <[email protected]>
> Cc: <[email protected]>
> ---
> 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 <[email protected]>
> + * Glory to Martin Uecker <[email protected]>. 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 <[email protected]>

Thanks!

-Kees

--
Kees Cook
Pixel Security

2018-07-15 01:59:05

by Linus Torvalds

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

On Sat, Jul 14, 2018 at 6:49 PM Kees Cook <[email protected]> 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

2018-07-15 02:00:18

by Linus Torvalds

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

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
<[email protected]> 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

2018-07-15 03:29:28

by Bart Van Assche

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

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

Hello everyone,

I will switch to Luc's sparse tree.

Thank you for the feedback.

Bart.








2018-07-15 04:09:06

by Bart Van Assche

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

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






2018-07-15 20:24:54

by Luc Van Oostenryck

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

On Sat, Jul 14, 2018 at 06:57:57PM -0700, Linus Torvalds wrote:
> On Sat, Jul 14, 2018 at 6:49 PM Kees Cook <[email protected]> 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

2018-07-16 15:23:28

by Konstantin Ryabitsev

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

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


Attachments:
(No filename) (870.00 B)
signature.asc (235.00 B)
Download all attachments