| Message ID | db883456d7d210e2ea9ac4a7b402dda73c3ea8cd.1687421893.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN,v3] xen/include: avoid using a compiler extension for BUILD_BUG_ON_ZERO. | expand |
On 22.06.2023 10:24, Nicola Vetrini wrote: > --- 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) ? -1 : 1; }) & 0U) Given your remark on named bitfields not being allowed to be zero length (which hopefully holds universally, i.e. isn't subject to compiler extensions), how about #define BUILD_BUG_ON_ZERO(cond) (sizeof(struct { int _:!(cond); }) & 0) ? (Implicitly, as said before, I question the value of the U suffixes here. Even better might be to make sure the expression is explicitly not of unsigned type, to avoid surprises if someone used ~BUILD_BUG_ON_ZERO() somewhere.) Jan
On 22/06/23 10:56, Jan Beulich wrote: > On 22.06.2023 10:24, Nicola Vetrini wrote: >> --- 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) ? -1 : 1; }) & 0U) > > Given your remark on named bitfields not being allowed to be zero length > (which hopefully holds universally, i.e. isn't subject to compiler > extensions), how about > > #define BUILD_BUG_ON_ZERO(cond) (sizeof(struct { int _:!(cond); }) & 0) Well, the reason for the change is to drop any assumption on compiler-specific quirks that may arise (it would be a surprise if this wasn't the case, though) > > ? (Implicitly, as said before, I question the value of the U suffixes here. > Even better might be to make sure the expression is explicitly not of > unsigned type, to avoid surprises if someone used ~BUILD_BUG_ON_ZERO() > somewhere.) > > Jan The documentation for the macro definition states that the expression must have value 0 and type size_t when cond is false. If I understand correctly what you're saying here, then this is not allowed by the documentation. Regards,
On 22.06.2023 11:34, Nicola Vetrini wrote: > > > On 22/06/23 10:56, Jan Beulich wrote: >> On 22.06.2023 10:24, Nicola Vetrini wrote: >>> --- 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) ? -1 : 1; }) & 0U) >> >> Given your remark on named bitfields not being allowed to be zero length >> (which hopefully holds universally, i.e. isn't subject to compiler >> extensions), how about >> >> #define BUILD_BUG_ON_ZERO(cond) (sizeof(struct { int _:!(cond); }) & 0) > > Well, the reason for the change is to drop any assumption on > compiler-specific quirks that may arise (it would be a surprise if this > wasn't the case, though) And the reason for my suggestion was that it's shorter, and typically I view "shorter" as also "easier to read/follow". >> ? (Implicitly, as said before, I question the value of the U suffixes here. >> Even better might be to make sure the expression is explicitly not of >> unsigned type, to avoid surprises if someone used ~BUILD_BUG_ON_ZERO() >> somewhere.) > > The documentation for the macro definition states that the expression > must have value 0 and type size_t when cond is false. If I understand > correctly what you're saying here, then this is not allowed by the > documentation. Well, it's the comment that says this, yes. Question is whether the comment mandates the behavior or merely describes the current implementation. Imo it's the latter ... But anyway, switching the result type is independent of the present goal. Jan
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index 67fc7c1d7e..d5f9c4a18a 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) ? -1 : 1; }) & 0U) #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond)) #endif
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. 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 --- xen/include/xen/lib.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)