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

The chosen ill-formed construct is a negative bitwidth in a
bitfield within a struct containing at least one named member,
which prevents the UB while keeping the semantics of the construct
for any memory layout of the struct (this motivates the
"sizeof(unsigned) * 8" in the definition of the macro).

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in V2:
- Avoid using a VLA as the compile-time assertion
- Do not drop _Static_assert
---
 xen/include/xen/lib.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 67fc7c1d7e..e57d272772 100644
--- 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 ")"); }) - 1U)
 #else
-#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
+#define BUILD_BUG_ON_ZERO(cond) \
+    (sizeof(struct { unsigned u : (cond) ? -1 : sizeof(unsigned) * 8; }) - sizeof(unsigned))
 #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
 #endif
 
-- 
2.34.1

On 21.06.2023 09:58, 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)."

Here and in the title I'm not happy about you referencing undefined
behavior. What we do here is use a well-known compiler extension (and I'm
sure you're aware we do so elsewhere, where it's actually going to remain
as is from all I can tell right now).

> --- 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 ")"); }) - 1U)

To be compatible with whatever odd ABIs new ports may have, maybe better to
AND or multiply with 0? (I also don't think a U suffix is warranted here.)

>  #else
> -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
> +#define BUILD_BUG_ON_ZERO(cond) \
> +    (sizeof(struct { unsigned u : (cond) ? -1 : sizeof(unsigned) * 8; }) - sizeof(unsigned))

What's wrong with just giving the bitfield a name:

#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int _:-!!(cond); })

Jan

On 21/06/23 10:48, Jan Beulich wrote:
> On 21.06.2023 09:58, 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)."
> 
> Here and in the title I'm not happy about you referencing undefined
> behavior. What we do here is use a well-known compiler extension (and I'm
> sure you're aware we do so elsewhere, where it's actually going to remain
> as is from all I can tell right now).
>

Noted, I'll change the commit message.

>> --- 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 ")"); }) - 1U)
> 
> To be compatible with whatever odd ABIs new ports may have, maybe better to
> AND or multiply with 0? (I also don't think a U suffix is warranted here.)

Good idea

> 
>>   #else
>> -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
>> +#define BUILD_BUG_ON_ZERO(cond) \
>> +    (sizeof(struct { unsigned u : (cond) ? -1 : sizeof(unsigned) * 8; }) - sizeof(unsigned))
> 
> What's wrong with just giving the bitfield a name:
> 
> #define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int _:-!!(cond); })
> 

A named bitfield with zero width is not allowed by the standard, which 
is why the fix introduces a constant positive width. I'll send a v3 
shortly addressing your previous remarks, though.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

On 22.06.2023 10:15, Nicola Vetrini wrote:
> On 21/06/23 10:48, Jan Beulich wrote:
>> On 21.06.2023 09:58, Nicola Vetrini wrote:
>>> -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
>>> +#define BUILD_BUG_ON_ZERO(cond) \
>>> +    (sizeof(struct { unsigned u : (cond) ? -1 : sizeof(unsigned) * 8; }) - sizeof(unsigned))
>>
>> What's wrong with just giving the bitfield a name:
>>
>> #define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int _:-!!(cond); })
>>
> 
> A named bitfield with zero width is not allowed by the standard, which 
> is why the fix introduces a constant positive width. I'll send a v3 
> shortly addressing your previous remarks, though.

Oh, right, but easy to overcome, I think: int _:1 - !!(cond);

Jan