Re: [PATCH v2 11/13] KVM: selftests: Add wrapper macro to handle and assert on expected SIGBUS

[Ackerley Tng <ackerleytng@google.com>]

Sean Christopherson <seanjc@google.com> writes:

> Extract the guest_memfd test's SIGBUS handling functionality into a common
> TEST_EXPECT_SIGBUS() macro in anticipation of adding more SIGBUS testcases.
> Eating a SIGBUS isn't terrible difficult, but it requires a non-trivial

terrible => terribly

> amount of boilerplate code, and using a macro allows selftests to print
> out the exact action that failed to generate a SIGBUS without the developer
> needing to remember to add a useful error message.
>

Adding TEST_FAIL() after action on behalf of the developer (easily
forgotten/left out) helps ensure that test runs don't falsely pass if
there was no SIGBUS.

> Explicitly mark the SIGBUS handler as "used", as gcc-14 at least likes to
> discard the function before linking.
>

I think you also meant to talk about opportunistically using TEST_FAIL()
instead of TEST_ASSERT(false, ...) here.

> Suggested-by: Ackerley Tng <ackerleytng@google.com>

Not sure if these still apply after the Suggested-by tag, but anyway:

Reviewed-by: Ackerley Tng <ackerleytng@google.com>
Tested-by: Ackerley Tng <ackerleytng@google.com>

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../testing/selftests/kvm/guest_memfd_test.c  | 18 +-----------------
>  .../testing/selftests/kvm/include/test_util.h | 19 +++++++++++++++++++
>  tools/testing/selftests/kvm/lib/test_util.c   |  7 +++++++
>  3 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 640636c76eb9..73c2e54e7297 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -14,8 +14,6 @@
>  #include <linux/bitmap.h>
>  #include <linux/falloc.h>
>  #include <linux/sizes.h>
> -#include <setjmp.h>
> -#include <signal.h>
>  #include <sys/mman.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> @@ -77,17 +75,8 @@ static void test_mmap_supported(int fd, size_t total_size)
>  	kvm_munmap(mem, total_size);
>  }
>  
> -static sigjmp_buf jmpbuf;
> -void fault_sigbus_handler(int signum)
> -{
> -	siglongjmp(jmpbuf, 1);
> -}
> -
>  static void test_fault_overflow(int fd, size_t total_size)
>  {
> -	struct sigaction sa_old, sa_new = {
> -		.sa_handler = fault_sigbus_handler,
> -	};
>  	size_t map_size = total_size * 4;
>  	const char val = 0xaa;
>  	char *mem;
> @@ -95,12 +84,7 @@ static void test_fault_overflow(int fd, size_t total_size)
>  
>  	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>  
> -	sigaction(SIGBUS, &sa_new, &sa_old);
> -	if (sigsetjmp(jmpbuf, 1) == 0) {
> -		memset(mem, 0xaa, map_size);
> -		TEST_ASSERT(false, "memset() should have triggered SIGBUS.");
> -	}
> -	sigaction(SIGBUS, &sa_old, NULL);
> +	TEST_EXPECT_SIGBUS(memset(mem, val, map_size));
>  
>  	for (i = 0; i < total_size; i++)
>  		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index c6ef895fbd9a..b4872ba8ed12 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -8,6 +8,8 @@
>  #ifndef SELFTEST_KVM_TEST_UTIL_H
>  #define SELFTEST_KVM_TEST_UTIL_H
>  
> +#include <setjmp.h>
> +#include <signal.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
>  #include <stdbool.h>
> @@ -78,6 +80,23 @@ do {									\
>  	__builtin_unreachable(); \
>  } while (0)
>  
> +extern sigjmp_buf expect_sigbus_jmpbuf;
> +void expect_sigbus_handler(int signum);
> +
> +#define TEST_EXPECT_SIGBUS(action)						\
> +do {										\
> +	struct sigaction sa_old, sa_new = {					\
> +		.sa_handler = expect_sigbus_handler,				\
> +	};									\
> +										\
> +	sigaction(SIGBUS, &sa_new, &sa_old);					\
> +	if (sigsetjmp(expect_sigbus_jmpbuf, 1) == 0) {				\
> +		action;								\
> +		TEST_FAIL("'%s' should have triggered SIGBUS", #action);	\
> +	}									\
> +	sigaction(SIGBUS, &sa_old, NULL);					\
> +} while (0)
> +
>  size_t parse_size(const char *size);
>  
>  int64_t timespec_to_ns(struct timespec ts);
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index 03eb99af9b8d..8a1848586a85 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -18,6 +18,13 @@
>  
>  #include "test_util.h"
>  
> +sigjmp_buf expect_sigbus_jmpbuf;
> +
> +void __attribute__((used)) expect_sigbus_handler(int signum)
> +{
> +	siglongjmp(expect_sigbus_jmpbuf, 1);
> +}
> +
>  /*
>   * Random number generator that is usable from guest code. This is the
>   * Park-Miller LCG using standard constants.
> -- 
> 2.51.0.618.g983fd99d29-goog