| Message ID | 20231127050342.1945270-1-yonghong.song@linux.dev (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b16904fd9f01b580db357ef2b1cc9e86d89576c2 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf: Fix a few selftest failures due to llvm18 change | expand |
Hello: This patch was applied to bpf/bpf-next.git (master) by Daniel Borkmann <daniel@iogearbox.net>: On Sun, 26 Nov 2023 21:03:42 -0800 you wrote: > With latest upstream llvm18, the following test cases failed: > $ ./test_progs -j > #13/2 bpf_cookie/multi_kprobe_link_api:FAIL > #13/3 bpf_cookie/multi_kprobe_attach_api:FAIL > #13 bpf_cookie:FAIL > #77 fentry_fexit:FAIL > #78/1 fentry_test/fentry:FAIL > #78 fentry_test:FAIL > #82/1 fexit_test/fexit:FAIL > #82 fexit_test:FAIL > #112/1 kprobe_multi_test/skel_api:FAIL > #112/2 kprobe_multi_test/link_api_addrs:FAIL > ... > #112 kprobe_multi_test:FAIL > #356/17 test_global_funcs/global_func17:FAIL > #356 test_global_funcs:FAIL > > [...] Here is the summary with links: - [bpf-next] bpf: Fix a few selftest failures due to llvm18 change https://git.kernel.org/bpf/bpf-next/c/b16904fd9f01 You are awesome, thank you!
On Sun, Nov 26, 2023 at 9:04 PM Yonghong Song <yonghong.song@linux.dev> wrote: > > With latest upstream llvm18, the following test cases failed: > $ ./test_progs -j > #13/2 bpf_cookie/multi_kprobe_link_api:FAIL > #13/3 bpf_cookie/multi_kprobe_attach_api:FAIL > #13 bpf_cookie:FAIL > #77 fentry_fexit:FAIL > #78/1 fentry_test/fentry:FAIL > #78 fentry_test:FAIL > #82/1 fexit_test/fexit:FAIL > #82 fexit_test:FAIL > #112/1 kprobe_multi_test/skel_api:FAIL > #112/2 kprobe_multi_test/link_api_addrs:FAIL > ... > #112 kprobe_multi_test:FAIL > #356/17 test_global_funcs/global_func17:FAIL > #356 test_global_funcs:FAIL > > Further analysis shows llvm upstream patch [1] is responsible > for the above failures. For example, for function bpf_fentry_test7() > in net/bpf/test_run.c, without [1], the asm code is: > 0000000000000400 <bpf_fentry_test7>: > 400: f3 0f 1e fa endbr64 > 404: e8 00 00 00 00 callq 0x409 <bpf_fentry_test7+0x9> > 409: 48 89 f8 movq %rdi, %rax > 40c: c3 retq > 40d: 0f 1f 00 nopl (%rax) > and with [1], the asm code is: > 0000000000005d20 <bpf_fentry_test7.specialized.1>: > 5d20: e8 00 00 00 00 callq 0x5d25 <bpf_fentry_test7.specialized.1+0x5> > 5d25: c3 retq > and <bpf_fentry_test7.specialized.1> is called instead of <bpf_fentry_test7> > and this caused test failures for #13/#77 etc. except #356. > > For test case #356/17, with [1] (progs/test_global_func17.c)), > the main prog looks like: > 0000000000000000 <global_func17>: > 0: b4 00 00 00 2a 00 00 00 w0 = 0x2a > 1: 95 00 00 00 00 00 00 00 exit > which passed verification while the test itself expects a verification > failure. > > Let us add 'barrier_var' style asm code in both places to prevent > function specialization which caused selftests failure. > > [1] https://github.com/llvm/llvm-project/pull/72903 > > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- > net/bpf/test_run.c | 2 +- > tools/testing/selftests/bpf/progs/test_global_func17.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > index c9fdcc5cdce1..711cf5d59816 100644 > --- a/net/bpf/test_run.c > +++ b/net/bpf/test_run.c > @@ -542,7 +542,7 @@ struct bpf_fentry_test_t { > > int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg) > { > - asm volatile (""); > + asm volatile ("": "+r"(arg)); > return (long)arg; > } > > diff --git a/tools/testing/selftests/bpf/progs/test_global_func17.c b/tools/testing/selftests/bpf/progs/test_global_func17.c > index a32e11c7d933..5de44b09e8ec 100644 > --- a/tools/testing/selftests/bpf/progs/test_global_func17.c > +++ b/tools/testing/selftests/bpf/progs/test_global_func17.c > @@ -5,6 +5,7 @@ > > __noinline int foo(int *p) > { > + barrier_var(p); > return p ? (*p = 42) : 0; > } > I recently stumbled upon no_clone ([0]) and no_ipa ([1]) attributes. Should we consider using those here instead? [0] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noclone-function-attribute [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noipa-function-attribute > -- > 2.34.1 >
On 11/27/23 1:49 PM, Andrii Nakryiko wrote: > On Sun, Nov 26, 2023 at 9:04 PM Yonghong Song <yonghong.song@linux.dev> wrote: >> With latest upstream llvm18, the following test cases failed: >> $ ./test_progs -j >> #13/2 bpf_cookie/multi_kprobe_link_api:FAIL >> #13/3 bpf_cookie/multi_kprobe_attach_api:FAIL >> #13 bpf_cookie:FAIL >> #77 fentry_fexit:FAIL >> #78/1 fentry_test/fentry:FAIL >> #78 fentry_test:FAIL >> #82/1 fexit_test/fexit:FAIL >> #82 fexit_test:FAIL >> #112/1 kprobe_multi_test/skel_api:FAIL >> #112/2 kprobe_multi_test/link_api_addrs:FAIL >> ... >> #112 kprobe_multi_test:FAIL >> #356/17 test_global_funcs/global_func17:FAIL >> #356 test_global_funcs:FAIL >> >> Further analysis shows llvm upstream patch [1] is responsible >> for the above failures. For example, for function bpf_fentry_test7() >> in net/bpf/test_run.c, without [1], the asm code is: >> 0000000000000400 <bpf_fentry_test7>: >> 400: f3 0f 1e fa endbr64 >> 404: e8 00 00 00 00 callq 0x409 <bpf_fentry_test7+0x9> >> 409: 48 89 f8 movq %rdi, %rax >> 40c: c3 retq >> 40d: 0f 1f 00 nopl (%rax) >> and with [1], the asm code is: >> 0000000000005d20 <bpf_fentry_test7.specialized.1>: >> 5d20: e8 00 00 00 00 callq 0x5d25 <bpf_fentry_test7.specialized.1+0x5> >> 5d25: c3 retq >> and <bpf_fentry_test7.specialized.1> is called instead of <bpf_fentry_test7> >> and this caused test failures for #13/#77 etc. except #356. >> >> For test case #356/17, with [1] (progs/test_global_func17.c)), >> the main prog looks like: >> 0000000000000000 <global_func17>: >> 0: b4 00 00 00 2a 00 00 00 w0 = 0x2a >> 1: 95 00 00 00 00 00 00 00 exit >> which passed verification while the test itself expects a verification >> failure. >> >> Let us add 'barrier_var' style asm code in both places to prevent >> function specialization which caused selftests failure. >> >> [1] https://github.com/llvm/llvm-project/pull/72903 >> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> >> --- >> net/bpf/test_run.c | 2 +- >> tools/testing/selftests/bpf/progs/test_global_func17.c | 1 + >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c >> index c9fdcc5cdce1..711cf5d59816 100644 >> --- a/net/bpf/test_run.c >> +++ b/net/bpf/test_run.c >> @@ -542,7 +542,7 @@ struct bpf_fentry_test_t { >> >> int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg) >> { >> - asm volatile (""); >> + asm volatile ("": "+r"(arg)); >> return (long)arg; >> } >> >> diff --git a/tools/testing/selftests/bpf/progs/test_global_func17.c b/tools/testing/selftests/bpf/progs/test_global_func17.c >> index a32e11c7d933..5de44b09e8ec 100644 >> --- a/tools/testing/selftests/bpf/progs/test_global_func17.c >> +++ b/tools/testing/selftests/bpf/progs/test_global_func17.c >> @@ -5,6 +5,7 @@ >> >> __noinline int foo(int *p) >> { >> + barrier_var(p); >> return p ? (*p = 42) : 0; >> } >> > I recently stumbled upon no_clone ([0]) and no_ipa ([1]) attributes. > Should we consider using those here instead? > > [0] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noclone-function-attribute > [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noipa-function-attribute noipa attribute might help here. But sadly, noclone and noipa are gcc specific and clang does not support either of them. > > >> -- >> 2.34.1 >>
On Mon, Nov 27, 2023 at 11:49 AM Yonghong Song <yonghong.song@linux.dev> wrote: > > > On 11/27/23 1:49 PM, Andrii Nakryiko wrote: > > On Sun, Nov 26, 2023 at 9:04 PM Yonghong Song <yonghong.song@linux.dev> wrote: > >> With latest upstream llvm18, the following test cases failed: > >> $ ./test_progs -j > >> #13/2 bpf_cookie/multi_kprobe_link_api:FAIL > >> #13/3 bpf_cookie/multi_kprobe_attach_api:FAIL > >> #13 bpf_cookie:FAIL > >> #77 fentry_fexit:FAIL > >> #78/1 fentry_test/fentry:FAIL > >> #78 fentry_test:FAIL > >> #82/1 fexit_test/fexit:FAIL > >> #82 fexit_test:FAIL > >> #112/1 kprobe_multi_test/skel_api:FAIL > >> #112/2 kprobe_multi_test/link_api_addrs:FAIL > >> ... > >> #112 kprobe_multi_test:FAIL > >> #356/17 test_global_funcs/global_func17:FAIL > >> #356 test_global_funcs:FAIL > >> > >> Further analysis shows llvm upstream patch [1] is responsible > >> for the above failures. For example, for function bpf_fentry_test7() > >> in net/bpf/test_run.c, without [1], the asm code is: > >> 0000000000000400 <bpf_fentry_test7>: > >> 400: f3 0f 1e fa endbr64 > >> 404: e8 00 00 00 00 callq 0x409 <bpf_fentry_test7+0x9> > >> 409: 48 89 f8 movq %rdi, %rax > >> 40c: c3 retq > >> 40d: 0f 1f 00 nopl (%rax) > >> and with [1], the asm code is: > >> 0000000000005d20 <bpf_fentry_test7.specialized.1>: > >> 5d20: e8 00 00 00 00 callq 0x5d25 <bpf_fentry_test7.specialized.1+0x5> > >> 5d25: c3 retq > >> and <bpf_fentry_test7.specialized.1> is called instead of <bpf_fentry_test7> > >> and this caused test failures for #13/#77 etc. except #356. > >> > >> For test case #356/17, with [1] (progs/test_global_func17.c)), > >> the main prog looks like: > >> 0000000000000000 <global_func17>: > >> 0: b4 00 00 00 2a 00 00 00 w0 = 0x2a > >> 1: 95 00 00 00 00 00 00 00 exit > >> which passed verification while the test itself expects a verification > >> failure. > >> > >> Let us add 'barrier_var' style asm code in both places to prevent > >> function specialization which caused selftests failure. > >> > >> [1] https://github.com/llvm/llvm-project/pull/72903 > >> > >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > >> --- > >> net/bpf/test_run.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_global_func17.c | 1 + > >> 2 files changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > >> index c9fdcc5cdce1..711cf5d59816 100644 > >> --- a/net/bpf/test_run.c > >> +++ b/net/bpf/test_run.c > >> @@ -542,7 +542,7 @@ struct bpf_fentry_test_t { > >> > >> int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg) > >> { > >> - asm volatile (""); > >> + asm volatile ("": "+r"(arg)); > >> return (long)arg; > >> } > >> > >> diff --git a/tools/testing/selftests/bpf/progs/test_global_func17.c b/tools/testing/selftests/bpf/progs/test_global_func17.c > >> index a32e11c7d933..5de44b09e8ec 100644 > >> --- a/tools/testing/selftests/bpf/progs/test_global_func17.c > >> +++ b/tools/testing/selftests/bpf/progs/test_global_func17.c > >> @@ -5,6 +5,7 @@ > >> > >> __noinline int foo(int *p) > >> { > >> + barrier_var(p); > >> return p ? (*p = 42) : 0; > >> } > >> > > I recently stumbled upon no_clone ([0]) and no_ipa ([1]) attributes. > > Should we consider using those here instead? > > > > [0] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noclone-function-attribute > > [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noipa-function-attribute > > noipa attribute might help here. But sadly, noclone and noipa are gcc specific > and clang does not support either of them. I see, that's too bad, I assumed Clang also supports something like that. Maybe someday. > > > > > > >> -- > >> 2.34.1 > >>
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index c9fdcc5cdce1..711cf5d59816 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -542,7 +542,7 @@ struct bpf_fentry_test_t { int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg) { - asm volatile (""); + asm volatile ("": "+r"(arg)); return (long)arg; } diff --git a/tools/testing/selftests/bpf/progs/test_global_func17.c b/tools/testing/selftests/bpf/progs/test_global_func17.c index a32e11c7d933..5de44b09e8ec 100644 --- a/tools/testing/selftests/bpf/progs/test_global_func17.c +++ b/tools/testing/selftests/bpf/progs/test_global_func17.c @@ -5,6 +5,7 @@ __noinline int foo(int *p) { + barrier_var(p); return p ? (*p = 42) : 0; }
With latest upstream llvm18, the following test cases failed: $ ./test_progs -j #13/2 bpf_cookie/multi_kprobe_link_api:FAIL #13/3 bpf_cookie/multi_kprobe_attach_api:FAIL #13 bpf_cookie:FAIL #77 fentry_fexit:FAIL #78/1 fentry_test/fentry:FAIL #78 fentry_test:FAIL #82/1 fexit_test/fexit:FAIL #82 fexit_test:FAIL #112/1 kprobe_multi_test/skel_api:FAIL #112/2 kprobe_multi_test/link_api_addrs:FAIL ... #112 kprobe_multi_test:FAIL #356/17 test_global_funcs/global_func17:FAIL #356 test_global_funcs:FAIL Further analysis shows llvm upstream patch [1] is responsible for the above failures. For example, for function bpf_fentry_test7() in net/bpf/test_run.c, without [1], the asm code is: 0000000000000400 <bpf_fentry_test7>: 400: f3 0f 1e fa endbr64 404: e8 00 00 00 00 callq 0x409 <bpf_fentry_test7+0x9> 409: 48 89 f8 movq %rdi, %rax 40c: c3 retq 40d: 0f 1f 00 nopl (%rax) and with [1], the asm code is: 0000000000005d20 <bpf_fentry_test7.specialized.1>: 5d20: e8 00 00 00 00 callq 0x5d25 <bpf_fentry_test7.specialized.1+0x5> 5d25: c3 retq and <bpf_fentry_test7.specialized.1> is called instead of <bpf_fentry_test7> and this caused test failures for #13/#77 etc. except #356. For test case #356/17, with [1] (progs/test_global_func17.c)), the main prog looks like: 0000000000000000 <global_func17>: 0: b4 00 00 00 2a 00 00 00 w0 = 0x2a 1: 95 00 00 00 00 00 00 00 exit which passed verification while the test itself expects a verification failure. Let us add 'barrier_var' style asm code in both places to prevent function specialization which caused selftests failure. [1] https://github.com/llvm/llvm-project/pull/72903 Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- net/bpf/test_run.c | 2 +- tools/testing/selftests/bpf/progs/test_global_func17.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)