* [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).