[PATCH v6 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic

Jiawei Zhao posted 2 patches 6 months, 1 week ago
There is a newer version of this series
[PATCH v6 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic
Posted by Jiawei Zhao 6 months, 1 week ago
When using GCC on x86-64 to compile an usdt prog with -O1 or higher
optimization, the compiler will generate SIB addressing mode for global
array and PC-relative addressing mode for global variable,
e.g. "1@-96(%rbp,%rax,8)" and "-1@4+t1(%rip)".

In this patch:
- force -O2 optimization for usdt.test.o to generate SIB addressing usdt
  argument spec.
- change the global variable t1 to a local variable, to avoid compiler
  generating PC-relative addressing mode for it.

Signed-off-by: Jiawei Zhao <phoenix500526@163.com>
---
 tools/testing/selftests/bpf/Makefile          |  8 ++++++++
 tools/testing/selftests/bpf/prog_tests/usdt.c | 18 ++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 910d8d6402ef..4b77d06d5c42 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -759,6 +759,14 @@ TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
 TRUNNER_BPF_CFLAGS :=
 $(eval $(call DEFINE_TEST_RUNNER,test_maps))
 
+# Force usdt.c to use -O2 optimization to generate SIB addressing
+# Only apply on x86 architecture where SIB addressing is relevant
+ifeq ($(ARCH), x86)
+$(OUTPUT)/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
+$(OUTPUT)/cpuv4/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
+$(OUTPUT)/no_alu32/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
+endif
+
 # Define test_verifier test runner.
 # It is much simpler than test_maps/test_progs and sufficiently different from
 # them (e.g., test.h is using completely pattern), that it's worth just
diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
index 495d66414b57..86f354d25aef 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -14,10 +14,15 @@ static volatile int idx = 2;
 static volatile __u64 bla = 0xFEDCBA9876543210ULL;
 static volatile short nums[] = {-1, -2, -3, -4};
 
-static volatile struct {
-	int x;
-	signed char y;
-} t1 = { 1, -127 };
+/*
+ * TODO:  At O2 optimization level, t1's USDT argument spec becomes -1@4+t1(%rip).
+ * Since libbpf doesn't support RIP addressing mode yet, this causes "unrecognized register" errors.
+ * This test will be re-enabled once libbpf supports RIP addressing mode.
+ */
+// static volatile struct {
+//	int x;
+//	signed char y;
+// } t1 = { 1, -127 };
 
 #define SEC(name) __attribute__((section(name), used))
 
@@ -27,6 +32,7 @@ unsigned short test_usdt12_semaphore SEC(".probes");
 
 static void __always_inline trigger_func(int x) {
 	long y = 42;
+	signed char t1 = -127;
 
 	if (test_usdt0_semaphore)
 		STAP_PROBE(test, usdt0);
@@ -36,7 +42,7 @@ static void __always_inline trigger_func(int x) {
 		STAP_PROBE12(test, usdt12,
 			     x, x + 1, y, x + y, 5,
 			     y / 7, bla, &bla, -9, nums[x],
-			     nums[idx], t1.y);
+			     nums[idx], t1);
 	}
 }
 
@@ -106,7 +112,7 @@ static void subtest_basic_usdt(void)
 	ASSERT_EQ(bss->usdt12_args[8], -9, "usdt12_arg9");
 	ASSERT_EQ(bss->usdt12_args[9], nums[1], "usdt12_arg10");
 	ASSERT_EQ(bss->usdt12_args[10], nums[idx], "usdt12_arg11");
-	ASSERT_EQ(bss->usdt12_args[11], t1.y, "usdt12_arg12");
+	ASSERT_EQ(bss->usdt12_args[11], -127, "usdt12_arg12");
 
 	int usdt12_expected_arg_sizes[12] = { 4, 4, 8, 8, 4, 8, 8, 8, 4, 2, 2, 1 };
 
-- 
2.43.0
Re: [PATCH v6 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic
Posted by Yonghong Song 6 months, 1 week ago

On 8/2/25 1:48 AM, Jiawei Zhao wrote:
> When using GCC on x86-64 to compile an usdt prog with -O1 or higher
> optimization, the compiler will generate SIB addressing mode for global
> array and PC-relative addressing mode for global variable,
> e.g. "1@-96(%rbp,%rax,8)" and "-1@4+t1(%rip)".
>
> In this patch:
> - force -O2 optimization for usdt.test.o to generate SIB addressing usdt
>    argument spec.
> - change the global variable t1 to a local variable, to avoid compiler
>    generating PC-relative addressing mode for it.
>
> Signed-off-by: Jiawei Zhao <phoenix500526@163.com>
> ---
>   tools/testing/selftests/bpf/Makefile          |  8 ++++++++
>   tools/testing/selftests/bpf/prog_tests/usdt.c | 18 ++++++++++++------
>   2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 910d8d6402ef..4b77d06d5c42 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -759,6 +759,14 @@ TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
>   TRUNNER_BPF_CFLAGS :=
>   $(eval $(call DEFINE_TEST_RUNNER,test_maps))
>   
> +# Force usdt.c to use -O2 optimization to generate SIB addressing
> +# Only apply on x86 architecture where SIB addressing is relevant
> +ifeq ($(ARCH), x86)
> +$(OUTPUT)/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
> +$(OUTPUT)/cpuv4/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
> +$(OUTPUT)/no_alu32/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
> +endif

This is no good. You should not change from -O0 to -O2. The existing usdt.c
test should be kept. I assume at -O0 level, the compiler probably
won't generate SIB pattern.

You could add another usdt test e.g. usdt_o2.c and force
usdt_o2 is compiled with -O2 optimizations and in usdt_o2 focusing on
SIB probe.

> +
>   # Define test_verifier test runner.
>   # It is much simpler than test_maps/test_progs and sufficiently different from
>   # them (e.g., test.h is using completely pattern), that it's worth just
> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
> index 495d66414b57..86f354d25aef 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
> @@ -14,10 +14,15 @@ static volatile int idx = 2;
>   static volatile __u64 bla = 0xFEDCBA9876543210ULL;
>   static volatile short nums[] = {-1, -2, -3, -4};
>   
> -static volatile struct {
> -	int x;
> -	signed char y;
> -} t1 = { 1, -127 };
> +/*
> + * TODO:  At O2 optimization level, t1's USDT argument spec becomes -1@4+t1(%rip).
> + * Since libbpf doesn't support RIP addressing mode yet, this causes "unrecognized register" errors.
> + * This test will be re-enabled once libbpf supports RIP addressing mode.
> + */
> +// static volatile struct {
> +//	int x;
> +//	signed char y;
> +// } t1 = { 1, -127 };
>   

[...]
Re:Re: [PATCH v6 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic
Posted by 赵佳炜 6 months, 1 week ago

OK, I’ve already added an usdt_o2 test and passed it. 




At 2025-08-06 03:42:22, "Yonghong Song" <yonghong.song@linux.dev> wrote:
>
>
>On 8/2/25 1:48 AM, Jiawei Zhao wrote:
>> When using GCC on x86-64 to compile an usdt prog with -O1 or higher
>> optimization, the compiler will generate SIB addressing mode for global
>> array and PC-relative addressing mode for global variable,
>> e.g. "1@-96(%rbp,%rax,8)" and "-1@4+t1(%rip)".
>>
>> In this patch:
>> - force -O2 optimization for usdt.test.o to generate SIB addressing usdt
>>    argument spec.
>> - change the global variable t1 to a local variable, to avoid compiler
>>    generating PC-relative addressing mode for it.
>>
>> Signed-off-by: Jiawei Zhao <phoenix500526@163.com>
>> ---
>>   tools/testing/selftests/bpf/Makefile          |  8 ++++++++
>>   tools/testing/selftests/bpf/prog_tests/usdt.c | 18 ++++++++++++------
>>   2 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 910d8d6402ef..4b77d06d5c42 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -759,6 +759,14 @@ TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
>>   TRUNNER_BPF_CFLAGS :=
>>   $(eval $(call DEFINE_TEST_RUNNER,test_maps))
>>   
>> +# Force usdt.c to use -O2 optimization to generate SIB addressing
>> +# Only apply on x86 architecture where SIB addressing is relevant
>> +ifeq ($(ARCH), x86)
>> +$(OUTPUT)/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
>> +$(OUTPUT)/cpuv4/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
>> +$(OUTPUT)/no_alu32/usdt.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS))
>> +endif
>
>This is no good. You should not change from -O0 to -O2. The existing usdt.c
>test should be kept. I assume at -O0 level, the compiler probably
>won't generate SIB pattern.
>
>You could add another usdt test e.g. usdt_o2.c and force
>usdt_o2 is compiled with -O2 optimizations and in usdt_o2 focusing on
>SIB probe.
>
>> +
>>   # Define test_verifier test runner.
>>   # It is much simpler than test_maps/test_progs and sufficiently different from
>>   # them (e.g., test.h is using completely pattern), that it's worth just
>> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
>> index 495d66414b57..86f354d25aef 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
>> @@ -14,10 +14,15 @@ static volatile int idx = 2;
>>   static volatile __u64 bla = 0xFEDCBA9876543210ULL;
>>   static volatile short nums[] = {-1, -2, -3, -4};
>>   
>> -static volatile struct {
>> -	int x;
>> -	signed char y;
>> -} t1 = { 1, -127 };
>> +/*
>> + * TODO:  At O2 optimization level, t1's USDT argument spec becomes -1@4+t1(%rip).
>> + * Since libbpf doesn't support RIP addressing mode yet, this causes "unrecognized register" errors.
>> + * This test will be re-enabled once libbpf supports RIP addressing mode.
>> + */
>> +// static volatile struct {
>> +//	int x;
>> +//	signed char y;
>> +// } t1 = { 1, -127 };
>>   
>
>[...]