Commit 0c326523 authored by Sean Christopherson's avatar Sean Christopherson

KVM: selftests: Define and use a custom static assert in lib headers

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: fcba483e ("KVM: selftests: Sanity check input to ioctls() at build time")
Fixes: ee379553 ("KVM: selftests: Refactor X86_FEATURE_* framework to prep for X86_PROPERTY_*")
Fixes: 53a7dc0f ("KVM: selftests: Add X86_PROPERTY_* framework to retrieve CPUID values")
Signed-off-by: default avatarSean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/r/20221122013309.1872347-1-seanjc@google.com
parent 553d1652
......@@ -22,6 +22,18 @@
#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); \
})
......
......@@ -72,11 +72,11 @@ struct kvm_x86_cpu_feature {
.bit = __bit, \
}; \
\
static_assert((fn & 0xc0000000) == 0 || \
kvm_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(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 || \
kvm_static_assert(low_bit < high_bit); \
kvm_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(idx < BIT(sizeof(property.index) * BITS_PER_BYTE)); \
property; \
})
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment