Re: [PATCH bpf-next v2 1/3] selftests/bpf: Refactor the failed assertion to another subtest

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 15, 2025 at 11:54 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
>
> When building the selftest with arm64/clang20, the following test failed:
>     ...
>     ubtest_multispec_usdt:PASS:usdt_100_called 0 nsec
>     subtest_multispec_usdt:PASS:usdt_100_sum 0 nsec
>     subtest_multispec_usdt:FAIL:usdt_300_bad_attach unexpected pointer: 0xaaaad82a2a80
>     #469/2   usdt/multispec:FAIL
>     #469     usdt:FAIL
>
> The failed assertion
>     subtest_multispec_usdt:FAIL:usdt_300_bad_attach unexpected pointer: 0xaaaad82a2a80
> is caused by bpf_program__attach_usdt() which is expected to fail. But
> with arm64/clang20 bpf_program__attach_usdt() actually succeeded.

I think I missed that it's unexpected *success* that is causing
issues. If that's so, then I think it might be more straightforward to
just ensure that test is expectedly failing regardless of compiler
code generation logic. Maybe something along the following lines:

diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c
b/tools/testing/selftests/bpf/prog_tests/usdt.c
index 495d66414b57..fdd8642cfdff 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -190,11 +190,21 @@ static void __always_inline f300(int x)
        STAP_PROBE1(test, usdt_300, x);
 }

+#define RP10(F, X)  F(*(X+0)); F(*(X+1));F(*(X+2)); F(*(X+3)); F(*(X+4)); \
+                   F(*(X+5)); F(*(X+6)); F(*(X+7)); F(*(X+8)); F(*(X+9));
+#define RP100(F, X) RP10(F,X+
0);RP10(F,X+10);RP10(F,X+20);RP10(F,X+30);RP10(F,X+40); \
+
RP10(F,X+50);RP10(F,X+60);RP10(F,X+70);RP10(F,X+80);RP10(F,X+90);
+
 __weak void trigger_300_usdts(void)
 {
-       R100(f300, 0);
-       R100(f300, 100);
-       R100(f300, 200);
+       volatile int arr[300], i;
+
+       for (i = 0; i < 300; i++)
+               arr[i] = 300;
+
+       RP100(f300, arr + 0);
+       RP100(f300, arr + 100);
+       RP100(f300, arr + 200);
 }


So basically force the compiler to use 300 different locations for
each of 300 USDT instantiations? I didn't check how that will look
like on arm64, but on x86 gcc it seems to generate what is expected of
it.

Can you please try it on arm64 and see if that works?

>
> Checking usdt probes with usdt.test.o,
>
> with gcc11 build binary:
>   stapsdt              0x0000002e       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x00000000000054f8, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[sp]
>   stapsdt              0x00000031       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x0000000000005510, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[sp, 4]
>   ...
>   stapsdt              0x00000032       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x0000000000005660, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[sp, 60]
>   ...
>   stapsdt              0x00000034       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x00000000000070e8, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[sp, 1192]
>   stapsdt              0x00000034       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x0000000000007100, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[sp, 1196]
>   ...
>   stapsdt              0x00000032       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x0000000000009ec4, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[sp, 60]
>
> with clang20 build binary:
>   stapsdt              0x0000002e       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x00000000000009a0, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[x9]
>   stapsdt              0x0000002e       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x00000000000009b8, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[x9]
>   ...
>   stapsdt              0x0000002e       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x0000000000002590, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[x9]
>   stapsdt              0x0000002e       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x00000000000025a8, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[x8]
>   ...
>   stapsdt              0x0000002f       NT_STAPSDT (SystemTap probe descriptors)
>     Provider: test
>     Name: usdt_300
>     Location: 0x0000000000007fdc, Base: 0x0000000000000000, Semaphore: 0x0000000000000008
>     Arguments: -4@[x10]
>
> There are total 301 locations for usdt_300. For gcc11 built binary, there are
> 300 spec's. But for clang20 built binary, there are 3 spec's. The libbpf default
> BPF_USDT_MAX_SPEC_CNT is 256. So for gcc11, the above bpf_program__attach_usdt() will
> fail, but the function will succeed for clang20.
>
> Note that we cannot just change BPF_USDT_MAX_SPEC_CNT from 256 to 2 (through overwriting
> BPF_USDT_MAX_SPEC_CNT before usdt.bpf.h) since it will cause other test failures.
> We cannot just set BPF_USDT_MAX_SPEC_CNT to 2 for test_usdt_multispec.c since we
> have below in the Makefile:
>   test_usdt.skel.h-deps := test_usdt.bpf.o test_usdt_multispec.bpf.o
> and the linker will enforce that BPF_USDT_MAX_SPEC_CNT values for both progs must
> be the same.
>
> The refactoring does not change existing test result. But the future change will
> allow to set BPF_USDT_MAX_SPEC_CNT to be 2 for arm64/clang20 case, which will have
> the same attachment failure as in gcc11.
>
> Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> ---
>  tools/testing/selftests/bpf/prog_tests/usdt.c | 35 +++++++++++++------
>  1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
> index 495d66414b57..dc29ef94312a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
> @@ -270,18 +270,8 @@ static void subtest_multispec_usdt(void)
>          */
>         trigger_300_usdts();
>
> -       /* we'll reuse usdt_100 BPF program for usdt_300 test */
>         bpf_link__destroy(skel->links.usdt_100);
> -       skel->links.usdt_100 = bpf_program__attach_usdt(skel->progs.usdt_100, -1, "/proc/self/exe",
> -                                                       "test", "usdt_300", NULL);
> -       err = -errno;
> -       if (!ASSERT_ERR_PTR(skel->links.usdt_100, "usdt_300_bad_attach"))
> -               goto cleanup;
> -       ASSERT_EQ(err, -E2BIG, "usdt_300_attach_err");
>
> -       /* let's check that there are no "dangling" BPF programs attached due
> -        * to partial success of the above test:usdt_300 attachment
> -        */
>         bss->usdt_100_called = 0;
>         bss->usdt_100_sum = 0;
>
> @@ -312,6 +302,29 @@ static void subtest_multispec_usdt(void)
>         test_usdt__destroy(skel);
>  }
>
> +static void subtest_multispec_fail_usdt(void)
> +{
> +       LIBBPF_OPTS(bpf_usdt_opts, opts);
> +       struct test_usdt *skel;
> +       int err;
> +
> +       skel = test_usdt__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "skel_open"))
> +               return;
> +
> +       skel->bss->my_pid = getpid();
> +
> +       skel->links.usdt_100 = bpf_program__attach_usdt(skel->progs.usdt_100, -1, "/proc/self/exe",
> +                                                       "test", "usdt_300", NULL);
> +       err = -errno;
> +       if (!ASSERT_ERR_PTR(skel->links.usdt_100, "usdt_300_bad_attach"))
> +               goto cleanup;
> +       ASSERT_EQ(err, -E2BIG, "usdt_300_attach_err");
> +
> +cleanup:
> +       test_usdt__destroy(skel);
> +}
> +
>  static FILE *urand_spawn(int *pid)
>  {
>         FILE *f;
> @@ -422,6 +435,8 @@ void test_usdt(void)
>                 subtest_basic_usdt();
>         if (test__start_subtest("multispec"))
>                 subtest_multispec_usdt();
> +       if (test__start_subtest("multispec_fail"))
> +               subtest_multispec_fail_usdt();
>         if (test__start_subtest("urand_auto_attach"))
>                 subtest_urandom_usdt(true /* auto_attach */);
>         if (test__start_subtest("urand_pid_attach"))
> --
> 2.47.1
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux