Fixes and cleanups for various issues in the vDSO selftests.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- Changes in v2: - Refer to -Wstrict-prototypes over -Wold-style-prototypes - Pick up Acks - Enable fixed warnings in Makefile - Link to v1: https://lore.kernel.org/r/20250502-selftests-vdso-fixes-v1-0-fb5d640a4f78@li...
--- Thomas Weißschuh (8): selftests: vDSO: chacha: Correctly skip test if necessary selftests: vDSO: clock_getres: Drop unused include of err.h selftests: vDSO: vdso_test_getrandom: Drop unused include of linux/compiler.h selftests: vDSO: vdso_test_getrandom: Drop some dead code selftests: vDSO: vdso_config: Avoid -Wunused-variables selftests: vDSO: enable -Wall selftests: vDSO: vdso_test_correctness: Fix -Wstrict-prototypes selftests: vDSO: vdso_test_getrandom: Always print TAP header
tools/testing/selftests/vDSO/Makefile | 2 +- tools/testing/selftests/vDSO/vdso_config.h | 2 ++ tools/testing/selftests/vDSO/vdso_test_chacha.c | 3 ++- tools/testing/selftests/vDSO/vdso_test_clock_getres.c | 1 - tools/testing/selftests/vDSO/vdso_test_correctness.c | 2 +- tools/testing/selftests/vDSO/vdso_test_getrandom.c | 18 +++++------------- 6 files changed, 11 insertions(+), 17 deletions(-) --- base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8 change-id: 20250423-selftests-vdso-fixes-d2ce74142359
Best regards,
According to kselftest.h ksft_exit_skip() is not meant to be called when a plan has already been printed.
Use the recommended function ksft_test_result_skip().
This fixes a bug, where the TAP output would be invalid when skipping:
TAP version 13 1..1 ok 2 # SKIP Not implemented on architecture
The SKIP line should start with "ok 1" as the plan only contains one test.
Fixes: 3b5992eaf730 ("selftests: vDSO: unconditionally build chacha test") Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- I'm not sure if this is not a general bug in ksft_exit_skip(). First ksft_xskip is incremented then read back through ksft_test_num() and then that result is incremented again.
In any case, using the correct function is better. --- tools/testing/selftests/vDSO/vdso_test_chacha.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c index 8757f738b0b1a76a48c83c5e5df79925a30c1bc7..0aad682b12c8836efabb49a65a47cf87466891a3 100644 --- a/tools/testing/selftests/vDSO/vdso_test_chacha.c +++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c @@ -76,7 +76,8 @@ static void reference_chacha20_blocks(uint8_t *dst_bytes, const uint32_t *key, u
void __weak __arch_chacha20_blocks_nostack(uint8_t *dst_bytes, const uint32_t *key, uint32_t *counter, size_t nblocks) { - ksft_exit_skip("Not implemented on architecture\n"); + ksft_test_result_skip("Not implemented on architecture\n"); + ksft_finished(); }
int main(int argc, char *argv[])
Nothing from err.h is used.
Drop the include.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/vDSO/vdso_test_clock_getres.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c index 38d46a8bf7cba7a9b4a9b13b5eb17aa207972bd0..b5d5f59f725a703c357dfca91bfe170aaaeb42fa 100644 --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c @@ -13,7 +13,6 @@
#define _GNU_SOURCE #include <elf.h> -#include <err.h> #include <fcntl.h> #include <stdint.h> #include <stdio.h>
The header is unused. Furthermore this is not a real UAPI header, but only exists in tools/include/. This prevents building the selftest against real UAPI headers.
Drop the include.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/vdso_test_getrandom.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index 95057f7567db22226d9cb09a667a56e387a33a46..f36e50f372f935e6d4da3175c81e210653bdce1d 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -21,7 +21,6 @@ #include <sys/wait.h> #include <sys/types.h> #include <linux/random.h> -#include <linux/compiler.h> #include <linux/ptrace.h>
#include "../kselftest.h"
vgetrandom_put_state() and the variable ret in kselftest() are never used.
Drop the dead code.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/vDSO/vdso_test_getrandom.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index f36e50f372f935e6d4da3175c81e210653bdce1d..b0e0d664508a38d6dde9df0a61ec8198ee928a17 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -100,15 +100,6 @@ static void *vgetrandom_get_state(void) return state; }
-static void vgetrandom_put_state(void *state) -{ - if (!state) - return; - pthread_mutex_lock(&vgrnd.lock); - vgrnd.states[vgrnd.len++] = state; - pthread_mutex_unlock(&vgrnd.lock); -} - static void vgetrandom_init(void) { const char *version = versions[VDSO_VERSION]; @@ -264,7 +255,7 @@ static void kselftest(void) } for (;;) { struct ptrace_syscall_info info = { 0 }; - int status, ret; + int status; ksft_assert(waitpid(child, &status, 0) >= 0); if (WIFEXITED(status)) { ksft_assert(WEXITSTATUS(status) == 0);
Hi Thomas,
On Mon, May 5, 2025 at 11:19 AM Thomas Weißschuh thomas.weissschuh@linutronix.de wrote:
vgetrandom_put_state() and the variable ret in kselftest() are never used.
Drop the dead code.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/vDSO/vdso_test_getrandom.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index f36e50f372f935e6d4da3175c81e210653bdce1d..b0e0d664508a38d6dde9df0a61ec8198ee928a17 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -100,15 +100,6 @@ static void *vgetrandom_get_state(void) return state; }
-static void vgetrandom_put_state(void *state) -{
if (!state)return;pthread_mutex_lock(&vgrnd.lock);vgrnd.states[vgrnd.len++] = state;pthread_mutex_unlock(&vgrnd.lock);-}
This sort of acts as example code / basic reference code for libcs and such. So I like having this function around. Could you just mark it as unused with an attribute but otherwise keep it?
Jason
On Mon, May 05, 2025 at 02:58:06PM +0200, Jason A. Donenfeld wrote:
Hi Thomas,
On Mon, May 5, 2025 at 11:19 AM Thomas Weißschuh thomas.weissschuh@linutronix.de wrote:
vgetrandom_put_state() and the variable ret in kselftest() are never used.
Drop the dead code.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/vDSO/vdso_test_getrandom.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index f36e50f372f935e6d4da3175c81e210653bdce1d..b0e0d664508a38d6dde9df0a61ec8198ee928a17 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -100,15 +100,6 @@ static void *vgetrandom_get_state(void) return state; }
-static void vgetrandom_put_state(void *state) -{
if (!state)return;pthread_mutex_lock(&vgrnd.lock);vgrnd.states[vgrnd.len++] = state;pthread_mutex_unlock(&vgrnd.lock);-}
This sort of acts as example code / basic reference code for libcs and such. So I like having this function around. Could you just mark it as unused with an attribute but otherwise keep it?
Sure, I'll do so.
If it is meant as an example, maybe the test could be extended to show how to use it?
Thomas
Not all users of this header make use of all its variables. For example vdso_test_correctness.c does not use "versions":
In file included from vdso_test_correctness.c:22: vdso_config.h:61:20: warning: ‘versions’ defined but not used [-Wunused-variable] 61 | static const char *versions[7] = { | ^~~~~~~~
Avoid those warnings through attribute((unused)).
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/vDSO/vdso_config.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/vDSO/vdso_config.h b/tools/testing/selftests/vDSO/vdso_config.h index 722260f9756198956f0dfccced907284b6851e76..5fdd0f36233742bc47ae79f23d2cfae5a0f56dee 100644 --- a/tools/testing/selftests/vDSO/vdso_config.h +++ b/tools/testing/selftests/vDSO/vdso_config.h @@ -58,6 +58,7 @@ #define VDSO_NAMES 1 #endif
+__attribute__((unused)) static const char *versions[7] = { "LINUX_2.6", "LINUX_2.6.15", @@ -68,6 +69,7 @@ static const char *versions[7] = { "LINUX_5.10" };
+__attribute__((unused)) static const char *names[2][7] = { { "__kernel_gettimeofday",
Protect against common programming errors through compiler warnings. These warnings are also used for the kernel itself.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 12a0614b9fd4983deffe5d6a7cfa06ba8d92a516..06d72254ec75dbdcc2b20935534199fabc40a9de 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -12,7 +12,7 @@ TEST_GEN_PROGS += vdso_test_correctness TEST_GEN_PROGS += vdso_test_getrandom TEST_GEN_PROGS += vdso_test_chacha
-CFLAGS := -std=gnu99 -O2 +CFLAGS := -std=gnu99 -O2 -Wall
ifeq ($(CONFIG_X86_32),y) LDLIBS += -lgcc_s
Functions definitions without any argument list produce a warning with -Wstrict-prototypes:
vdso_test_correctness.c:111:13: warning: function declaration isn’t a prototype [-Wstrict-prototypes] 111 | static void fill_function_pointers() | ^~~~~~~~~~~~~~~~~~~~~~
Explicitly use an empty argument list.
Now that all selftests a free of this warning, enable it in the Makefile.
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- tools/testing/selftests/vDSO/Makefile | 2 +- tools/testing/selftests/vDSO/vdso_test_correctness.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 06d72254ec75dbdcc2b20935534199fabc40a9de..918a2caa070ebc681a9525f0518afffcf10f5ae3 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -12,7 +12,7 @@ TEST_GEN_PROGS += vdso_test_correctness TEST_GEN_PROGS += vdso_test_getrandom TEST_GEN_PROGS += vdso_test_chacha
-CFLAGS := -std=gnu99 -O2 -Wall +CFLAGS := -std=gnu99 -O2 -Wall -Wstrict-prototypes
ifeq ($(CONFIG_X86_32),y) LDLIBS += -lgcc_s diff --git a/tools/testing/selftests/vDSO/vdso_test_correctness.c b/tools/testing/selftests/vDSO/vdso_test_correctness.c index 5fb97ad67eeaf17b6cfa4f82783c57894f03e5c5..da651cf53c6ca4242085de109c7fc57bd807297c 100644 --- a/tools/testing/selftests/vDSO/vdso_test_correctness.c +++ b/tools/testing/selftests/vDSO/vdso_test_correctness.c @@ -108,7 +108,7 @@ static void *vsyscall_getcpu(void) }
-static void fill_function_pointers() +static void fill_function_pointers(void) { void *vdso = dlopen("linux-vdso.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
The TAP specification requires that the output begins with a header line. If vgetrandom_init() fails and skips the test, that header line is missing.
Call vgetrandom_init() after ksft_print_header().
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/vDSO/vdso_test_getrandom.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c index b0e0d664508a38d6dde9df0a61ec8198ee928a17..01892d8e65d754d0353f7df2b63910d5be8cd1bc 100644 --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c @@ -232,6 +232,7 @@ static void kselftest(void) pid_t child;
ksft_print_header(); + vgetrandom_init(); ksft_set_plan(2);
for (size_t i = 0; i < 1000; ++i) { @@ -285,8 +286,6 @@ static void usage(const char *argv0)
int main(int argc, char *argv[]) { - vgetrandom_init(); - if (argc == 1) { kselftest(); return 0; @@ -296,6 +295,9 @@ int main(int argc, char *argv[]) usage(argv[0]); return 1; } + + vgetrandom_init(); + if (!strcmp(argv[1], "bench-single")) bench_single(); else if (!strcmp(argv[1], "bench-multi"))
linux-kselftest-mirror@lists.linaro.org
-
Jason A. Donenfeld -
Thomas Weißschuh