* [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change
@ 2023-11-27 5:03 Yonghong Song
2023-11-27 14:00 ` patchwork-bot+netdevbpf
2023-11-27 18:49 ` Andrii Nakryiko
0 siblings, 2 replies; 5+ messages in thread
From: Yonghong Song @ 2023-11-27 5:03 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
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;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change
2023-11-27 5:03 [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change Yonghong Song
@ 2023-11-27 14:00 ` patchwork-bot+netdevbpf
2023-11-27 18:49 ` Andrii Nakryiko
1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-11-27 14:00 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, ast, andrii, daniel, kernel-team, martin.lau
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change
2023-11-27 5:03 [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change Yonghong Song
2023-11-27 14:00 ` patchwork-bot+netdevbpf
@ 2023-11-27 18:49 ` Andrii Nakryiko
2023-11-27 19:49 ` Yonghong Song
1 sibling, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2023-11-27 18:49 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change
2023-11-27 18:49 ` Andrii Nakryiko
@ 2023-11-27 19:49 ` Yonghong Song
2023-11-27 19:57 ` Andrii Nakryiko
0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2023-11-27 19:49 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
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
>>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change
2023-11-27 19:49 ` Yonghong Song
@ 2023-11-27 19:57 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2023-11-27 19:57 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
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
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-27 19:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-27 5:03 [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change Yonghong Song
2023-11-27 14:00 ` patchwork-bot+netdevbpf
2023-11-27 18:49 ` Andrii Nakryiko
2023-11-27 19:49 ` Yonghong Song
2023-11-27 19:57 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox