linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
@ 2018-08-25 18:16 Masahiro Yamada
  2018-08-27 20:05 ` Daniel Santos
  0 siblings, 1 reply; 22+ messages in thread
From: Masahiro Yamada @ 2018-08-25 18:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Kees Cook, Nick Desaulniers, Daniel Santos, Masahiro Yamada,
	Christopher Li, linux-sparse, linux-kernel

__compiletime_assert_fallback() is supposed to stop building earlier
by using the negative-array-size method in case the compiler does not
support "error" attribute, but has never worked like that.

You can simply try:

    BUILD_BUG_ON(1);

GCC immediately terminates the build, but Clang does not report
anything because Clang does not support the "error" attribute now.
It will later fail at link time, but __compiletime_assert_fallback()
is not working at least.

The root cause is commit 1d6a0d19c855 ("bug.h: prevent double evaluation
of `condition' in BUILD_BUG_ON").  Prior to that commit, BUILD_BUG_ON()
was checked by the negative-array-size method *and* the link-time trick.
Since that commit, the negative-array-size is not effective because
'__cond' is no longer constant.  As the comment in <linux/build_bug.h>
says, GCC (and Clang as well) only emits the error for obvious cases.

When '__cond' is a variable,

    ((void)sizeof(char[1 - 2 * __cond]))

... is not obvious for the compiler to know the array size is negative.

Reverting that commit would break BUILD_BUG() because negative-size-array
is evaluated before the code is optimized out.

Let's give up __compiletime_assert_fallback().  This commit does not
change the current behavior since it just rips off the useless code.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

Changes in v2:
  - Rebase

 include/linux/compiler.h | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866..87c776c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -314,29 +314,14 @@ static inline void *offset_to_ptr(const int *off)
 #endif
 #ifndef __compiletime_error
 # define __compiletime_error(message)
-/*
- * Sparse complains of variable sized arrays due to the temporary variable in
- * __compiletime_assert. Unfortunately we can't just expand it out to make
- * sparse see a constant array size without breaking compiletime_assert on old
- * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
- */
-# ifndef __CHECKER__
-#  define __compiletime_error_fallback(condition) \
-	do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
-# endif
-#endif
-#ifndef __compiletime_error_fallback
-# define __compiletime_error_fallback(condition) do { } while (0)
 #endif
 
 #ifdef __OPTIMIZE__
 # define __compiletime_assert(condition, msg, prefix, suffix)		\
 	do {								\
-		int __cond = !(condition);				\
 		extern void prefix ## suffix(void) __compiletime_error(msg); \
-		if (__cond)						\
+		if (!(condition))					\
 			prefix ## suffix();				\
-		__compiletime_error_fallback(__cond);			\
 	} while (0)
 #else
 # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-25 18:16 [PATCH v2] compiler.h: give up __compiletime_assert_fallback() Masahiro Yamada
@ 2018-08-27 20:05 ` Daniel Santos
  2018-08-27 20:09   ` Nick Desaulniers
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Santos @ 2018-08-27 20:05 UTC (permalink / raw)
  To: Masahiro Yamada, Linus Torvalds
  Cc: Kees Cook, Nick Desaulniers, Christopher Li, linux-sparse, linux-kernel

Hello Masahiro,


On 08/25/2018 01:16 PM, Masahiro Yamada wrote:
> __compiletime_assert_fallback() is supposed to stop building earlier
> by using the negative-array-size method in case the compiler does not
> support "error" attribute, but has never worked like that.
>
> You can simply try:
>
>     BUILD_BUG_ON(1);
>
> GCC immediately terminates the build, but Clang does not report
> anything because Clang does not support the "error" attribute now.
> It will later fail at link time, but __compiletime_assert_fallback()
> is not working at least.
>
> The root cause is commit 1d6a0d19c855 ("bug.h: prevent double evaluation
> of `condition' in BUILD_BUG_ON").

I didn't really think this particular patch was necessary, but it was
requested that I eliminate double evaluation and I didn't feel like
arguing it at the time. :)  In my philosophy however, one should *never*
use an expression with side effects in any type of assert.

> Prior to that commit, BUILD_BUG_ON()
> was checked by the negative-array-size method *and* the link-time trick.
> Since that commit, the negative-array-size is not effective because
> '__cond' is no longer constant.

Now we're back to the question of "what do you mean by 'constant'"?  If
you mean a C constant expression (as defined in the C standard) than
almost none of this code fits that criteria.  For these compile-time
assertions to work, we are concerned with the data flow analysis and
constant propagation performed by the compiler during optimization.  You
will notice in include/linux/compiler.h that __compiletime_assert is a
no-op when __OPTIMIZE__ is not defined.

> As the comment in <linux/build_bug.h>
> says, GCC (and Clang as well) only emits the error for obvious cases.
>
> When '__cond' is a variable,
>
>     ((void)sizeof(char[1 - 2 * __cond]))
>
> ... is not obvious for the compiler to know the array size is negative.
>
> Reverting that commit would break BUILD_BUG() because negative-size-array
> is evaluated before the code is optimized out.
>
> Let's give up __compiletime_assert_fallback().  This commit does not
> change the current behavior since it just rips off the useless code.

Clang is not the only target audience of
__compiletime_assert_fallback().  Instead of ripping out something that
may benefit builds with gcc 4.2 and earlier, why not override its
definition in compiler-clang.h with something that will break the build
for Clang?  It would need an #ifndef __compiletime_error_fallback here
though.

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> Changes in v2:
>   - Rebase
>
>  include/linux/compiler.h | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 681d866..87c776c 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -314,29 +314,14 @@ static inline void *offset_to_ptr(const int *off)
>  #endif
>  #ifndef __compiletime_error
>  # define __compiletime_error(message)
> -/*
> - * Sparse complains of variable sized arrays due to the temporary variable in
> - * __compiletime_assert. Unfortunately we can't just expand it out to make
> - * sparse see a constant array size without breaking compiletime_assert on old
> - * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
> - */
> -# ifndef __CHECKER__
> -#  define __compiletime_error_fallback(condition) \
> -	do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
> -# endif
> -#endif
> -#ifndef __compiletime_error_fallback
> -# define __compiletime_error_fallback(condition) do { } while (0)
>  #endif
>  
>  #ifdef __OPTIMIZE__
>  # define __compiletime_assert(condition, msg, prefix, suffix)		\
>  	do {								\
> -		int __cond = !(condition);				\
>  		extern void prefix ## suffix(void) __compiletime_error(msg); \
> -		if (__cond)						\
> +		if (!(condition))					\
>  			prefix ## suffix();				\
> -		__compiletime_error_fallback(__cond);			\
>  	} while (0)
>  #else
>  # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)

To give any more meaningful feedback I think I will need to experiment
with Clang, older GCC versions and icc.  It occurred to me that I should
probably clean up and publish my __builtin_constant_p test program and
also generate results for more recent compilers.  I can extend it to
test various negative sized array constructs and it could help inform
this decision.

IMO, the most ideal solution would be a set of C2x (or future)
extensions providing something similar to C++'s constexpr, GCC's
__builtin_constant_p and our BUILD_BUG_ON.  This would cross deeply into
territory traditionally considered to belong to the implementation, so
it's no small request.  A lot would have to be resolved for it to work
in the standard.

Daniel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-27 20:05 ` Daniel Santos
@ 2018-08-27 20:09   ` Nick Desaulniers
  2018-08-27 20:42     ` Daniel Santos
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Desaulniers @ 2018-08-27 20:09 UTC (permalink / raw)
  To: daniel.santos
  Cc: Masahiro Yamada, Linus Torvalds, Kees Cook, sparse, linux-sparse, LKML

On Mon, Aug 27, 2018 at 1:05 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>
> Hello Masahiro,
>
>
> On 08/25/2018 01:16 PM, Masahiro Yamada wrote:
> > __compiletime_assert_fallback() is supposed to stop building earlier
> > by using the negative-array-size method in case the compiler does not
> > support "error" attribute, but has never worked like that.
> >
> > You can simply try:
> >
> >     BUILD_BUG_ON(1);
> >
> > GCC immediately terminates the build, but Clang does not report
> > anything because Clang does not support the "error" attribute now.
> > It will later fail at link time, but __compiletime_assert_fallback()
> > is not working at least.
> >
> > The root cause is commit 1d6a0d19c855 ("bug.h: prevent double evaluation
> > of `condition' in BUILD_BUG_ON").
>
> I didn't really think this particular patch was necessary, but it was
> requested that I eliminate double evaluation and I didn't feel like
> arguing it at the time. :)  In my philosophy however, one should *never*
> use an expression with side effects in any type of assert.
>
> > Prior to that commit, BUILD_BUG_ON()
> > was checked by the negative-array-size method *and* the link-time trick.
> > Since that commit, the negative-array-size is not effective because
> > '__cond' is no longer constant.
>
> Now we're back to the question of "what do you mean by 'constant'"?  If
> you mean a C constant expression (as defined in the C standard) than
> almost none of this code fits that criteria.  For these compile-time
> assertions to work, we are concerned with the data flow analysis and
> constant propagation performed by the compiler during optimization.  You
> will notice in include/linux/compiler.h that __compiletime_assert is a
> no-op when __OPTIMIZE__ is not defined.

Depending on optimizations for static assertions sounds problematic.

>
> > As the comment in <linux/build_bug.h>
> > says, GCC (and Clang as well) only emits the error for obvious cases.
> >
> > When '__cond' is a variable,
> >
> >     ((void)sizeof(char[1 - 2 * __cond]))
> >
> > ... is not obvious for the compiler to know the array size is negative.
> >
> > Reverting that commit would break BUILD_BUG() because negative-size-array
> > is evaluated before the code is optimized out.
> >
> > Let's give up __compiletime_assert_fallback().  This commit does not
> > change the current behavior since it just rips off the useless code.
>
> Clang is not the only target audience of
> __compiletime_assert_fallback().  Instead of ripping out something that
> may benefit builds with gcc 4.2 and earlier, why not override its

Note that with commit cafa0010cd51 ("Raise the minimum required gcc
version to 4.6") that gcc < 4.6 is irrelevant.

> definition in compiler-clang.h with something that will break the build
> for Clang?  It would need an #ifndef __compiletime_error_fallback here
> though.
>
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >
> > Changes in v2:
> >   - Rebase
> >
> >  include/linux/compiler.h | 17 +----------------
> >  1 file changed, 1 insertion(+), 16 deletions(-)
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index 681d866..87c776c 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -314,29 +314,14 @@ static inline void *offset_to_ptr(const int *off)
> >  #endif
> >  #ifndef __compiletime_error
> >  # define __compiletime_error(message)
> > -/*
> > - * Sparse complains of variable sized arrays due to the temporary variable in
> > - * __compiletime_assert. Unfortunately we can't just expand it out to make
> > - * sparse see a constant array size without breaking compiletime_assert on old
> > - * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
> > - */
> > -# ifndef __CHECKER__
> > -#  define __compiletime_error_fallback(condition) \
> > -     do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
> > -# endif
> > -#endif
> > -#ifndef __compiletime_error_fallback
> > -# define __compiletime_error_fallback(condition) do { } while (0)
> >  #endif
> >
> >  #ifdef __OPTIMIZE__
> >  # define __compiletime_assert(condition, msg, prefix, suffix)                \
> >       do {                                                            \
> > -             int __cond = !(condition);                              \
> >               extern void prefix ## suffix(void) __compiletime_error(msg); \
> > -             if (__cond)                                             \
> > +             if (!(condition))                                       \
> >                       prefix ## suffix();                             \
> > -             __compiletime_error_fallback(__cond);                   \
> >       } while (0)
> >  #else
> >  # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
>
> To give any more meaningful feedback I think I will need to experiment
> with Clang, older GCC versions and icc.  It occurred to me that I should
> probably clean up and publish my __builtin_constant_p test program and
> also generate results for more recent compilers.  I can extend it to
> test various negative sized array constructs and it could help inform
> this decision.
>
> IMO, the most ideal solution would be a set of C2x (or future)
> extensions providing something similar to C++'s constexpr, GCC's
> __builtin_constant_p and our BUILD_BUG_ON.  This would cross deeply into

Note that __builtin_constant_p is a wild beast with lots of edge
cases, and dependencies on compiler and optimization level.  I'm
trying to sort out some of these differences right now with llvm
developers.

> territory traditionally considered to belong to the implementation, so
> it's no small request.  A lot would have to be resolved for it to work
> in the standard.
>
> Daniel



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-27 20:09   ` Nick Desaulniers
@ 2018-08-27 20:42     ` Daniel Santos
  2018-08-27 21:01       ` Nick Desaulniers
                         ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Daniel Santos @ 2018-08-27 20:42 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Masahiro Yamada, Linus Torvalds, Kees Cook, sparse, linux-sparse, LKML

Hello Nick,

On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
>> Now we're back to the question of "what do you mean by 'constant'"?  If
>> you mean a C constant expression (as defined in the C standard) than
>> almost none of this code fits that criteria.  For these compile-time
>> assertions to work, we are concerned with the data flow analysis and
>> constant propagation performed by the compiler during optimization.  You
>> will notice in include/linux/compiler.h that __compiletime_assert is a
>> no-op when __OPTIMIZE__ is not defined.
> Depending on optimizations for static assertions sounds problematic.

(with my best Palpatine voice) It is unavoidable.

Actually it's theoretically possible, but the compiler would have to do
something akin to copying it's control flow graph et. al, run -O2-ish
optimizations, perform the static assertions and then throw away the
optimized control flow graph and emit code based upon the original.


>>> As the comment in <linux/build_bug.h>
>>> says, GCC (and Clang as well) only emits the error for obvious cases.
>>>
>>> When '__cond' is a variable,
>>>
>>>     ((void)sizeof(char[1 - 2 * __cond]))
>>>
>>> ... is not obvious for the compiler to know the array size is negative.
>>>
>>> Reverting that commit would break BUILD_BUG() because negative-size-array
>>> is evaluated before the code is optimized out.
>>>
>>> Let's give up __compiletime_assert_fallback().  This commit does not
>>> change the current behavior since it just rips off the useless code.
>> Clang is not the only target audience of
>> __compiletime_assert_fallback().  Instead of ripping out something that
>> may benefit builds with gcc 4.2 and earlier, why not override its
> Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> version to 4.6") that gcc < 4.6 is irrelevant.

Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
I guess I would be OK with its removal, but I still think it would be
better if a similar mechanism to break the Clang build could be found.

>> definition in compiler-clang.h with something that will break the build
>> for Clang?  It would need an #ifndef __compiletime_error_fallback here
>> though.
>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>>> ---
>>>
>>> Changes in v2:
>>>   - Rebase
>>>
>>>  include/linux/compiler.h | 17 +----------------
>>>  1 file changed, 1 insertion(+), 16 deletions(-)
>>>
>>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>>> index 681d866..87c776c 100644
>>> --- a/include/linux/compiler.h
>>> +++ b/include/linux/compiler.h
>>> @@ -314,29 +314,14 @@ static inline void *offset_to_ptr(const int *off)
>>>  #endif
>>>  #ifndef __compiletime_error
>>>  # define __compiletime_error(message)
>>> -/*
>>> - * Sparse complains of variable sized arrays due to the temporary variable in
>>> - * __compiletime_assert. Unfortunately we can't just expand it out to make
>>> - * sparse see a constant array size without breaking compiletime_assert on old
>>> - * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
>>> - */
>>> -# ifndef __CHECKER__
>>> -#  define __compiletime_error_fallback(condition) \
>>> -     do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
>>> -# endif
>>> -#endif
>>> -#ifndef __compiletime_error_fallback
>>> -# define __compiletime_error_fallback(condition) do { } while (0)
>>>  #endif
>>>
>>>  #ifdef __OPTIMIZE__
>>>  # define __compiletime_assert(condition, msg, prefix, suffix)                \
>>>       do {                                                            \
>>> -             int __cond = !(condition);                              \
>>>               extern void prefix ## suffix(void) __compiletime_error(msg); \
>>> -             if (__cond)                                             \
>>> +             if (!(condition))                                       \
>>>                       prefix ## suffix();                             \
>>> -             __compiletime_error_fallback(__cond);                   \
>>>       } while (0)
>>>  #else
>>>  # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
>> To give any more meaningful feedback I think I will need to experiment
>> with Clang, older GCC versions and icc.  It occurred to me that I should
>> probably clean up and publish my __builtin_constant_p test program and
>> also generate results for more recent compilers.  I can extend it to
>> test various negative sized array constructs and it could help inform
>> this decision.
>>
>> IMO, the most ideal solution would be a set of C2x (or future)
>> extensions providing something similar to C++'s constexpr, GCC's
>> __builtin_constant_p and our BUILD_BUG_ON.  This would cross deeply into
> Note that __builtin_constant_p is a wild beast with lots of edge
> cases, and dependencies on compiler and optimization level.  I'm
> trying to sort out some of these differences right now with llvm
> developers.

Yes it is.  IIRC, I even found cases where __builtin_constant_p returned
false, but the emitted code replaced the value in question with a
constant.  So at least at one point, constant propagation and
__builtin_constant_p weren't always entirely coherent.

I was working on a GCC extension that would solve this problem earlier
this year but bills and paying work interrupted me. :)  The solution
involved adding a "const" attribute for function parameters and
variables that would simply create a warning or error if the value
wasn't compiled away at the time middle-end optimizations completed and
RTL emitted.  It isn't finished -- maybe for gcc10.

Daniel

>
>> territory traditionally considered to belong to the implementation, so
>> it's no small request.  A lot would have to be resolved for it to work
>> in the standard.
>>
>> Daniel



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-27 20:42     ` Daniel Santos
@ 2018-08-27 21:01       ` Nick Desaulniers
  2018-08-28 10:55       ` Arnd Bergmann
  2018-08-28 23:00       ` Nick Desaulniers
  2 siblings, 0 replies; 22+ messages in thread
From: Nick Desaulniers @ 2018-08-27 21:01 UTC (permalink / raw)
  To: daniel.santos
  Cc: Masahiro Yamada, Linus Torvalds, Kees Cook, sparse, linux-sparse, LKML

On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>
> Hello Nick,
>
> On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> >> Now we're back to the question of "what do you mean by 'constant'"?  If
> >> you mean a C constant expression (as defined in the C standard) than
> >> almost none of this code fits that criteria.  For these compile-time
> >> assertions to work, we are concerned with the data flow analysis and
> >> constant propagation performed by the compiler during optimization.  You
> >> will notice in include/linux/compiler.h that __compiletime_assert is a
> >> no-op when __OPTIMIZE__ is not defined.
> > Depending on optimizations for static assertions sounds problematic.
>
> (with my best Palpatine voice) It is unavoidable.

LOL

>
> Actually it's theoretically possible, but the compiler would have to do
> something akin to copying it's control flow graph et. al, run -O2-ish
> optimizations, perform the static assertions and then throw away the
> optimized control flow graph and emit code based upon the original.
>
>
> >>> As the comment in <linux/build_bug.h>
> >>> says, GCC (and Clang as well) only emits the error for obvious cases.
> >>>
> >>> When '__cond' is a variable,
> >>>
> >>>     ((void)sizeof(char[1 - 2 * __cond]))
> >>>
> >>> ... is not obvious for the compiler to know the array size is negative.
> >>>
> >>> Reverting that commit would break BUILD_BUG() because negative-size-array
> >>> is evaluated before the code is optimized out.
> >>>
> >>> Let's give up __compiletime_assert_fallback().  This commit does not
> >>> change the current behavior since it just rips off the useless code.
> >> Clang is not the only target audience of
> >> __compiletime_assert_fallback().  Instead of ripping out something that
> >> may benefit builds with gcc 4.2 and earlier, why not override its
> > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> > version to 4.6") that gcc < 4.6 is irrelevant.
>
> Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
> I guess I would be OK with its removal, but I still think it would be
> better if a similar mechanism to break the Clang build could be found.
>
> >> definition in compiler-clang.h with something that will break the build
> >> for Clang?  It would need an #ifndef __compiletime_error_fallback here
> >> though.
> >>
> >>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>> Reviewed-by: Kees Cook <keescook@chromium.org>
> >>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>>   - Rebase
> >>>
> >>>  include/linux/compiler.h | 17 +----------------
> >>>  1 file changed, 1 insertion(+), 16 deletions(-)
> >>>
> >>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> >>> index 681d866..87c776c 100644
> >>> --- a/include/linux/compiler.h
> >>> +++ b/include/linux/compiler.h
> >>> @@ -314,29 +314,14 @@ static inline void *offset_to_ptr(const int *off)
> >>>  #endif
> >>>  #ifndef __compiletime_error
> >>>  # define __compiletime_error(message)
> >>> -/*
> >>> - * Sparse complains of variable sized arrays due to the temporary variable in
> >>> - * __compiletime_assert. Unfortunately we can't just expand it out to make
> >>> - * sparse see a constant array size without breaking compiletime_assert on old
> >>> - * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
> >>> - */
> >>> -# ifndef __CHECKER__
> >>> -#  define __compiletime_error_fallback(condition) \
> >>> -     do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
> >>> -# endif
> >>> -#endif
> >>> -#ifndef __compiletime_error_fallback
> >>> -# define __compiletime_error_fallback(condition) do { } while (0)
> >>>  #endif
> >>>
> >>>  #ifdef __OPTIMIZE__
> >>>  # define __compiletime_assert(condition, msg, prefix, suffix)                \
> >>>       do {                                                            \
> >>> -             int __cond = !(condition);                              \
> >>>               extern void prefix ## suffix(void) __compiletime_error(msg); \
> >>> -             if (__cond)                                             \
> >>> +             if (!(condition))                                       \
> >>>                       prefix ## suffix();                             \
> >>> -             __compiletime_error_fallback(__cond);                   \
> >>>       } while (0)
> >>>  #else
> >>>  # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
> >> To give any more meaningful feedback I think I will need to experiment
> >> with Clang, older GCC versions and icc.  It occurred to me that I should
> >> probably clean up and publish my __builtin_constant_p test program and
> >> also generate results for more recent compilers.  I can extend it to
> >> test various negative sized array constructs and it could help inform
> >> this decision.
> >>
> >> IMO, the most ideal solution would be a set of C2x (or future)
> >> extensions providing something similar to C++'s constexpr, GCC's
> >> __builtin_constant_p and our BUILD_BUG_ON.  This would cross deeply into
> > Note that __builtin_constant_p is a wild beast with lots of edge
> > cases, and dependencies on compiler and optimization level.  I'm
> > trying to sort out some of these differences right now with llvm
> > developers.
>
> Yes it is.  IIRC, I even found cases where __builtin_constant_p returned
> false, but the emitted code replaced the value in question with a
> constant.  So at least at one point, constant propagation and
> __builtin_constant_p weren't always entirely coherent.

What?! Do you have a link to a repro on godbolt or a gcc bugreport?

Here's a different case I found that looks problematic:
https://godbolt.org/z/g_iqwh

>
> I was working on a GCC extension that would solve this problem earlier
> this year but bills and paying work interrupted me. :)  The solution
> involved adding a "const" attribute for function parameters and
> variables that would simply create a warning or error if the value
> wasn't compiled away at the time middle-end optimizations completed and
> RTL emitted.  It isn't finished -- maybe for gcc10.
>

Speaking with a coworker internally now, discussing Clang's
diagnose_if/enable_if attributes.  It seems that _Static_assert always
(between compilers) considers parameters non-ICE, which sounds like a
defect to me.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-27 20:42     ` Daniel Santos
  2018-08-27 21:01       ` Nick Desaulniers
@ 2018-08-28 10:55       ` Arnd Bergmann
  2018-08-28 13:46         ` Masahiro Yamada
  2018-08-28 23:00       ` Nick Desaulniers
  2 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2018-08-28 10:55 UTC (permalink / raw)
  To: daniel.santos
  Cc: Nick Desaulniers, Masahiro Yamada, Linus Torvalds, Kees Cook,
	Christopher Li, linux-sparse, Linux Kernel Mailing List

On Mon, Aug 27, 2018 at 10:44 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>
> On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> >> Now we're back to the question of "what do you mean by 'constant'"?  If
> >> you mean a C constant expression (as defined in the C standard) than
> >> almost none of this code fits that criteria.  For these compile-time
> >> assertions to work, we are concerned with the data flow analysis and
> >> constant propagation performed by the compiler during optimization.  You
> >> will notice in include/linux/compiler.h that __compiletime_assert is a
> >> no-op when __OPTIMIZE__ is not defined.
> > Depending on optimizations for static assertions sounds problematic.
>
> (with my best Palpatine voice) It is unavoidable.
>
> Actually it's theoretically possible, but the compiler would have to do
> something akin to copying it's control flow graph et. al, run -O2-ish
> optimizations, perform the static assertions and then throw away the
> optimized control flow graph and emit code based upon the original.

In the context of the kernel, compiling with anything less than -O2 or
-Os is not an issue, we don't do it anyway. -O0 never worked, and
AFAICT we only build one file with -O1, but that is something we can do
away with as well:

from fs/reiserfs/Makefile:
# gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
# functions are used.  This causes the compiler to advance the stack
# pointer out of the available stack space, corrupting kernel space,
# and causing a panic. Since this behavior only affects ppc32, this ifeq
# will work around it. If any other architecture displays this behavior,
# add it here.
ccflags-$(CONFIG_PPC32) := $(call cc-ifversion, -lt, 0400, -O1)

     Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-28 10:55       ` Arnd Bergmann
@ 2018-08-28 13:46         ` Masahiro Yamada
  0 siblings, 0 replies; 22+ messages in thread
From: Masahiro Yamada @ 2018-08-28 13:46 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Daniel Santos, Nick Desaulniers, Linus Torvalds, Kees Cook,
	Christopher Li, linux-sparse, Linux Kernel Mailing List

2018-08-28 19:55 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> On Mon, Aug 27, 2018 at 10:44 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>>
>> On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
>> >> Now we're back to the question of "what do you mean by 'constant'"?  If
>> >> you mean a C constant expression (as defined in the C standard) than
>> >> almost none of this code fits that criteria.  For these compile-time
>> >> assertions to work, we are concerned with the data flow analysis and
>> >> constant propagation performed by the compiler during optimization.  You
>> >> will notice in include/linux/compiler.h that __compiletime_assert is a
>> >> no-op when __OPTIMIZE__ is not defined.
>> > Depending on optimizations for static assertions sounds problematic.
>>
>> (with my best Palpatine voice) It is unavoidable.
>>
>> Actually it's theoretically possible, but the compiler would have to do
>> something akin to copying it's control flow graph et. al, run -O2-ish
>> optimizations, perform the static assertions and then throw away the
>> optimized control flow graph and emit code based upon the original.
>
> In the context of the kernel, compiling with anything less than -O2 or
> -Os is not an issue, we don't do it anyway. -O0 never worked, and
> AFAICT we only build one file with -O1, but that is something we can do
> away with as well:
>
> from fs/reiserfs/Makefile:
> # gcc -O2 (the kernel default)  is overaggressive on ppc32 when many inline
> # functions are used.  This causes the compiler to advance the stack
> # pointer out of the available stack space, corrupting kernel space,
> # and causing a panic. Since this behavior only affects ppc32, this ifeq
> # will work around it. If any other architecture displays this behavior,
> # add it here.
> ccflags-$(CONFIG_PPC32) := $(call cc-ifversion, -lt, 0400, -O1)
>
>      Arnd

Recently, I sent out patches to remove redundant GCC version checks,
including this one.

https://lore.kernel.org/patchwork/patch/977808/

I do not know who is maintaining reiserfs, though.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-27 20:42     ` Daniel Santos
  2018-08-27 21:01       ` Nick Desaulniers
  2018-08-28 10:55       ` Arnd Bergmann
@ 2018-08-28 23:00       ` Nick Desaulniers
  2018-08-31 16:46         ` Nick Desaulniers
  2 siblings, 1 reply; 22+ messages in thread
From: Nick Desaulniers @ 2018-08-28 23:00 UTC (permalink / raw)
  To: daniel.santos
  Cc: Masahiro Yamada, Linus Torvalds, Kees Cook, sparse, linux-sparse, LKML

On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>
> Hello Nick,
>
> On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> >>> Let's give up __compiletime_assert_fallback().  This commit does not
> >>> change the current behavior since it just rips off the useless code.
> >> Clang is not the only target audience of
> >> __compiletime_assert_fallback().  Instead of ripping out something that
> >> may benefit builds with gcc 4.2 and earlier, why not override its
> > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> > version to 4.6") that gcc < 4.6 is irrelevant.
>
> Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
> I guess I would be OK with its removal, but I still think it would be
> better if a similar mechanism to break the Clang build could be found.

I'm consulting with our best language lawyers to see what combinations
of _Static_assert and __builtin_constant_p would do the trick.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-28 23:00       ` Nick Desaulniers
@ 2018-08-31 16:46         ` Nick Desaulniers
  2018-09-26 18:00           ` Matthias Kaehlcke
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Desaulniers @ 2018-08-31 16:46 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Masahiro Yamada, Kees Cook, sparse, linux-sparse, LKML, daniel.santos

On Tue, Aug 28, 2018 at 4:00 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
> >
> > Hello Nick,
> >
> > On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> > >>> Let's give up __compiletime_assert_fallback().  This commit does not
> > >>> change the current behavior since it just rips off the useless code.
> > >> Clang is not the only target audience of
> > >> __compiletime_assert_fallback().  Instead of ripping out something that
> > >> may benefit builds with gcc 4.2 and earlier, why not override its
> > > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> > > version to 4.6") that gcc < 4.6 is irrelevant.
> >
> > Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
> > I guess I would be OK with its removal, but I still think it would be
> > better if a similar mechanism to break the Clang build could be found.
>
> I'm consulting with our best language lawyers to see what combinations
> of _Static_assert and __builtin_constant_p would do the trick.

Linus,
Can this patch be merged in the meantime?

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-08-31 16:46         ` Nick Desaulniers
@ 2018-09-26 18:00           ` Matthias Kaehlcke
  2018-09-26 18:03             ` Nick Desaulniers
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Kaehlcke @ 2018-09-26 18:00 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Linus Torvalds, Masahiro Yamada, Kees Cook, sparse, linux-sparse,
	LKML, daniel.santos, Chris Wilson, Jani Nikula

On Fri, Aug 31, 2018 at 09:46:02AM -0700, Nick Desaulniers wrote:
> On Tue, Aug 28, 2018 at 4:00 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
> > >
> > > Hello Nick,
> > >
> > > On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> > > >>> Let's give up __compiletime_assert_fallback().  This commit does not
> > > >>> change the current behavior since it just rips off the useless code.
> > > >> Clang is not the only target audience of
> > > >> __compiletime_assert_fallback().  Instead of ripping out something that
> > > >> may benefit builds with gcc 4.2 and earlier, why not override its
> > > > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> > > > version to 4.6") that gcc < 4.6 is irrelevant.
> > >
> > > Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
> > > I guess I would be OK with its removal, but I still think it would be
> > > better if a similar mechanism to break the Clang build could be found.
> >
> > I'm consulting with our best language lawyers to see what combinations
> > of _Static_assert and __builtin_constant_p would do the trick.
> 
> Linus,
> Can this patch be merged in the meantime?

friendly ping :)

With c5c2b11894f4 ("drm/i915: Warn against variable length arrays")
clang raises plenty of vla warnings about
__compiletime_error_fallback() in the i915 driver. Would be great to
get rid of those without having to revert that commit.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 18:00           ` Matthias Kaehlcke
@ 2018-09-26 18:03             ` Nick Desaulniers
  2018-09-26 18:26               ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Desaulniers @ 2018-09-26 18:03 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Linus Torvalds, Masahiro Yamada, Kees Cook, sparse, linux-sparse,
	LKML, daniel.santos, chris, jani.nikula

On Wed, Sep 26, 2018 at 11:00 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Fri, Aug 31, 2018 at 09:46:02AM -0700, Nick Desaulniers wrote:
> > On Tue, Aug 28, 2018 at 4:00 PM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
> > > >
> > > > Hello Nick,
> > > >
> > > > On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> > > > >>> Let's give up __compiletime_assert_fallback().  This commit does not
> > > > >>> change the current behavior since it just rips off the useless code.
> > > > >> Clang is not the only target audience of
> > > > >> __compiletime_assert_fallback().  Instead of ripping out something that
> > > > >> may benefit builds with gcc 4.2 and earlier, why not override its
> > > > > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> > > > > version to 4.6") that gcc < 4.6 is irrelevant.
> > > >
> > > > Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
> > > > I guess I would be OK with its removal, but I still think it would be
> > > > better if a similar mechanism to break the Clang build could be found.
> > >
> > > I'm consulting with our best language lawyers to see what combinations
> > > of _Static_assert and __builtin_constant_p would do the trick.
> >
> > Linus,
> > Can this patch be merged in the meantime?
>
> friendly ping :)
>
> With c5c2b11894f4 ("drm/i915: Warn against variable length arrays")
> clang raises plenty of vla warnings about
> __compiletime_error_fallback() in the i915 driver. Would be great to
> get rid of those without having to revert that commit.

I've been meaning to follow up on this, thanks Matthias.  I too would
really like this patch.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 18:03             ` Nick Desaulniers
@ 2018-09-26 18:26               ` Kees Cook
  2018-09-26 18:42                 ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2018-09-26 18:26 UTC (permalink / raw)
  To: Greg KH
  Cc: Nick Desaulniers, Matthias Kaehlcke, Linus Torvalds,
	Masahiro Yamada, Christopher Li, Sparse Mailing-list, LKML,
	Daniel Santos, Chris Wilson, Jani Nikula

On Wed, Sep 26, 2018 at 11:03 AM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Wed, Sep 26, 2018 at 11:00 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>>
>> On Fri, Aug 31, 2018 at 09:46:02AM -0700, Nick Desaulniers wrote:
>> > On Tue, Aug 28, 2018 at 4:00 PM Nick Desaulniers
>> > <ndesaulniers@google.com> wrote:
>> > >
>> > > On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
>> > > >
>> > > > Hello Nick,
>> > > >
>> > > > On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
>> > > > >>> Let's give up __compiletime_assert_fallback().  This commit does not
>> > > > >>> change the current behavior since it just rips off the useless code.
>> > > > >> Clang is not the only target audience of
>> > > > >> __compiletime_assert_fallback().  Instead of ripping out something that
>> > > > >> may benefit builds with gcc 4.2 and earlier, why not override its
>> > > > > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
>> > > > > version to 4.6") that gcc < 4.6 is irrelevant.
>> > > >
>> > > > Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
>> > > > I guess I would be OK with its removal, but I still think it would be
>> > > > better if a similar mechanism to break the Clang build could be found.
>> > >
>> > > I'm consulting with our best language lawyers to see what combinations
>> > > of _Static_assert and __builtin_constant_p would do the trick.
>> >
>> > Linus,
>> > Can this patch be merged in the meantime?
>>
>> friendly ping :)
>>
>> With c5c2b11894f4 ("drm/i915: Warn against variable length arrays")
>> clang raises plenty of vla warnings about
>> __compiletime_error_fallback() in the i915 driver. Would be great to
>> get rid of those without having to revert that commit.
>
> I've been meaning to follow up on this, thanks Matthias.  I too would
> really like this patch.

Adding Greg to the thread. Between Masahiro's detailed commit log and
the Clang-familiar reviewers, I think this should land for 4.19 (as
part of the other Clang-sanity patches that are already in 4.19). This
has no impact on gcc now that we're requiring 4.6+.

https://lore.kernel.org/patchwork/patch/977668/

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 18:26               ` Kees Cook
@ 2018-09-26 18:42                 ` Greg KH
  2018-09-26 18:45                   ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2018-09-26 18:42 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nick Desaulniers, Matthias Kaehlcke, Linus Torvalds,
	Masahiro Yamada, Christopher Li, Sparse Mailing-list, LKML,
	Daniel Santos, Chris Wilson, Jani Nikula

On Wed, Sep 26, 2018 at 11:26:46AM -0700, Kees Cook wrote:
> On Wed, Sep 26, 2018 at 11:03 AM, Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > On Wed, Sep 26, 2018 at 11:00 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> >>
> >> On Fri, Aug 31, 2018 at 09:46:02AM -0700, Nick Desaulniers wrote:
> >> > On Tue, Aug 28, 2018 at 4:00 PM Nick Desaulniers
> >> > <ndesaulniers@google.com> wrote:
> >> > >
> >> > > On Mon, Aug 27, 2018 at 1:42 PM Daniel Santos <daniel.santos@pobox.com> wrote:
> >> > > >
> >> > > > Hello Nick,
> >> > > >
> >> > > > On 08/27/2018 03:09 PM, Nick Desaulniers wrote:
> >> > > > >>> Let's give up __compiletime_assert_fallback().  This commit does not
> >> > > > >>> change the current behavior since it just rips off the useless code.
> >> > > > >> Clang is not the only target audience of
> >> > > > >> __compiletime_assert_fallback().  Instead of ripping out something that
> >> > > > >> may benefit builds with gcc 4.2 and earlier, why not override its
> >> > > > > Note that with commit cafa0010cd51 ("Raise the minimum required gcc
> >> > > > > version to 4.6") that gcc < 4.6 is irrelevant.
> >> > > >
> >> > > > Ah, I guess I'm not keeping up, that's wonderful news!  Considering that
> >> > > > I guess I would be OK with its removal, but I still think it would be
> >> > > > better if a similar mechanism to break the Clang build could be found.
> >> > >
> >> > > I'm consulting with our best language lawyers to see what combinations
> >> > > of _Static_assert and __builtin_constant_p would do the trick.
> >> >
> >> > Linus,
> >> > Can this patch be merged in the meantime?
> >>
> >> friendly ping :)
> >>
> >> With c5c2b11894f4 ("drm/i915: Warn against variable length arrays")
> >> clang raises plenty of vla warnings about
> >> __compiletime_error_fallback() in the i915 driver. Would be great to
> >> get rid of those without having to revert that commit.
> >
> > I've been meaning to follow up on this, thanks Matthias.  I too would
> > really like this patch.
> 
> Adding Greg to the thread. Between Masahiro's detailed commit log and
> the Clang-familiar reviewers, I think this should land for 4.19 (as
> part of the other Clang-sanity patches that are already in 4.19). This
> has no impact on gcc now that we're requiring 4.6+.
> 
> https://lore.kernel.org/patchwork/patch/977668/

I'm not digging up a compiler.h patch from a web site and adding it to
the tree this late in the release cycle.  Especially given that it
hasn't had any testing anywhere...

nice try though :)

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 18:42                 ` Greg KH
@ 2018-09-26 18:45                   ` Kees Cook
  2018-09-26 19:03                     ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2018-09-26 18:45 UTC (permalink / raw)
  To: Greg KH
  Cc: Nick Desaulniers, Matthias Kaehlcke, Linus Torvalds,
	Masahiro Yamada, Christopher Li, Sparse Mailing-list, LKML,
	Daniel Santos, Chris Wilson, Jani Nikula

On Wed, Sep 26, 2018 at 11:42 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> I'm not digging up a compiler.h patch from a web site and adding it to
> the tree this late in the release cycle.  Especially given that it
> hasn't had any testing anywhere...

Good point about it not living in -next.

Who should be carrying these sorts of patches? In the past it's been
Andrew or Masahiro, yes? For linux-next, maybe it can go via -mm?

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 18:45                   ` Kees Cook
@ 2018-09-26 19:03                     ` Greg KH
  2018-09-26 19:29                       ` Nick Desaulniers
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2018-09-26 19:03 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nick Desaulniers, Matthias Kaehlcke, Linus Torvalds,
	Masahiro Yamada, Christopher Li, Sparse Mailing-list, LKML,
	Daniel Santos, Chris Wilson, Jani Nikula

On Wed, Sep 26, 2018 at 11:45:19AM -0700, Kees Cook wrote:
> On Wed, Sep 26, 2018 at 11:42 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > I'm not digging up a compiler.h patch from a web site and adding it to
> > the tree this late in the release cycle.  Especially given that it
> > hasn't had any testing anywhere...
> 
> Good point about it not living in -next.
> 
> Who should be carrying these sorts of patches? In the past it's been
> Andrew or Masahiro, yes? For linux-next, maybe it can go via -mm?

Either is fine with me, as long as it isn't one of my trees :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 19:03                     ` Greg KH
@ 2018-09-26 19:29                       ` Nick Desaulniers
  2018-09-26 19:35                         ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Nick Desaulniers @ 2018-09-26 19:29 UTC (permalink / raw)
  To: Kees Cook, Matthias Kaehlcke
  Cc: Linus Torvalds, Masahiro Yamada, sparse, linux-sparse, LKML,
	daniel.santos, chris, jani.nikula, Greg KH

On Wed, Sep 26, 2018 at 12:03 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 26, 2018 at 11:45:19AM -0700, Kees Cook wrote:
> > On Wed, Sep 26, 2018 at 11:42 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > I'm not digging up a compiler.h patch from a web site and adding it to
> > > the tree this late in the release cycle.  Especially given that it
> > > hasn't had any testing anywhere...
> >
> > Good point about it not living in -next.
> >
> > Who should be carrying these sorts of patches? In the past it's been
> > Andrew or Masahiro, yes? For linux-next, maybe it can go via -mm?
>
> Either is fine with me, as long as it isn't one of my trees :)
>
> thanks,
>
> greg k-h

Besides, I think we want the v2: https://lkml.org/lkml/2018/8/25/103
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 19:29                       ` Nick Desaulniers
@ 2018-09-26 19:35                         ` Kees Cook
  2018-10-10  6:10                           ` Joel Stanley
  0 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2018-09-26 19:35 UTC (permalink / raw)
  To: Nick Desaulniers, Andrew Morton
  Cc: Matthias Kaehlcke, Linus Torvalds, Masahiro Yamada,
	Christopher Li, Sparse Mailing-list, LKML, Daniel Santos,
	Chris Wilson, Jani Nikula, Greg KH

On Wed, Sep 26, 2018 at 12:29 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Wed, Sep 26, 2018 at 12:03 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Wed, Sep 26, 2018 at 11:45:19AM -0700, Kees Cook wrote:
>> > On Wed, Sep 26, 2018 at 11:42 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > > I'm not digging up a compiler.h patch from a web site and adding it to
>> > > the tree this late in the release cycle.  Especially given that it
>> > > hasn't had any testing anywhere...
>> >
>> > Good point about it not living in -next.
>> >
>> > Who should be carrying these sorts of patches? In the past it's been
>> > Andrew or Masahiro, yes? For linux-next, maybe it can go via -mm?
>>
>> Either is fine with me, as long as it isn't one of my trees :)
>>
>> thanks,
>>
>> greg k-h
>
> Besides, I think we want the v2: https://lkml.org/lkml/2018/8/25/103

Yeah, that's what I'd linked to in the patchwork URL. Andrew, can you take this?

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-09-26 19:35                         ` Kees Cook
@ 2018-10-10  6:10                           ` Joel Stanley
  2018-10-10  7:03                             ` Miguel Ojeda
  0 siblings, 1 reply; 22+ messages in thread
From: Joel Stanley @ 2018-10-10  6:10 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nick Desaulniers, Andrew Morton, mka, Linus Torvalds,
	Masahiro Yamada, sparse, linux-sparse, Linux Kernel Mailing List,
	daniel.santos, chris, jani.nikula, Greg KH

On Thu, 27 Sep 2018 at 05:07, Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Sep 26, 2018 at 12:29 PM, Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > On Wed, Sep 26, 2018 at 12:03 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Wed, Sep 26, 2018 at 11:45:19AM -0700, Kees Cook wrote:
> >> > On Wed, Sep 26, 2018 at 11:42 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > > I'm not digging up a compiler.h patch from a web site and adding it to
> >> > > the tree this late in the release cycle.  Especially given that it
> >> > > hasn't had any testing anywhere...
> >> >
> >> > Good point about it not living in -next.
> >> >
> >> > Who should be carrying these sorts of patches? In the past it's been
> >> > Andrew or Masahiro, yes? For linux-next, maybe it can go via -mm?
> >>
> >> Either is fine with me, as long as it isn't one of my trees :)
> >>
> >> thanks,
> >>
> >> greg k-h
> >
> > Besides, I think we want the v2: https://lkml.org/lkml/2018/8/25/103
>
> Yeah, that's what I'd linked to in the patchwork URL. Andrew, can you take this?

clang built -next is blowing up now that Kees' -Wvla patch has been
included. This patch fixes it.

Kees, perhaps it should go in your tree along side of the -Wvla patch
if no one else wants to take it?

Cheers,

Joel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-10-10  6:10                           ` Joel Stanley
@ 2018-10-10  7:03                             ` Miguel Ojeda
  2018-10-10 14:49                               ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Miguel Ojeda @ 2018-10-10  7:03 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Kees Cook, Nick Desaulniers, Andrew Morton, Matthias Kaehlcke,
	Linus Torvalds, Masahiro Yamada, Christopher Li, linux-sparse,
	linux-kernel, daniel.santos, chris, jani.nikula, Greg KH

On Wed, Oct 10, 2018 at 8:12 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Thu, 27 Sep 2018 at 05:07, Kees Cook <keescook@chromium.org> wrote:
> >
> > Yeah, that's what I'd linked to in the patchwork URL. Andrew, can you take this?
>
> clang built -next is blowing up now that Kees' -Wvla patch has been
> included. This patch fixes it.
>
> Kees, perhaps it should go in your tree along side of the -Wvla patch
> if no one else wants to take it?
>

I can take it along in the compiler attributes tree, since that
touches the compiler*.h stuff. Although that would make it
not-only-attributes, i.e. slightly lying :-)

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-10-10  7:03                             ` Miguel Ojeda
@ 2018-10-10 14:49                               ` Kees Cook
  2018-10-11  2:48                                 ` Masahiro Yamada
  0 siblings, 1 reply; 22+ messages in thread
From: Kees Cook @ 2018-10-10 14:49 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Joel Stanley, Nick Desaulniers, Andrew Morton, Matthias Kaehlcke,
	Linus Torvalds, Masahiro Yamada, Christopher Li,
	Sparse Mailing-list, linux-kernel, Daniel Santos, Chris Wilson,
	Jani Nikula, Greg KH

On Wed, Oct 10, 2018 at 12:03 AM, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> On Wed, Oct 10, 2018 at 8:12 AM Joel Stanley <joel@jms.id.au> wrote:
>>
>> On Thu, 27 Sep 2018 at 05:07, Kees Cook <keescook@chromium.org> wrote:
>> >
>> > Yeah, that's what I'd linked to in the patchwork URL. Andrew, can you take this?
>>
>> clang built -next is blowing up now that Kees' -Wvla patch has been
>> included. This patch fixes it.
>>
>> Kees, perhaps it should go in your tree along side of the -Wvla patch
>> if no one else wants to take it?
>>
>
> I can take it along in the compiler attributes tree, since that
> touches the compiler*.h stuff. Although that would make it
> not-only-attributes, i.e. slightly lying :-)

Oh, I had assumed Masahiro was going to carry it. If that's not true,
sure I'll pick it up as part of my VLA "series".

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-10-10 14:49                               ` Kees Cook
@ 2018-10-11  2:48                                 ` Masahiro Yamada
  2018-10-11 15:15                                   ` Kees Cook
  0 siblings, 1 reply; 22+ messages in thread
From: Masahiro Yamada @ 2018-10-11  2:48 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Joel Stanley, Nick Desaulniers, Andrew Morton,
	Matthias Kaehlcke, Linus Torvalds, Christopher Li, linux-sparse,
	Linux Kernel Mailing List, Daniel Santos, Chris Wilson,
	jani.nikula, Greg Kroah-Hartman

On Wed, Oct 10, 2018 at 11:51 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Oct 10, 2018 at 12:03 AM, Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> > On Wed, Oct 10, 2018 at 8:12 AM Joel Stanley <joel@jms.id.au> wrote:
> >>
> >> On Thu, 27 Sep 2018 at 05:07, Kees Cook <keescook@chromium.org> wrote:
> >> >
> >> > Yeah, that's what I'd linked to in the patchwork URL. Andrew, can you take this?
> >>
> >> clang built -next is blowing up now that Kees' -Wvla patch has been
> >> included. This patch fixes it.
> >>
> >> Kees, perhaps it should go in your tree along side of the -Wvla patch
> >> if no one else wants to take it?
> >>
> >
> > I can take it along in the compiler attributes tree, since that
> > touches the compiler*.h stuff. Although that would make it
> > not-only-attributes, i.e. slightly lying :-)
>
> Oh, I had assumed Masahiro was going to carry it.

No, I am not.

Putting all sort of things into kbuild basket
is painful for me.



> If that's not true,
> sure I'll pick it up as part of my VLA "series".


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] compiler.h: give up __compiletime_assert_fallback()
  2018-10-11  2:48                                 ` Masahiro Yamada
@ 2018-10-11 15:15                                   ` Kees Cook
  0 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2018-10-11 15:15 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Miguel Ojeda, Joel Stanley, Nick Desaulniers, Andrew Morton,
	Matthias Kaehlcke, Linus Torvalds, Christopher Li,
	Sparse Mailing-list, Linux Kernel Mailing List, Daniel Santos,
	Chris Wilson, Jani Nikula, Greg Kroah-Hartman

On Wed, Oct 10, 2018 at 7:48 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> On Wed, Oct 10, 2018 at 11:51 PM Kees Cook <keescook@chromium.org> wrote:
>>
>> On Wed, Oct 10, 2018 at 12:03 AM, Miguel Ojeda
>> <miguel.ojeda.sandonis@gmail.com> wrote:
>> > On Wed, Oct 10, 2018 at 8:12 AM Joel Stanley <joel@jms.id.au> wrote:
>> >>
>> >> On Thu, 27 Sep 2018 at 05:07, Kees Cook <keescook@chromium.org> wrote:
>> >> >
>> >> > Yeah, that's what I'd linked to in the patchwork URL. Andrew, can you take this?
>> >>
>> >> clang built -next is blowing up now that Kees' -Wvla patch has been
>> >> included. This patch fixes it.
>> >>
>> >> Kees, perhaps it should go in your tree along side of the -Wvla patch
>> >> if no one else wants to take it?
>> >>
>> >
>> > I can take it along in the compiler attributes tree, since that
>> > touches the compiler*.h stuff. Although that would make it
>> > not-only-attributes, i.e. slightly lying :-)
>>
>> Oh, I had assumed Masahiro was going to carry it.
>
> No, I am not.
>
> Putting all sort of things into kbuild basket
> is painful for me.

Okay, I'll take it for the VLA series.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-10-11 15:15 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-25 18:16 [PATCH v2] compiler.h: give up __compiletime_assert_fallback() Masahiro Yamada
2018-08-27 20:05 ` Daniel Santos
2018-08-27 20:09   ` Nick Desaulniers
2018-08-27 20:42     ` Daniel Santos
2018-08-27 21:01       ` Nick Desaulniers
2018-08-28 10:55       ` Arnd Bergmann
2018-08-28 13:46         ` Masahiro Yamada
2018-08-28 23:00       ` Nick Desaulniers
2018-08-31 16:46         ` Nick Desaulniers
2018-09-26 18:00           ` Matthias Kaehlcke
2018-09-26 18:03             ` Nick Desaulniers
2018-09-26 18:26               ` Kees Cook
2018-09-26 18:42                 ` Greg KH
2018-09-26 18:45                   ` Kees Cook
2018-09-26 19:03                     ` Greg KH
2018-09-26 19:29                       ` Nick Desaulniers
2018-09-26 19:35                         ` Kees Cook
2018-10-10  6:10                           ` Joel Stanley
2018-10-10  7:03                             ` Miguel Ojeda
2018-10-10 14:49                               ` Kees Cook
2018-10-11  2:48                                 ` Masahiro Yamada
2018-10-11 15:15                                   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).