[XEN PATCH] xen/include/xen/lib.h: avoid undefined behavior.

Nicola Vetrini posted 1 patch 10 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/e8e56dc4e45ec79f3e5380544b56c3e0b3b2bcb9.1686824437.git.nicola.vetrini@bugseng.com
xen/include/xen/lib.h | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
[XEN PATCH] xen/include/xen/lib.h: avoid undefined behavior.
Posted by Nicola Vetrini 10 months, 2 weeks ago
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
 
 #ifndef NDEBUG
 #define ASSERT(p) \
-- 
2.34.1
Re: [XEN PATCH] xen/include/xen/lib.h: avoid undefined behavior.
Posted by Jan Beulich 10 months, 2 weeks ago
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
Re: [XEN PATCH] xen/include/xen/lib.h: avoid undefined behavior.
Posted by Andrew Cooper 10 months, 2 weeks ago
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