| Message ID | e8e56dc4e45ec79f3e5380544b56c3e0b3b2bcb9.1686824437.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN] xen/include/xen/lib.h: avoid undefined behavior. | expand |
On 15/06/2023 11:30 am, Nicola Vetrini wrote: > Redefine BUILD_BUG_ON_ZERO to fully comply with C99 avoiding > undefined behavior 58 ("A structure or union is defined as > containing no named members (6.7.2.1)." > This also avoids a dependency on the compiler and its version. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > --- > xen/include/xen/lib.h | 16 +--------------- > 1 file changed, 1 insertion(+), 15 deletions(-) > > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h > index 67fc7c1d7e..a266159b9f 100644 > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -40,22 +40,8 @@ > unlikely(ret_warn_on_); \ > }) > > -/* All clang versions supported by Xen have _Static_assert. */ > -#if defined(__clang__) || \ > - (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) > -/* Force a compilation error if condition is true */ > -#define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); }) > - > -/* Force a compilation error if condition is true, but also produce a > - result (of value 0 and type size_t), so the expression can be used > - e.g. in a structure initializer (or where-ever else comma expressions > - aren't permitted). */ > -#define BUILD_BUG_ON_ZERO(cond) \ > - sizeof(struct { _Static_assert(!(cond), "!(" #cond ")"); }) > -#else > -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); }) > +#define BUILD_BUG_ON_ZERO(cond) (sizeof(char[(cond)? -1 : 1]) - 1U) > #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond)) > -#endif Getting rid of memberless structs is fine. Getting rid of _Static_assert() is absolutely not, because this change massively obfuscates build time error messages. The MISRA work can do whatever is necessary to get _Static_assert() permitted for use globally across Xen. ~Andrew
On 15.06.2023 12:30, Nicola Vetrini wrote: > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -40,22 +40,8 @@ > unlikely(ret_warn_on_); \ > }) > > -/* All clang versions supported by Xen have _Static_assert. */ > -#if defined(__clang__) || \ > - (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) > -/* Force a compilation error if condition is true */ > -#define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); }) > - > -/* Force a compilation error if condition is true, but also produce a > - result (of value 0 and type size_t), so the expression can be used > - e.g. in a structure initializer (or where-ever else comma expressions > - aren't permitted). */ > -#define BUILD_BUG_ON_ZERO(cond) \ > - sizeof(struct { _Static_assert(!(cond), "!(" #cond ")"); }) > -#else > -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); }) > +#define BUILD_BUG_ON_ZERO(cond) (sizeof(char[(cond)? -1 : 1]) - 1U) In addition to what Andrew has said, I'd further like to ask that you try to first understand why certain things are the way they are, by looking at history. We did use an array here up to commit c8e857a0aff3, and if you go there you'll find an explanation why it wants to be something else. Jan
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index 67fc7c1d7e..a266159b9f 100644 --- a/xen/include/xen/lib.h +++ b/xen/include/xen/lib.h @@ -40,22 +40,8 @@ unlikely(ret_warn_on_); \ }) -/* All clang versions supported by Xen have _Static_assert. */ -#if defined(__clang__) || \ - (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) -/* Force a compilation error if condition is true */ -#define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); }) - -/* Force a compilation error if condition is true, but also produce a - result (of value 0 and type size_t), so the expression can be used - e.g. in a structure initializer (or where-ever else comma expressions - aren't permitted). */ -#define BUILD_BUG_ON_ZERO(cond) \ - sizeof(struct { _Static_assert(!(cond), "!(" #cond ")"); }) -#else -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); }) +#define BUILD_BUG_ON_ZERO(cond) (sizeof(char[(cond)? -1 : 1]) - 1U) #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond)) -#endif #ifndef NDEBUG #define ASSERT(p) \
Redefine BUILD_BUG_ON_ZERO to fully comply with C99 avoiding undefined behavior 58 ("A structure or union is defined as containing no named members (6.7.2.1)." This also avoids a dependency on the compiler and its version. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/include/xen/lib.h | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)