Skip to content

Commit 0c32652

Browse files
committed
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: fcba483 ("KVM: selftests: Sanity check input to ioctls() at build time") Fixes: ee37955 ("KVM: selftests: Refactor X86_FEATURE_* framework to prep for X86_PROPERTY_*") Fixes: 53a7dc0 ("KVM: selftests: Add X86_PROPERTY_* framework to retrieve CPUID values") Signed-off-by: Sean Christopherson <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 553d165 commit 0c32652

File tree

2 files changed

+24
-12
lines changed

2 files changed

+24
-12
lines changed

tools/testing/selftests/kvm/include/kvm_util_base.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@
2222

2323
#include "sparsebit.h"
2424

25+
/*
26+
* Provide a version of static_assert() that is guaranteed to have an optional
27+
* message param. If _ISOC11_SOURCE is defined, glibc (/usr/include/assert.h)
28+
* #undefs and #defines static_assert() as a direct alias to _Static_assert(),
29+
* i.e. effectively makes the message mandatory. Many KVM selftests #define
30+
* _GNU_SOURCE for various reasons, and _GNU_SOURCE implies _ISOC11_SOURCE. As
31+
* a result, static_assert() behavior is non-deterministic and may or may not
32+
* require a message depending on #include order.
33+
*/
34+
#define __kvm_static_assert(expr, msg, ...) _Static_assert(expr, msg)
35+
#define kvm_static_assert(expr, ...) __kvm_static_assert(expr, ##__VA_ARGS__, #expr)
36+
2537
#define KVM_DEV_PATH "/dev/kvm"
2638
#define KVM_MAX_VCPUS 512
2739

@@ -196,7 +208,7 @@ static inline bool kvm_has_cap(long cap)
196208

197209
#define kvm_do_ioctl(fd, cmd, arg) \
198210
({ \
199-
static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd), ""); \
211+
kvm_static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd)); \
200212
ioctl(fd, cmd, arg); \
201213
})
202214

tools/testing/selftests/kvm/include/x86_64/processor.h

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,11 @@ struct kvm_x86_cpu_feature {
7272
.bit = __bit, \
7373
}; \
7474
\
75-
static_assert((fn & 0xc0000000) == 0 || \
76-
(fn & 0xc0000000) == 0x40000000 || \
77-
(fn & 0xc0000000) == 0x80000000 || \
78-
(fn & 0xc0000000) == 0xc0000000); \
79-
static_assert(idx < BIT(sizeof(feature.index) * BITS_PER_BYTE)); \
75+
kvm_static_assert((fn & 0xc0000000) == 0 || \
76+
(fn & 0xc0000000) == 0x40000000 || \
77+
(fn & 0xc0000000) == 0x80000000 || \
78+
(fn & 0xc0000000) == 0xc0000000); \
79+
kvm_static_assert(idx < BIT(sizeof(feature.index) * BITS_PER_BYTE)); \
8080
feature; \
8181
})
8282

@@ -191,12 +191,12 @@ struct kvm_x86_cpu_property {
191191
.hi_bit = high_bit, \
192192
}; \
193193
\
194-
static_assert(low_bit < high_bit); \
195-
static_assert((fn & 0xc0000000) == 0 || \
196-
(fn & 0xc0000000) == 0x40000000 || \
197-
(fn & 0xc0000000) == 0x80000000 || \
198-
(fn & 0xc0000000) == 0xc0000000); \
199-
static_assert(idx < BIT(sizeof(property.index) * BITS_PER_BYTE)); \
194+
kvm_static_assert(low_bit < high_bit); \
195+
kvm_static_assert((fn & 0xc0000000) == 0 || \
196+
(fn & 0xc0000000) == 0x40000000 || \
197+
(fn & 0xc0000000) == 0x80000000 || \
198+
(fn & 0xc0000000) == 0xc0000000); \
199+
kvm_static_assert(idx < BIT(sizeof(property.index) * BITS_PER_BYTE)); \
200200
property; \
201201
})
202202

0 commit comments

Comments
 (0)