OSDN Git Service

KVM: selftests: Define and use a custom static assert in lib headers
authorSean Christopherson <seanjc@google.com>
Tue, 22 Nov 2022 01:33:09 +0000 (01:33 +0000)
committerSean Christopherson <seanjc@google.com>
Thu, 1 Dec 2022 23:31:46 +0000 (15:31 -0800)
Define and use kvm_static_assert() in the common KVM selftests headers to
provide deterministic behavior, and to allow creating static asserts
without dummy messages.

The kernel's static_assert() makes the message param optional, and on the
surface, tools/include/linux/build_bug.h appears to follow suit.  However,
glibc may override static_assert() and redefine it as a direct alias of
_Static_assert(), which makes the message parameter mandatory.  This leads
to non-deterministic behavior as KVM selftests code that utilizes
static_assert() without a custom message may or not compile depending on
the order of includes.  E.g. recently added asserts in
x86_64/processor.h fail on some systems with errors like

  In file included from lib/memstress.c:11:0:
  include/x86_64/processor.h: In function ‘this_cpu_has_p’:
  include/x86_64/processor.h:193:34: error: expected ‘,’ before ‘)’ token
    static_assert(low_bit < high_bit);     \
                                    ^
due to _Static_assert() expecting a comma before a message.  The "message
optional" version of static_assert() uses macro magic to strip away the
comma when presented with empty an __VA_ARGS__

  #ifndef static_assert
  #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
  #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
  #endif // static_assert

and effectively generates "_Static_assert(expr, #expr)".

The incompatible version of static_assert() gets defined by this snippet
in /usr/include/assert.h:

  #if defined __USE_ISOC11 && !defined __cplusplus
  # undef static_assert
  # define static_assert _Static_assert
  #endif

which yields "_Static_assert(expr)" and thus fails as above.

KVM selftests don't actually care about using C11, but __USE_ISOC11 gets
defined because of _GNU_SOURCE, which many tests do #define.  _GNU_SOURCE
triggers a massive pile of defines in /usr/include/features.h, including
_ISOC11_SOURCE:

  /* If _GNU_SOURCE was defined by the user, turn on all the other features.  */
  #ifdef _GNU_SOURCE
  # undef  _ISOC95_SOURCE
  # define _ISOC95_SOURCE 1
  # undef  _ISOC99_SOURCE
  # define _ISOC99_SOURCE 1
  # undef  _ISOC11_SOURCE
  # define _ISOC11_SOURCE 1
  # undef  _POSIX_SOURCE
  # define _POSIX_SOURCE  1
  # undef  _POSIX_C_SOURCE
  # define _POSIX_C_SOURCE        200809L
  # undef  _XOPEN_SOURCE
  # define _XOPEN_SOURCE  700
  # undef  _XOPEN_SOURCE_EXTENDED
  # define _XOPEN_SOURCE_EXTENDED 1
  # undef  _LARGEFILE64_SOURCE
  # define _LARGEFILE64_SOURCE    1
  # undef  _DEFAULT_SOURCE
  # define _DEFAULT_SOURCE        1
  # undef  _ATFILE_SOURCE
  # define _ATFILE_SOURCE 1
  #endif

which further down in /usr/include/features.h leads to:

  /* This is to enable the ISO C11 extension.  */
  #if (defined _ISOC11_SOURCE \
       || (defined __STDC_VERSION__ && __STDC_VERSION__ >= 201112L))
  # define __USE_ISOC11   1
  #endif

To make matters worse, /usr/include/assert.h doesn't guard against
multiple inclusion by turning itself into a nop, but instead #undefs a
few macros and continues on.  As a result, it's all but impossible to
ensure the "message optional" version of static_assert() will actually be
used, e.g. explicitly including assert.h and #undef'ing static_assert()
doesn't work as a later inclusion of assert.h will again redefine its
version.

  #ifdef  _ASSERT_H

  # undef _ASSERT_H
  # undef assert
  # undef __ASSERT_VOID_CAST

  # ifdef __USE_GNU
  #  undef assert_perror
  # endif

  #endif /* assert.h      */

  #define _ASSERT_H       1
  #include <features.h>

Fixes: fcba483e8246 ("KVM: selftests: Sanity check input to ioctls() at build time")
Fixes: ee3795536664 ("KVM: selftests: Refactor X86_FEATURE_* framework to prep for X86_PROPERTY_*")
Fixes: 53a7dc0f215e ("KVM: selftests: Add X86_PROPERTY_* framework to retrieve CPUID values")
Signed-off-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/r/20221122013309.1872347-1-seanjc@google.com
tools/testing/selftests/kvm/include/kvm_util_base.h
tools/testing/selftests/kvm/include/x86_64/processor.h

index c7685c7..9fa0d34 100644 (file)
 
 #include "sparsebit.h"
 
+/*
+ * Provide a version of static_assert() that is guaranteed to have an optional
+ * message param.  If _ISOC11_SOURCE is defined, glibc (/usr/include/assert.h)
+ * #undefs and #defines static_assert() as a direct alias to _Static_assert(),
+ * i.e. effectively makes the message mandatory.  Many KVM selftests #define
+ * _GNU_SOURCE for various reasons, and _GNU_SOURCE implies _ISOC11_SOURCE.  As
+ * a result, static_assert() behavior is non-deterministic and may or may not
+ * require a message depending on #include order.
+ */
+#define __kvm_static_assert(expr, msg, ...) _Static_assert(expr, msg)
+#define kvm_static_assert(expr, ...) __kvm_static_assert(expr, ##__VA_ARGS__, #expr)
+
 #define KVM_DEV_PATH "/dev/kvm"
 #define KVM_MAX_VCPUS 512
 
@@ -196,7 +208,7 @@ static inline bool kvm_has_cap(long cap)
 
 #define kvm_do_ioctl(fd, cmd, arg)                                             \
 ({                                                                             \
-       static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd), "");   \
+       kvm_static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd));   \
        ioctl(fd, cmd, arg);                                                    \
 })
 
index 22852bd..411549e 100644 (file)
@@ -72,11 +72,11 @@ struct kvm_x86_cpu_feature {
                .bit = __bit,                                                   \
        };                                                                      \
                                                                                \
-       static_assert((fn & 0xc0000000) == 0 ||                                 \
-                     (fn & 0xc0000000) == 0x40000000 ||                        \
-                     (fn & 0xc0000000) == 0x80000000 ||                        \
-                     (fn & 0xc0000000) == 0xc0000000);                         \
-       static_assert(idx < BIT(sizeof(feature.index) * BITS_PER_BYTE));        \
+       kvm_static_assert((fn & 0xc0000000) == 0 ||                             \
+                         (fn & 0xc0000000) == 0x40000000 ||                    \
+                         (fn & 0xc0000000) == 0x80000000 ||                    \
+                         (fn & 0xc0000000) == 0xc0000000);                     \
+       kvm_static_assert(idx < BIT(sizeof(feature.index) * BITS_PER_BYTE));    \
        feature;                                                                \
 })
 
@@ -191,12 +191,12 @@ struct kvm_x86_cpu_property {
                .hi_bit = high_bit,                                             \
        };                                                                      \
                                                                                \
-       static_assert(low_bit < high_bit);                                      \
-       static_assert((fn & 0xc0000000) == 0 ||                                 \
-                     (fn & 0xc0000000) == 0x40000000 ||                        \
-                     (fn & 0xc0000000) == 0x80000000 ||                        \
-                     (fn & 0xc0000000) == 0xc0000000);                         \
-       static_assert(idx < BIT(sizeof(property.index) * BITS_PER_BYTE));       \
+       kvm_static_assert(low_bit < high_bit);                                  \
+       kvm_static_assert((fn & 0xc0000000) == 0 ||                             \
+                         (fn & 0xc0000000) == 0x40000000 ||                    \
+                         (fn & 0xc0000000) == 0x80000000 ||                    \
+                         (fn & 0xc0000000) == 0xc0000000);                     \
+       kvm_static_assert(idx < BIT(sizeof(property.index) * BITS_PER_BYTE));   \
        property;                                                               \
 })