Re: [PATCH bpf-next] bpf: Fix a few selftest failures due to llvm18 change

[Yonghong Song <yonghong.song@linux.dev>]

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
>>