2017-08-24 16:45:23

by Joe Stringer

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

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

Signed-off-by: Joe Stringer <[email protected]>
---
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


2017-08-24 16:59:08

by Joe Stringer

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

On 24 August 2017 at 09:45, Joe Stringer <[email protected]> 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/[email protected]
>
> Signed-off-by: Joe Stringer <[email protected]>

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.

2017-08-24 21:20:21

by Arnd Bergmann

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

On Thu, Aug 24, 2017 at 6:45 PM, Joe Stringer <[email protected]> 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

2017-08-25 11:45:33

by Michal Nazarewicz

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

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/[email protected]
>
> Signed-off-by: Joe Stringer <[email protected]>
> ---
> 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»

2017-08-25 20:54:03

by Joe Stringer

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

On 24 August 2017 at 14:20, Arnd Bergmann <[email protected]> wrote:
> On Thu, Aug 24, 2017 at 6:45 PM, Joe Stringer <[email protected]> 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.

2017-08-25 20:54:32

by Joe Stringer

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

On 25 August 2017 at 04:45, Michal Nazarewicz <[email protected]> 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/[email protected]
>>
>> Signed-off-by: Joe Stringer <[email protected]>
>> ---
>> 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.