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

The choice of the bitwise AND operation to bring the result to 0
when cond is false boils down to possibly better portability,
and the 'U' suffix to make it obvious that this operation results
in an unsigned value.

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
Changes in V3:
- Changed the operation to bring the result to 0 when the
construct does not lead to a compilation error
Changes in V4:
- Switched to a shorter construct for the second definition.
---
 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..460cab6734 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 ")"); }) & 0U)
 #else
-#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
+#define BUILD_BUG_ON_ZERO(cond) \
+    (sizeof(struct { unsigned u : !(cond); }) & 0U)
 #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
 #endif
 
-- 
2.34.1

On 23.06.2023 10:59, 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 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.
> 
> The choice of the bitwise AND operation to bring the result to 0
> when cond is false boils down to possibly better portability,
> and the 'U' suffix to make it obvious that this operation results
> in an unsigned value.
> 
> 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
> Changes in V3:
> - Changed the operation to bring the result to 0 when the
> construct does not lead to a compilation error
> Changes in V4:
> - Switched to a shorter construct for the second definition.

Which sadly renders part of the description inapplicable now (there's
no negative width bitfield anymore, afaics). Could probably be swapped
for "zero" while committing, if some other maintainer wants to ack it
in its present form. I'm not happy to, with the continued use of the
two U suffixes. It may seem minor, but to me it feels like setting a
bad precedent.

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 ")"); }) & 0U)
>  #else
> -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
> +#define BUILD_BUG_ON_ZERO(cond) \
> +    (sizeof(struct { unsigned u : !(cond); }) & 0U)
>  #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>  #endif
>

Hi,

First, one remark about the title. We don't usually add full stop in the 
title. I am happy to fix it on commit.

On 23/06/2023 18:16, Jan Beulich wrote:
> I'm not happy to, with the continued use of the
> two U suffixes. It may seem minor, but to me it feels like setting a
> bad precedent.

I wasn't able to find the reasoning behind your objections in the 
archive. I would like to understand your concern before providing any 
ack. Would you be able to give a pointer?

Cheers,

-- 
Julien Grall

On 24.06.2023 09:11, Julien Grall wrote:
> On 23/06/2023 18:16, Jan Beulich wrote:
>> I'm not happy to, with the continued use of the
>> two U suffixes. It may seem minor, but to me it feels like setting a
>> bad precedent.
> 
> I wasn't able to find the reasoning behind your objections in the 
> archive. I would like to understand your concern before providing any 
> ack. Would you be able to give a pointer?

I appreciate the Misra-invoked desire to add U suffixes where
otherwise (visual) ambiguities may exist. But on numbers like
0 or 1, and when use of e.g. resulting #define-s doesn't require
the constants to be of unsigned type, I view such suffixes purely
as clutter. In the specific case I might go as far as questioning
why, when U is added, L isn't added as well, to "support" the
size_t result aspect also from the "width of type" perspective.

Jan

On 24/06/23 09:32, Jan Beulich wrote:
> On 24.06.2023 09:11, Julien Grall wrote:
>> On 23/06/2023 18:16, Jan Beulich wrote:
>>> I'm not happy to, with the continued use of the
>>> two U suffixes. It may seem minor, but to me it feels like setting a
>>> bad precedent.
>>
>> I wasn't able to find the reasoning behind your objections in the
>> archive. I would like to understand your concern before providing any
>> ack. Would you be able to give a pointer?
> 
> I appreciate the Misra-invoked desire to add U suffixes where
> otherwise (visual) ambiguities may exist. But on numbers like
> 0 or 1, and when use of e.g. resulting #define-s doesn't require
> the constants to be of unsigned type, I view such suffixes purely
> as clutter. In the specific case I might go as far as questioning
> why, when U is added, L isn't added as well, to "support" the
> size_t result aspect also from the "width of type" perspective.
> 
> Jan

I will purge the 'U's in a v5 (and correct the wrong commit message as 
well), since they are not necessary for this patch.

Regards,

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