linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] build_bug.h: add wrapper for _Static_assert
@ 2019-02-03 19:24 Rasmus Villemoes
  2019-02-04 23:09 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Rasmus Villemoes @ 2019-02-03 19:24 UTC (permalink / raw)
  To: Kees Cook, Nick Desaulniers, Andrew Morton, Masahiro Yamada,
	Luc Van Oostenryck, Rasmus Villemoes
  Cc: linux-kernel

BUILD_BUG_ON() is a little annoying, since it cannot be used outside
function scope. So one cannot put assertions about the sizeof() a
struct next to the struct definition, but has to hide that in some
more or less arbitrary function.

Since gcc 4.6 (which is now also the required minimum), there is
support for the C11 _Static_assert in all C modes, including gnu89. So
add a simple wrapper for that.

_Static_assert() requires a message argument, which is usually quite
redundant (and I believe that bug got fixed at least in newer C++
standards), but we can easily work around that with a little macro
magic, making it optional.

For example, adding

  static_assert(sizeof(struct printf_spec) == 8);

in vsprintf.c and modifying that struct to violate it, one gets

./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
 #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")

godbolt.org suggests that _Static_assert() has been support by clang
since at least 3.0.0.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/build_bug.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index faeec7433aab..4bf9ba847b44 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -58,4 +58,23 @@
  */
 #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
 
+/**
+ * static_assert - check integer constant expression at build time
+ *
+ * static_assert() is a wrapper for the C11 _Static_assert, with a
+ * little macro magic to make the message optional (defaulting to the
+ * stringification of the tested expression).
+ *
+ * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
+ * scope, but requires the expression to be an integer constant
+ * expression (i.e., it is not enough that __builtin_constant_p() is
+ * true for expr).
+ *
+ * Also note that BUILD_BUG_ON() fails the build if the condition is
+ * true, while static_assert() fails the build if the expression is
+ * false.
+ */
+#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
+#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
+
 #endif	/* _LINUX_BUILD_BUG_H */
-- 
2.20.1


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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-03 19:24 [PATCH] build_bug.h: add wrapper for _Static_assert Rasmus Villemoes
@ 2019-02-04 23:09 ` Andrew Morton
  2019-02-04 23:12   ` Andrew Morton
  2019-02-05  8:05 ` Masahiro Yamada
  2019-02-08 20:30 ` [PATCH v2 1/3] " Rasmus Villemoes
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2019-02-04 23:09 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Kees Cook, Nick Desaulniers, Masahiro Yamada, Luc Van Oostenryck,
	linux-kernel

On Sun,  3 Feb 2019 20:24:00 +0100 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> function scope. So one cannot put assertions about the sizeof() a
> struct next to the struct definition, but has to hide that in some
> more or less arbitrary function.
> 
> Since gcc 4.6 (which is now also the required minimum), there is
> support for the C11 _Static_assert in all C modes, including gnu89. So
> add a simple wrapper for that.
> 
> _Static_assert() requires a message argument, which is usually quite
> redundant (and I believe that bug got fixed at least in newer C++
> standards), but we can easily work around that with a little macro
> magic, making it optional.
> 
> For example, adding
> 
>   static_assert(sizeof(struct printf_spec) == 8);
> 
> in vsprintf.c and modifying that struct to violate it, one gets
> 
> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
>  #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> 
> godbolt.org suggests that _Static_assert() has been support by clang
> since at least 3.0.0.
> 

It would be (very) nice to actually use this macro in a few places so
it gets its build testing while in -next.


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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-04 23:09 ` Andrew Morton
@ 2019-02-04 23:12   ` Andrew Morton
  2019-02-05  9:53     ` Rasmus Villemoes
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2019-02-04 23:12 UTC (permalink / raw)
  To: Rasmus Villemoes, Kees Cook, Nick Desaulniers, Masahiro Yamada,
	Luc Van Oostenryck, linux-kernel

On Mon, 4 Feb 2019 15:09:16 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Sun,  3 Feb 2019 20:24:00 +0100 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> 
> > BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> > function scope. So one cannot put assertions about the sizeof() a
> > struct next to the struct definition, but has to hide that in some
> > more or less arbitrary function.
> > 
> > Since gcc 4.6 (which is now also the required minimum), there is
> > support for the C11 _Static_assert in all C modes, including gnu89. So
> > add a simple wrapper for that.
> > 
> > _Static_assert() requires a message argument, which is usually quite
> > redundant (and I believe that bug got fixed at least in newer C++
> > standards), but we can easily work around that with a little macro
> > magic, making it optional.
> > 
> > For example, adding
> > 
> >   static_assert(sizeof(struct printf_spec) == 8);
> > 
> > in vsprintf.c and modifying that struct to violate it, one gets
> > 
> > ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
> >  #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> > 
> > godbolt.org suggests that _Static_assert() has been support by clang
> > since at least 3.0.0.
> > 
> 
> It would be (very) nice to actually use this macro in a few places so
> it gets its build testing while in -next.

ie, just about every BUILD_BUG_ON in mm/ could use this.  Let's switch
a few?


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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-03 19:24 [PATCH] build_bug.h: add wrapper for _Static_assert Rasmus Villemoes
  2019-02-04 23:09 ` Andrew Morton
@ 2019-02-05  8:05 ` Masahiro Yamada
  2019-02-05  9:38   ` Rasmus Villemoes
  2019-02-08 20:30 ` [PATCH v2 1/3] " Rasmus Villemoes
  2 siblings, 1 reply; 12+ messages in thread
From: Masahiro Yamada @ 2019-02-05  8:05 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Kees Cook, Nick Desaulniers, Andrew Morton, Luc Van Oostenryck,
	Linux Kernel Mailing List

On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> function scope. So one cannot put assertions about the sizeof() a
> struct next to the struct definition, but has to hide that in some
> more or less arbitrary function.
>
> Since gcc 4.6 (which is now also the required minimum), there is
> support for the C11 _Static_assert in all C modes, including gnu89. So
> add a simple wrapper for that.
>
> _Static_assert() requires a message argument, which is usually quite
> redundant (and I believe that bug got fixed at least in newer C++
> standards), but we can easily work around that with a little macro
> magic, making it optional.
>
> For example, adding
>
>   static_assert(sizeof(struct printf_spec) == 8);
>
> in vsprintf.c and modifying that struct to violate it, one gets
>
> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
>  #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
>
> godbolt.org suggests that _Static_assert() has been support by clang
> since at least 3.0.0.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  include/linux/build_bug.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
> index faeec7433aab..4bf9ba847b44 100644
> --- a/include/linux/build_bug.h
> +++ b/include/linux/build_bug.h
> @@ -58,4 +58,23 @@
>   */
>  #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>
> +/**
> + * static_assert - check integer constant expression at build time
> + *
> + * static_assert() is a wrapper for the C11 _Static_assert, with a
> + * little macro magic to make the message optional (defaulting to the
> + * stringification of the tested expression).
> + *
> + * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
> + * scope, but requires the expression to be an integer constant
> + * expression (i.e., it is not enough that __builtin_constant_p() is
> + * true for expr).
> + *
> + * Also note that BUILD_BUG_ON() fails the build if the condition is
> + * true, while static_assert() fails the build if the expression is
> + * false.
> + */
> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")

What is the "" "" for?


Bikeshed:

There might be room for argument about
where this macro should go.

Another possible place is <linux/compiler.h>
where compiletime_assert() is defined.


Thanks.



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-05  8:05 ` Masahiro Yamada
@ 2019-02-05  9:38   ` Rasmus Villemoes
  2019-02-05 14:06     ` Masahiro Yamada
  2019-02-07  0:14     ` Nick Desaulniers
  0 siblings, 2 replies; 12+ messages in thread
From: Rasmus Villemoes @ 2019-02-05  9:38 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Kees Cook, Nick Desaulniers, Andrew Morton, Luc Van Oostenryck,
	Linux Kernel Mailing List

On 05/02/2019 09.05, Masahiro Yamada wrote:
> On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>>
>> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
>> function scope. So one cannot put assertions about the sizeof() a
>> struct next to the struct definition, but has to hide that in some
>> more or less arbitrary function.
>>
>> Since gcc 4.6 (which is now also the required minimum), there is
>> support for the C11 _Static_assert in all C modes, including gnu89. So
>> add a simple wrapper for that.
>>
>> _Static_assert() requires a message argument, which is usually quite
>> redundant (and I believe that bug got fixed at least in newer C++
>> standards), but we can easily work around that with a little macro
>> magic, making it optional.
>>
>> For example, adding
>>
>>   static_assert(sizeof(struct printf_spec) == 8);
>>
>> in vsprintf.c and modifying that struct to violate it, one gets
>>
>> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
>>  #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
>>
>> godbolt.org suggests that _Static_assert() has been support by clang
>> since at least 3.0.0.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>>  include/linux/build_bug.h | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
>> index faeec7433aab..4bf9ba847b44 100644
>> --- a/include/linux/build_bug.h
>> +++ b/include/linux/build_bug.h
>> @@ -58,4 +58,23 @@
>>   */
>>  #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>>
>> +/**
>> + * static_assert - check integer constant expression at build time
>> + *
>> + * static_assert() is a wrapper for the C11 _Static_assert, with a
>> + * little macro magic to make the message optional (defaulting to the
>> + * stringification of the tested expression).
>> + *
>> + * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
>> + * scope, but requires the expression to be an integer constant
>> + * expression (i.e., it is not enough that __builtin_constant_p() is
>> + * true for expr).
>> + *
>> + * Also note that BUILD_BUG_ON() fails the build if the condition is
>> + * true, while static_assert() fails the build if the expression is
>> + * false.
>> + */
>> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
>> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> 
> What is the "" "" for?

Good point. It's a leftover from when I had a fallback-implementation of
_Static_assert for gcc < 4.6, where I wanted to ensure that the second
argument was a string literal, even if my fallback implementation
ignored that argument. Now it's actually a little harmful, because

foobar.c:5:34: error: expected string literal before ‘expected’
 static_assert(sizeof(long) == 8, expected 64 bit machine);

is better than

foobar.c:4:34: error: expected ‘)’ before ‘expected’
 static_assert(sizeof(long) == 8, expected 64 bit machine);

> Bikeshed:
> 
> There might be room for argument about
> where this macro should go.
> 
> Another possible place is <linux/compiler.h>
> where compiletime_assert() is defined.

I'd rather move compiletime_assert to build_bug.h, and rename it so that
it becomes an implementation detail of BUILD_BUG. There are not that
many direct users of compiletime_assert(), and I think we should
standardize on fewer ways of achieving the same thing. static_assert()
for checking ICEs, usable at any scope, and BUILD_BUG_* for checking
that the optimizer is sufficiently smart.

This would also be a step towards another cleanup I'd like to do: make
build_bug.h not depend on compiler.h, because we already have a
dependency in the other direction (ARRAY_SIZE using BUILD_BUG_ON_ZERO).

Rasmus

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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-04 23:12   ` Andrew Morton
@ 2019-02-05  9:53     ` Rasmus Villemoes
  2019-02-05 20:27       ` Luc Van Oostenryck
  0 siblings, 1 reply; 12+ messages in thread
From: Rasmus Villemoes @ 2019-02-05  9:53 UTC (permalink / raw)
  To: Andrew Morton, Kees Cook, Nick Desaulniers, Masahiro Yamada,
	Luc Van Oostenryck, linux-kernel

On 05/02/2019 00.12, Andrew Morton wrote:
>>
>> It would be (very) nice to actually use this macro in a few places so
>> it gets its build testing while in -next.
> 
> ie, just about every BUILD_BUG_ON in mm/ could use this.  Let's switch
> a few?
> 

Perhaps, but some make sense where they are, e.g. when iterating over
two arrays using one iterator variable, it's fine to assert they have
the same size just above the loop. And, unfortunately, static_assert()
is not quite a drop-in replacement for BUILD_BUG_ON even when the
argument is an ICE due to -Wdeclaration-after-statement.

I'll fix the two BUILD_BUGs I know I've added over the years (one in
vsprintf.c, one in fs/namei.c).

Rasmus

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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-05  9:38   ` Rasmus Villemoes
@ 2019-02-05 14:06     ` Masahiro Yamada
  2019-02-07  0:14     ` Nick Desaulniers
  1 sibling, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2019-02-05 14:06 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Kees Cook, Nick Desaulniers, Andrew Morton, Luc Van Oostenryck,
	Linux Kernel Mailing List

On Tue, Feb 5, 2019 at 6:39 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 05/02/2019 09.05, Masahiro Yamada wrote:
> > On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
> > <linux@rasmusvillemoes.dk> wrote:
> >>
> >> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> >> function scope. So one cannot put assertions about the sizeof() a
> >> struct next to the struct definition, but has to hide that in some
> >> more or less arbitrary function.
> >>
> >> Since gcc 4.6 (which is now also the required minimum), there is
> >> support for the C11 _Static_assert in all C modes, including gnu89. So
> >> add a simple wrapper for that.
> >>
> >> _Static_assert() requires a message argument, which is usually quite
> >> redundant (and I believe that bug got fixed at least in newer C++
> >> standards), but we can easily work around that with a little macro
> >> magic, making it optional.
> >>
> >> For example, adding
> >>
> >>   static_assert(sizeof(struct printf_spec) == 8);
> >>
> >> in vsprintf.c and modifying that struct to violate it, one gets
> >>
> >> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
> >>  #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >>
> >> godbolt.org suggests that _Static_assert() has been support by clang
> >> since at least 3.0.0.
> >>
> >> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> >> ---
> >>  include/linux/build_bug.h | 19 +++++++++++++++++++
> >>  1 file changed, 19 insertions(+)
> >>
> >> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
> >> index faeec7433aab..4bf9ba847b44 100644
> >> --- a/include/linux/build_bug.h
> >> +++ b/include/linux/build_bug.h
> >> @@ -58,4 +58,23 @@
> >>   */
> >>  #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
> >>
> >> +/**
> >> + * static_assert - check integer constant expression at build time
> >> + *
> >> + * static_assert() is a wrapper for the C11 _Static_assert, with a
> >> + * little macro magic to make the message optional (defaulting to the
> >> + * stringification of the tested expression).
> >> + *
> >> + * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
> >> + * scope, but requires the expression to be an integer constant
> >> + * expression (i.e., it is not enough that __builtin_constant_p() is
> >> + * true for expr).
> >> + *
> >> + * Also note that BUILD_BUG_ON() fails the build if the condition is
> >> + * true, while static_assert() fails the build if the expression is
> >> + * false.
> >> + */
> >> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> >> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >
> > What is the "" "" for?
>
> Good point. It's a leftover from when I had a fallback-implementation of
> _Static_assert for gcc < 4.6, where I wanted to ensure that the second
> argument was a string literal, even if my fallback implementation
> ignored that argument. Now it's actually a little harmful, because
>
> foobar.c:5:34: error: expected string literal before ‘expected’
>  static_assert(sizeof(long) == 8, expected 64 bit machine);
>
> is better than
>
> foobar.c:4:34: error: expected ‘)’ before ‘expected’
>  static_assert(sizeof(long) == 8, expected 64 bit machine);
>
> > Bikeshed:
> >
> > There might be room for argument about
> > where this macro should go.
> >
> > Another possible place is <linux/compiler.h>
> > where compiletime_assert() is defined.
>
> I'd rather move compiletime_assert to build_bug.h, and rename it so that
> it becomes an implementation detail of BUILD_BUG. There are not that
> many direct users of compiletime_assert(), and I think we should
> standardize on fewer ways of achieving the same thing. static_assert()
> for checking ICEs, usable at any scope, and BUILD_BUG_* for checking
> that the optimizer is sufficiently smart.
>
> This would also be a step towards another cleanup I'd like to do: make
> build_bug.h not depend on compiler.h,


Probably, you cannot do this.
compiletime_assert relies on __attribute__((__error__(...)))
that is only supported by GCC.


> because we already have a
> dependency in the other direction (ARRAY_SIZE using BUILD_BUG_ON_ZERO).
>
> Rasmus



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-05  9:53     ` Rasmus Villemoes
@ 2019-02-05 20:27       ` Luc Van Oostenryck
  0 siblings, 0 replies; 12+ messages in thread
From: Luc Van Oostenryck @ 2019-02-05 20:27 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Andrew Morton, Kees Cook, Nick Desaulniers, Masahiro Yamada,
	linux-kernel

On Tue, Feb 05, 2019 at 10:53:31AM +0100, Rasmus Villemoes wrote:
> On 05/02/2019 00.12, Andrew Morton wrote:
> >>
> >> It would be (very) nice to actually use this macro in a few places so
> >> it gets its build testing while in -next.
> > 
> > ie, just about every BUILD_BUG_ON in mm/ could use this.  Let's switch
> > a few?
> > 
> 
> Perhaps, but some make sense where they are, e.g. when iterating over
> two arrays using one iterator variable, it's fine to assert they have
> the same size just above the loop. And, unfortunately, static_assert()
> is not quite a drop-in replacement for BUILD_BUG_ON even when the
> argument is an ICE due to -Wdeclaration-after-statement.

Yes, unfortunately.

-- Luc

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

* Re: [PATCH] build_bug.h: add wrapper for _Static_assert
  2019-02-05  9:38   ` Rasmus Villemoes
  2019-02-05 14:06     ` Masahiro Yamada
@ 2019-02-07  0:14     ` Nick Desaulniers
  1 sibling, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-02-07  0:14 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Masahiro Yamada, Kees Cook, Andrew Morton, Luc Van Oostenryck,
	Linux Kernel Mailing List

On Tue, Feb 5, 2019 at 1:38 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 05/02/2019 09.05, Masahiro Yamada wrote:
> > On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
> > <linux@rasmusvillemoes.dk> wrote:
> >> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> >> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >
> > What is the "" "" for?
>
> Good point. It's a leftover from when I had a fallback-implementation of
> _Static_assert for gcc < 4.6, where I wanted to ensure that the second
> argument was a string literal, even if my fallback implementation
> ignored that argument. Now it's actually a little harmful, because

I had assumed it was for the "optional" part of the error message,
since whether you pass a string error message or not, the C
preprocessor should join them all together?

>
> foobar.c:5:34: error: expected string literal before ‘expected’
>  static_assert(sizeof(long) == 8, expected 64 bit machine);

Hopefully you'd put `expected` in double quotes?

Note: I'm _very_ happy to see this being added.  Once it's landed, I
too can think of some places that it would work better than
BUILD_BUG_ON().
-- 
Thanks,
~Nick Desaulniers

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

* [PATCH v2 1/3] build_bug.h: add wrapper for _Static_assert
  2019-02-03 19:24 [PATCH] build_bug.h: add wrapper for _Static_assert Rasmus Villemoes
  2019-02-04 23:09 ` Andrew Morton
  2019-02-05  8:05 ` Masahiro Yamada
@ 2019-02-08 20:30 ` Rasmus Villemoes
  2019-02-08 20:30   ` [PATCH v2 2/3] lib/vsprintf.c: move sizeof(struct printf_spec) next to its definition Rasmus Villemoes
  2019-02-08 20:30   ` [PATCH v2 3/3] linux/fs.h: move member alignment check next to definition of struct filename Rasmus Villemoes
  2 siblings, 2 replies; 12+ messages in thread
From: Rasmus Villemoes @ 2019-02-08 20:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Masahiro Yamada, Nick Desaulniers, Kees Cook, Luc Van Oostenryck,
	linux-kernel, Rasmus Villemoes, Alexander Viro

BUILD_BUG_ON() is a little annoying, since it cannot be used outside
function scope. So one cannot put assertions about the sizeof() a
struct next to the struct definition, but has to hide that in some
more or less arbitrary function.

Since gcc 4.6 (which is now also the required minimum), there is
support for the C11 _Static_assert in all C modes, including gnu89. So
add a simple wrapper for that.

_Static_assert() requires a message argument, which is usually quite
redundant (and I believe that bug got fixed at least in newer C++
standards), but we can easily work around that with a little macro
magic, making it optional.

For example, adding

  static_assert(sizeof(struct printf_spec) == 8);

in vsprintf.c and modifying that struct to violate it, one gets

./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
 #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")

godbolt.org suggests that _Static_assert() has been support by clang
since at least 3.0.0.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2: drop useless "" "" wrapping

 include/linux/build_bug.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index faeec7433aab..0fe5426f2bdc 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -58,4 +58,23 @@
  */
 #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
 
+/**
+ * static_assert - check integer constant expression at build time
+ *
+ * static_assert() is a wrapper for the C11 _Static_assert, with a
+ * little macro magic to make the message optional (defaulting to the
+ * stringification of the tested expression).
+ *
+ * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
+ * scope, but requires the expression to be an integer constant
+ * expression (i.e., it is not enough that __builtin_constant_p() is
+ * true for expr).
+ *
+ * Also note that BUILD_BUG_ON() fails the build if the condition is
+ * true, while static_assert() fails the build if the expression is
+ * false.
+ */
+#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
+#define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
+
 #endif	/* _LINUX_BUILD_BUG_H */
-- 
2.20.1


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

* [PATCH v2 2/3] lib/vsprintf.c: move sizeof(struct printf_spec) next to its definition
  2019-02-08 20:30 ` [PATCH v2 1/3] " Rasmus Villemoes
@ 2019-02-08 20:30   ` Rasmus Villemoes
  2019-02-08 20:30   ` [PATCH v2 3/3] linux/fs.h: move member alignment check next to definition of struct filename Rasmus Villemoes
  1 sibling, 0 replies; 12+ messages in thread
From: Rasmus Villemoes @ 2019-02-08 20:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Masahiro Yamada, Nick Desaulniers, Kees Cook, Luc Van Oostenryck,
	linux-kernel, Rasmus Villemoes

At the time of d0484193 (lib/vsprintf.c: expand field_width to 24
bits), there was no compiletime_assert/BUILD_BUG/.... variant that
could be used outside function scope. Now we have static_assert(), so
move the assertion next to the definition instead of hiding it in some
arbitrary function.

Also add the appropriate #include to avoid relying on build_bug.h
being pulled in via some arbitrary chain of includes.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2: added, mostly as an example of use of static_assert(), to check
that the compiler actually groks it.

 lib/vsprintf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3add92329bae..30b00de4f321 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -17,6 +17,7 @@
  */
 
 #include <stdarg.h>
+#include <linux/build_bug.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/module.h>	/* for KSYM_SYMBOL_LEN */
@@ -405,6 +406,8 @@ struct printf_spec {
 	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
 	signed int	precision:16;	/* # of digits/chars */
 } __packed;
+static_assert(sizeof(struct printf_spec) == 8);
+
 #define FIELD_WIDTH_MAX ((1 << 23) - 1)
 #define PRECISION_MAX ((1 << 15) - 1)
 
@@ -422,8 +425,6 @@ char *number(char *buf, char *end, unsigned long long num,
 	int field_width = spec.field_width;
 	int precision = spec.precision;
 
-	BUILD_BUG_ON(sizeof(struct printf_spec) != 8);
-
 	/* locase = 0 or 0x20. ORing digits or letters with 'locase'
 	 * produces same digits or (maybe lowercased) letters */
 	locase = (spec.flags & SMALL);
-- 
2.20.1


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

* [PATCH v2 3/3] linux/fs.h: move member alignment check next to definition of struct filename
  2019-02-08 20:30 ` [PATCH v2 1/3] " Rasmus Villemoes
  2019-02-08 20:30   ` [PATCH v2 2/3] lib/vsprintf.c: move sizeof(struct printf_spec) next to its definition Rasmus Villemoes
@ 2019-02-08 20:30   ` Rasmus Villemoes
  1 sibling, 0 replies; 12+ messages in thread
From: Rasmus Villemoes @ 2019-02-08 20:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Masahiro Yamada, Nick Desaulniers, Kees Cook, Luc Van Oostenryck,
	linux-kernel, Rasmus Villemoes, Alexander Viro

Instead of doing this compile-time check in some slightly arbitrary
user of struct filename, put it next to the definition.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2: added, mostly as an example of use of static_assert(), to check
that the compiler actually groks it. Feel free to NAK it as useless
churn.

 fs/namei.c         | 2 --
 include/linux/fs.h | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 914178cdbe94..d604f6b3bcc3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -39,7 +39,6 @@
 #include <linux/bitops.h>
 #include <linux/init_task.h>
 #include <linux/uaccess.h>
-#include <linux/build_bug.h>
 
 #include "internal.h"
 #include "mount.h"
@@ -131,7 +130,6 @@ getname_flags(const char __user *filename, int flags, int *empty)
 	struct filename *result;
 	char *kname;
 	int len;
-	BUILD_BUG_ON(offsetof(struct filename, iname) % sizeof(long) != 0);
 
 	result = audit_reusename(filename);
 	if (result)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 29d8e2cfed0e..8dce6932e620 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -37,6 +37,8 @@
 #include <linux/uuid.h>
 #include <linux/errseq.h>
 #include <linux/ioprio.h>
+#include <linux/build_bug.h>
+#include <linux/stddef.h>
 
 #include <asm/byteorder.h>
 #include <uapi/linux/fs.h>
@@ -2487,6 +2489,7 @@ struct filename {
 	struct audit_names	*aname;
 	const char		iname[];
 };
+static_assert(offsetof(struct filename, iname) % sizeof(long) == 0);
 
 extern long vfs_truncate(const struct path *, loff_t);
 extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
-- 
2.20.1


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

end of thread, other threads:[~2019-02-08 20:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-03 19:24 [PATCH] build_bug.h: add wrapper for _Static_assert Rasmus Villemoes
2019-02-04 23:09 ` Andrew Morton
2019-02-04 23:12   ` Andrew Morton
2019-02-05  9:53     ` Rasmus Villemoes
2019-02-05 20:27       ` Luc Van Oostenryck
2019-02-05  8:05 ` Masahiro Yamada
2019-02-05  9:38   ` Rasmus Villemoes
2019-02-05 14:06     ` Masahiro Yamada
2019-02-07  0:14     ` Nick Desaulniers
2019-02-08 20:30 ` [PATCH v2 1/3] " Rasmus Villemoes
2019-02-08 20:30   ` [PATCH v2 2/3] lib/vsprintf.c: move sizeof(struct printf_spec) next to its definition Rasmus Villemoes
2019-02-08 20:30   ` [PATCH v2 3/3] linux/fs.h: move member alignment check next to definition of struct filename Rasmus Villemoes

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