Re: [XEN PATCH v6] xen/include: avoid using a compiler extension for BUILD_BUG_ON_ZERO

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.06.2023 17:37, Nicola Vetrini wrote:
> Redefine BUILD_BUG_ON_ZERO to avoid using a compiler extension
> that gives an acceptable semantics to C99 undefined behavior 58
> ("A structure or union is defined as containing no named members
> (6.7.2.1)").
> 
> The first definition includes an additional named field of type
> char.
> 
> The chosen ill-formed construct for the second definition is a struct
> with a named bitfield of width 0 when the condition is true,
> which prevents the UB without using the compiler extension while keeping
> the semantic of the construct.
> 
> The choice of the bitwise AND operation to bring the result to 0
> when cond is false boils down to possibly better portability.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>

The code change looks okay to me now, but I'd like to double check
towards the testing you did with this change: While it is clear that
you will have checked that the tree builds with the adjustments, I
expect that would have been with a compiler supporting _Static_assert.
Did you also check with an older compiler, using the alternative
implementation? Plus did you also check both constructs for actually
triggering when the supplied condition turns out to be true?

Jan

> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -51,9 +51,10 @@
>     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 ")"); })
> +    (sizeof(struct { char c; _Static_assert(!(cond), "!(" #cond ")"); }) & 0)
>  #else
> -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
> +#define BUILD_BUG_ON_ZERO(cond) \
> +    (sizeof(struct { unsigned u : !(cond); }) & 0)
>  #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>  #endif
>