All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.