[Devel] [PATCH vz10 v2] selftests/breakpoints: fix compilation warnings in breakpoint_test

[Vasileios Almpanis]

Reviewed-by: Vasileios Almpanis <vasileios.almpanis at virtuozzo.com>

On 3/24/26 1:14 PM, Konstantin Khorenko wrote:
>    breakpoint_test.c: In function 'read_var':
>    breakpoint_test.c:186:33: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>      186 |                         sval = *(short *)&dummy_var[i];
>          |                                 ^~~~~~~~~~~~~~~~~~~~~~
>    breakpoint_test.c:189:33: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>      189 |                         ival = *(int *)&dummy_var[i];
>          |                                 ^~~~~~~~~~~~~~~~~~~~
>    breakpoint_test.c:177:52: warning: variable 'lval' set but not used [-Wunused-but-set-variable]
>    breakpoint_test.c:177:36: warning: variable 'ival' set but not used [-Wunused-but-set-variable]
>    breakpoint_test.c:177:26: warning: variable 'sval' set but not used [-Wunused-but-set-variable]
>    breakpoint_test.c:177:14: warning: variable 'cval' set but not used [-Wunused-but-set-variable]
>    breakpoint_test.c: In function 'trigger_tests':
>    breakpoint_test.c:206:14: warning: unused variable 'val' [-Wunused-variable]
>    breakpoint_test.c: In function 'launch_tests':
>    breakpoint_test.c:335:33: warning: unused variable 'i' [-Wunused-variable]
>    breakpoint_test.c: In function 'main':
>    breakpoint_test.c:390:13: warning: unused variable 'ret' [-Wunused-variable]
>    breakpoint_test.c: In function 'toggle_breakpoint':
>    breakpoint_test.c:91:22: warning: 'xlen' may be used uninitialized [-Wmaybe-uninitialized]
>       91 |         vdr7 = (xlen | xtype) << 16;
>          |                ~~~~~~^~~~~~~~
>
> Fix the following warnings:
>
> - [-Wstrict-aliasing] in read_var(): suppress with a GCC diagnostic pragma --
>     the type-punned reads are intentional here: we need sized memory accesses
>     at specific addresses to trigger hardware breakpoints.
>
> - [-Wunused-but-set-variable] in read_var(): instead of suppressing the
>     warning with a pragma, remove the unnecessary local variables (cval, sval,
>     ival, lval) entirely and use volatile casts on the pointer dereferences.
>     This is the correct fix because:
>     1. The purpose of these reads is to trigger hardware watchpoints via
>        memory access at specific addresses, not to use the values.
>     2. With -O2 the compiler is free to eliminate a dead store to a
>        non-volatile local, so it can remove the read altogether, which
>        would silently break the test.
>     3. Casting the pointer to volatile forces the compiler to emit the
>        load instruction regardless of optimization level, which is
>        exactly the semantics we need.
>
> - [-Wmaybe-uninitialized]: initialize xlen in toggle_breakpoint().
> - [-Wunused-variable]: remove unused variables val, i, ret.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-127529
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
>
> Feature: fix selftests
> ---
> v2: dropped "unused" pragma and removed unused variables instead.
>      The second pragma to stay here.
> ---
>   .../selftests/breakpoints/breakpoint_test.c   | 24 ++++++++++++-------
>   1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/breakpoints/breakpoint_test.c b/tools/testing/selftests/breakpoints/breakpoint_test.c
> index d46962a24724..6a45fde12c0b 100644
> --- a/tools/testing/selftests/breakpoints/breakpoint_test.c
> +++ b/tools/testing/selftests/breakpoints/breakpoint_test.c
> @@ -55,7 +55,7 @@ static void toggle_breakpoint(int n, int type, int len,
>   {
>   	int ret;
>   
> -	int xtype, xlen;
> +	int xtype, xlen = 0;
>   	unsigned long vdr7, dr7;
>   
>   	switch (type) {
> @@ -172,29 +172,37 @@ static void write_var(int len)
>   	}
>   }
>   
> +/*
> + * Type-punned reads are intentional here: we need sized memory accesses
> + * at specific addresses to trigger hardware breakpoints. Alternatives
> + * like memcpy() do not guarantee a single load of the required width,
> + * so we suppress the strict-aliasing warning with a pragma instead.
> + */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstrict-aliasing"
>   static void read_var(int len)
>   {
> -	char cval; short sval; int ival; long long lval;
>   	int i;
>   
>   	for (i = 0; i < 4; i++) {
>   		switch (len) {
>   		case 1:
> -			cval = *(char *)&dummy_var[i];
> +			*(volatile char *)&dummy_var[i];
>   			break;
>   		case 2:
> -			sval = *(short *)&dummy_var[i];
> +			*(volatile short *)&dummy_var[i];
>   			break;
>   		case 4:
> -			ival = *(int *)&dummy_var[i];
> +			*(volatile int *)&dummy_var[i];
>   			break;
>   		case 8:
> -			lval = *(long long *)&dummy_var[i];
> +			*(volatile long long *)&dummy_var[i];
>   			break;
>   		}
>   		check_trapped();
>   	}
>   }
> +#pragma GCC diagnostic pop
>   
>   /*
>    * Do the r/w/x accesses to trigger the breakpoints. And run
> @@ -203,7 +211,6 @@ static void read_var(int len)
>   static void trigger_tests(void)
>   {
>   	int len, local, global, i;
> -	char val;
>   	int ret;
>   
>   	ret = ptrace(PTRACE_TRACEME, 0, NULL, 0);
> @@ -332,7 +339,7 @@ static void launch_tests(void)
>   {
>   	char buf[1024];
>   	unsigned int tests = 0;
> -	int len, local, global, i;
> +	int len, local, global;
>   
>   	tests += 3 * COUNT_ISN_BPS;
>   	tests += sizeof(long) / 2 * 3 * COUNT_WPS;
> @@ -387,7 +394,6 @@ static void launch_tests(void)
>   int main(int argc, char **argv)
>   {
>   	pid_t pid;
> -	int ret;
>   
>   	ksft_print_header();
>