[PATCH net-next] compiler: Document behavior compiling with -O0

[PATCH net-next] compiler: Document behavior compiling with -O0

[Joe Stringer]

Recent changes[0] to make use of __compiletime_assert() from container_of()
increased the scope of this macro, resulting in a wider set of
situations where developers cannot compile their code using "-O0". I
noticed this when making use of the macro in my own development, and
spent more time than I'd like to admit tracking the problem down. This
patch documents the behavior in lieu of a compile-time assertion
implementation that does not rely on optimizations.

Example compilation failure:

./include/linux/compiler.h:547:38: error: call to ‘__compiletime_assert_94’ declared with attribute error: pointer type mismatch in container_of()
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                      ^
./include/linux/compiler.h:530:4: note: in definition of macro ‘__compiletime_assert’
    prefix ## suffix();    \
    ^~~~~~
./include/linux/compiler.h:547:2: note: in expansion of macro ‘_compiletime_assert’
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
  ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
 #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                     ^~~~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
  ^~~~~~~~~~~~~~~~

[0] http://lkml.kernel.org/r/20170525120316.24473-7-abbotti@mev.co.uk

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 include/linux/compiler.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index eca8ad75e28b..bb640167fdac 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -517,6 +517,11 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 # define __compiletime_error_fallback(condition) do { } while (0)
 #endif
 
+/*
+ * __compiletime_assert() relies on compiler optimizations to remove the check
+ * against '__cond' if 'condition' is false. As a result, compiling with -O0
+ * will cause compilation errors here regardless of the value of 'condition'.
+ */
 #define __compiletime_assert(condition, msg, prefix, suffix)		\
 	do {								\
 		bool __cond = !(condition);				\
-- 
2.14.1

Re: [PATCH net-next] compiler: Document behavior compiling with -O0

[Joe Stringer]

On 24 August 2017 at 09:45, Joe Stringer <joe@ovn.org> wrote:
> Recent changes[0] to make use of __compiletime_assert() from container_of()
> increased the scope of this macro, resulting in a wider set of
> situations where developers cannot compile their code using "-O0". I
> noticed this when making use of the macro in my own development, and
> spent more time than I'd like to admit tracking the problem down. This
> patch documents the behavior in lieu of a compile-time assertion
> implementation that does not rely on optimizations.
>
> Example compilation failure:
>
> ./include/linux/compiler.h:547:38: error: call to ‘__compiletime_assert_94’ declared with attribute error: pointer type mismatch in container_of()
>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>                                       ^
> ./include/linux/compiler.h:530:4: note: in definition of macro ‘__compiletime_assert’
>     prefix ## suffix();    \
>     ^~~~~~
> ./include/linux/compiler.h:547:2: note: in expansion of macro ‘_compiletime_assert’
>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>   ^~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
>  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>                                      ^~~~~~~~~~~~~~~~~~
> ./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
>   ^~~~~~~~~~~~~~~~
>
> [0] http://lkml.kernel.org/r/20170525120316.24473-7-abbotti@mev.co.uk
>
> Signed-off-by: Joe Stringer <joe@ovn.org>

This patch was written against the net-next tree, but is not targeted
at the net-next tree. My usual submissions go there so my formatting
scripts just inserted that. I can strip out and resend against a
particular tree if this is a problem.

Re: [PATCH net-next] compiler: Document behavior compiling with -O0

[Arnd Bergmann]

On Thu, Aug 24, 2017 at 6:45 PM, Joe Stringer <joe@ovn.org> wrote:
> Recent changes[0] to make use of __compiletime_assert() from container_of()
> increased the scope of this macro, resulting in a wider set of
> situations where developers cannot compile their code using "-O0". I
> noticed this when making use of the macro in my own development, and
> spent more time than I'd like to admit tracking the problem down. This
> patch documents the behavior in lieu of a compile-time assertion
> implementation that does not rely on optimizations.
>
> Example compilation failure:

Maybe the macro should be enclosed in "#ifdef __OPTIMIZE__"?

Generally speaking we at least rely on function inlining to create a
working kernel, but sometimes there may be a reason to compile a
single file without optimizations.

        Arnd

Re: [PATCH net-next] compiler: Document behavior compiling with -O0

[Joe Stringer]

On 24 August 2017 at 14:20, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Aug 24, 2017 at 6:45 PM, Joe Stringer <joe@ovn.org> wrote:
>> Recent changes[0] to make use of __compiletime_assert() from container_of()
>> increased the scope of this macro, resulting in a wider set of
>> situations where developers cannot compile their code using "-O0". I
>> noticed this when making use of the macro in my own development, and
>> spent more time than I'd like to admit tracking the problem down. This
>> patch documents the behavior in lieu of a compile-time assertion
>> implementation that does not rely on optimizations.
>>
>> Example compilation failure:
>
> Maybe the macro should be enclosed in "#ifdef __OPTIMIZE__"?
>
> Generally speaking we at least rely on function inlining to create a
> working kernel, but sometimes there may be a reason to compile a
> single file without optimizations.

Right. The above approach seems okay to me, if the developer really
doesn't notice issues when they've been debugging -O0, it should be
picked up fairly quickly during review.

Re: [PATCH net-next] compiler: Document behavior compiling with -O0

[Michal Nazarewicz]

On Thu, Aug 24 2017, Joe Stringer wrote:
> Recent changes[0] to make use of __compiletime_assert() from container_of()
> increased the scope of this macro, resulting in a wider set of
> situations where developers cannot compile their code using "-O0". I
> noticed this when making use of the macro in my own development, and
> spent more time than I'd like to admit tracking the problem down. This
> patch documents the behavior in lieu of a compile-time assertion
> implementation that does not rely on optimizations.
>
> Example compilation failure:
>
> ./include/linux/compiler.h:547:38: error: call to ‘__compiletime_assert_94’ declared with attribute error: pointer type mismatch in container_of()
>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>                                       ^
> ./include/linux/compiler.h:530:4: note: in definition of macro ‘__compiletime_assert’
>     prefix ## suffix();    \
>     ^~~~~~
> ./include/linux/compiler.h:547:2: note: in expansion of macro ‘_compiletime_assert’
>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>   ^~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
>  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>                                      ^~~~~~~~~~~~~~~~~~
> ./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
>   ^~~~~~~~~~~~~~~~
>
> [0] http://lkml.kernel.org/r/20170525120316.24473-7-abbotti@mev.co.uk
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
>  include/linux/compiler.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index eca8ad75e28b..bb640167fdac 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -517,6 +517,11 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>  # define __compiletime_error_fallback(condition) do { } while (0)
>  #endif
>  
> +/*
> + * __compiletime_assert() relies on compiler optimizations to remove the check
> + * against '__cond' if 'condition' is false. As a result, compiling with -O0
> + * will cause compilation errors here regardless of the value of 'condition'.
> + */
>  #define __compiletime_assert(condition, msg, prefix, suffix)		\
>  	do {								\
>  		bool __cond = !(condition);				\

Could __builtin_choose_expr help here?  Something like:

#define __compiletime_assert(condition, msg, prefix, suffix)		\
	do {								\
		bool __cond = !(condition);				\
		extern int prefix ## suffix(void) __compiletime_error(msg); \
		__builting_choose_expr(cond, prefix ## suffix(), 0);	\
		__compiletime_error_fallback(__cond);			\
	} while (0)

Or better still, _Static_assert?

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

Re: [PATCH net-next] compiler: Document behavior compiling with -O0

[Joe Stringer]

On 25 August 2017 at 04:45, Michal Nazarewicz <mina86@mina86.com> wrote:
> On Thu, Aug 24 2017, Joe Stringer wrote:
>> Recent changes[0] to make use of __compiletime_assert() from container_of()
>> increased the scope of this macro, resulting in a wider set of
>> situations where developers cannot compile their code using "-O0". I
>> noticed this when making use of the macro in my own development, and
>> spent more time than I'd like to admit tracking the problem down. This
>> patch documents the behavior in lieu of a compile-time assertion
>> implementation that does not rely on optimizations.
>>
>> Example compilation failure:
>>
>> ./include/linux/compiler.h:547:38: error: call to ‘__compiletime_assert_94’ declared with attribute error: pointer type mismatch in container_of()
>>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>>                                       ^
>> ./include/linux/compiler.h:530:4: note: in definition of macro ‘__compiletime_assert’
>>     prefix ## suffix();    \
>>     ^~~~~~
>> ./include/linux/compiler.h:547:2: note: in expansion of macro ‘_compiletime_assert’
>>   _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>>   ^~~~~~~~~~~~~~~~~~~
>> ./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
>>  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>>                                      ^~~~~~~~~~~~~~~~~~
>> ./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>>   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
>>   ^~~~~~~~~~~~~~~~
>>
>> [0] http://lkml.kernel.org/r/20170525120316.24473-7-abbotti@mev.co.uk
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>>  include/linux/compiler.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index eca8ad75e28b..bb640167fdac 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -517,6 +517,11 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>>  # define __compiletime_error_fallback(condition) do { } while (0)
>>  #endif
>>
>> +/*
>> + * __compiletime_assert() relies on compiler optimizations to remove the check
>> + * against '__cond' if 'condition' is false. As a result, compiling with -O0
>> + * will cause compilation errors here regardless of the value of 'condition'.
>> + */
>>  #define __compiletime_assert(condition, msg, prefix, suffix)         \
>>       do {                                                            \
>>               bool __cond = !(condition);                             \
>
> Could __builtin_choose_expr help here?  Something like:
>
> #define __compiletime_assert(condition, msg, prefix, suffix)            \
>         do {                                                            \
>                 bool __cond = !(condition);                             \
>                 extern int prefix ## suffix(void) __compiletime_error(msg); \
>                 __builting_choose_expr(cond, prefix ## suffix(), 0);    \
>                 __compiletime_error_fallback(__cond);                   \
>         } while (0)
>
> Or better still, _Static_assert?

I tried both of the above, and they both complain that "condition"
isn't a constant.