mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Jiawei Zhao <phoenix500526@163.com>, ast@kernel.org
Cc: daniel@iogearbox.net, andrii@kernel.org, bpf@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic
Date: Tue, 5 Aug 2025 12:42:22 -0700	[thread overview]
Message-ID: <058d4d74-23ff-444b-a773-9d45204900f2@linux.dev> (raw)
In-Reply-To: <20250802084803.108777-3-phoenix500526@163.com>



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 };
>   

[...]


  reply	other threads:[~2025-08-05 19:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-02  8:48 [PATCH v6 0/2] libbpf: fix USDT SIB argument handling causing unrecognized register error Jiawei Zhao
2025-08-02  8:48 ` [PATCH v6 1/2] " Jiawei Zhao
2025-08-04 10:23   ` Jiri Olsa
2025-08-05  4:15     ` 赵佳炜
2025-08-02  8:48 ` [PATCH v6 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic Jiawei Zhao
2025-08-05 19:42   ` Yonghong Song [this message]
2025-08-06  9:47     ` 赵佳炜

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=058d4d74-23ff-444b-a773-9d45204900f2@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=phoenix500526@163.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®