On Sun, Nov 26, 2023 at 9:04 PM Yonghong Song <yonghong.song@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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 >
