| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5f3a942c.1b6e.198b1b181ef.Coremail.phoenix500526@163.com>
Date: Sat, 16 Aug 2025 15:04:17 +0800 (CST)
From: 赵佳炜 <phoenix500526@....com>
To: "Jiri Olsa" <olsajiri@...il.com>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
yonghong.song@...ux.dev, bpf@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re:Re: [PATCH bpf-next v10 3/3] selftests/bpf: make usdt_o2
reliably generate SIB USDT arg spec
Hi, Jiri. Sorry for my oversight, the optimized compile condition didn't work properly.
Could you try to replace the `__attribute__((optimize("O2")))` with `#pragma GCC optimize("O2")`
in usdt_o2.c and verify it one more time? I guess it'll help. In fact, the usdt1 argument spec generated
by the `__attribute__((optimize("O2")))` on my machine was `8@...x`, not `8@(%rdx,%rax,8)`.
For more detail:
> - #if defined(__GNUC__) && !defined(__clang__)
> - __attribute__((optimize("O2")))
>
> +#if (defined(__GNUC__) && !defined(__clang__))
> +#pragma GCC optimize("O2")
> +#else
> +#pragma message("non-gcc compiler: the correct probes might not be installed")
> +#endif
Thanks
At 2025-08-15 20:44:32, "Jiri Olsa" <olsajiri@...il.com> wrote:
>On Thu, Aug 14, 2025 at 04:07:39PM +0000, Jiawei Zhao wrote:
>> usdt_o2 is intended to exercise the SIB (Scale-Index-Base) argument
>> handling in libbpf's USDT path. With GCC 13 this reliably produced a
>> SIB-form argument (e.g. 8@(%rdx,%rax,8)), but with newer GCC (e.g. 15)
>> the compiler frequently optimizes the probe argument into a plain
>> register (e.g. 8@...x) or a stack slot, so the test stops covering the
>> SIB code path and becomes flaky across toolchains.
>>
>> Force a SIB memory operand in the probe by:
>> * placing the base pointer into %rdx and the index into %rax using an
>> empty inline asm with output constraints ("=d", "=a") and matching
>> inputs
>> * immediately passing base[idx] to STAP_PROBE1.
>> * only enable on x86 platform.
>>
>> This makes the compiler encode the operand as SIB (base + index8),
>> which in .note.stapsdt shows up as 8@(%rdx,%rax,8) regardless of GCC
>> version. A memory clobber and noinline prevent reordering/re-allocation
>> around the probe site.
>>
>> This change is x86_64-specific and does not alter program semantics; it
>> only stabilizes the USDT argument shape so the test consistently
>> validates SIB handling. Clang historically prefers stack temporaries for
>> such operands, but the selftests build with GCC, and this keeps behavior
>> stable across GCC versions without introducing a separate .S file.
>>
>> Signed-off-by: Jiawei Zhao <phoenix500526@....com>
>> ---
>> .../selftests/bpf/prog_tests/usdt_o2.c | 20 ++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c
>> index f02dcf5188ab..e46d5743ad24 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c
>> @@ -15,11 +15,19 @@ __attribute__((optimize("O2")))
>> int lets_test_this(int);
>> static volatile __u64 array[1] = {test_value};
>>
>> -static __always_inline void trigger_func(void)
>> +static noinline void trigger_func(void)
>> {
>> +#if defined(__x86_64__) || defined(__i386__)
>> /* Base address + offset + (index * scale) */
>> - for (volatile int i = 0; i <= 0; i++)
>> - STAP_PROBE1(test, usdt1, array[i]);
>> + /* Force SIB addressing with inline assembly */
>> + const __u64 *base;
>> + __u32 idx;
>> + /* binding base to %rdx and idx to %rax */
>> + asm volatile("" : "=d"(base), "=a"(idx) : "0"(array), "1"((__u32)0) : "memory");
>> + STAP_PROBE1(test, usdt1, base[idx]);
>
>hum, I still end up with
>
> stapsdt 0x0000002a NT_STAPSDT (SystemTap probe descriptors)
> Provider: test
> Name: usdt1
> Location: 0x00000000007674c9, Base: 0x00000000035bc698, Semaphore: 0x0000000000000000
> Arguments: 8@...x
>
>disasm being:
>
> static noinline void trigger_func(void)
> {
> 76749f: 55 push %rbp
> 7674a0: 48 89 e5 mov %rsp,%rbp
> /* Base address + offset + (index * scale) */
> /* Force SIB addressing with inline assembly */
> const __u64 *base;
> __u32 idx;
> /* binding base to %rdx and idx to %rax */
> asm volatile("" : "=d"(base), "=a"(idx) : "0"(array), "1"((__u32)0) : "memory");
> 7674a3: ba 20 49 9c 03 mov $0x39c4920,%edx
> 7674a8: b8 00 00 00 00 mov $0x0,%eax
> 7674ad: 48 89 55 f8 mov %rdx,-0x8(%rbp)
> 7674b1: 89 45 f4 mov %eax,-0xc(%rbp)
> STAP_PROBE1(test, usdt1, base[idx]);
> 7674b4: 8b 45 f4 mov -0xc(%rbp),%eax
> 7674b7: 48 8d 14 c5 00 00 00 lea 0x0(,%rax,8),%rdx
> 7674be: 00
> 7674bf: 48 8b 45 f8 mov -0x8(%rbp),%rax
> 7674c3: 48 01 d0 add %rdx,%rax
> 7674c6: 48 8b 00 mov (%rax),%rax
> 7674c9: 90 nop
> #else
> STAP_PROBE1(test, usdt1, array[0]);
> #endif
> }
> 7674ca: 90 nop
> 7674cb: 5d pop %rbp
> 7674cc: c3 ret
>
>
>I wonder we could also try to bring in Andrii's usdt.h [1] and overload usdt
>arguments like outlined in the hack below (full code in [1])
>
>we will probably need smarter and sustainable change, but you I guess you get
>the idea
>
>jirka
>
>
>[1] https://github.com/anakryiko/usdt
>[2] git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git usdt_hack
>---
>diff --git a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c
>index e46d5743ad24..7bb098c37de5 100644
>--- a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c
>+++ b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c
>@@ -4,6 +4,8 @@
>
> #include "../sdt.h"
> #include "test_usdt_o2.skel.h"
>+#define USDT_ARGS ".asciz \"(,%%rax,8)\"\n"
>+#include "usdt.h"
>
> #if defined(__GNUC__) && !defined(__clang__)
> __attribute__((optimize("O2")))
>@@ -28,6 +30,7 @@ static noinline void trigger_func(void)
> #else
> STAP_PROBE1(test, usdt1, array[0]);
> #endif
>+ USDT(krava, test1, 1, 2);
> }
>
> static void basic_sib_usdt(void)
>diff --git a/tools/testing/selftests/bpf/usdt.h b/tools/testing/selftests/bpf/usdt.h
>index 549d1f774810..960ebd6aa88b 100644
>--- a/tools/testing/selftests/bpf/usdt.h
>+++ b/tools/testing/selftests/bpf/usdt.h
>@@ -403,6 +403,10 @@ struct usdt_sema { volatile unsigned short active; };
> __asm__ __volatile__ ("" :: "m" (sema));
> #endif
>
>+#ifndef USDT_ARGS
>+#define USDT_ARGS __usdt_asm_args(__VA_ARGS__)
>+#endif
>+
> /* main USDT definition (nop and .note.stapsdt metadata) */
> #define __usdt_probe(group, name, sema_def, sema, ...) do { \
> sema_def(sema) \
>@@ -418,7 +422,7 @@ struct usdt_sema { volatile unsigned short active; };
> __usdt_asm1( __usdt_asm_addr sema) \
> __usdt_asm_strz(group) \
> __usdt_asm_strz(name) \
>- __usdt_asm_args(__VA_ARGS__) \
>+ USDT_ARGS \
> __usdt_asm1( .ascii "\0") \
> __usdt_asm1(994: .balign 4) \
> __usdt_asm1( .popsection) \
Powered by blists - more mailing lists