Re: [PATCH bpf-next] bpf: Prevent inlining of bpf_fentry_test7()

[Alan Maguire <alan.maguire@oracle.com>]

On 26/08/2023 21:08, Yonghong Song wrote:
> With latest clang18, I hit test_progs failures for the following test:
>   #13/2    bpf_cookie/multi_kprobe_link_api:FAIL
>   #13/3    bpf_cookie/multi_kprobe_attach_api:FAIL
>   #13      bpf_cookie:FAIL
>   #75      fentry_fexit:FAIL
>   #76/1    fentry_test/fentry:FAIL
>   #76      fentry_test:FAIL
>   #80/1    fexit_test/fexit:FAIL
>   #80      fexit_test:FAIL
>   #110/1   kprobe_multi_test/skel_api:FAIL
>   #110/2   kprobe_multi_test/link_api_addrs:FAIL
>   #110/3   kprobe_multi_test/link_api_syms:FAIL
>   #110/4   kprobe_multi_test/attach_api_pattern:FAIL
>   #110/5   kprobe_multi_test/attach_api_addrs:FAIL
>   #110/6   kprobe_multi_test/attach_api_syms:FAIL
>   #110     kprobe_multi_test:FAIL
> 
> For example, for #13/2, the error messages are
>   ...
>   kprobe_multi_test_run:FAIL:kprobe_test7_result unexpected kprobe_test7_result: actual 0 != expected 1
>   ...
>   kprobe_multi_test_run:FAIL:kretprobe_test7_result unexpected kretprobe_test7_result: actual 0 != expected 1
> 
> clang17 does not have this issue.
> 
> Further investigation shows that kernel func bpf_fentry_test7(), used
> in the above tests, is inlined by the compiler although it is
> marked as noinline.
> 
>   int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>   {
>         return (long)arg;
>   }
> 
> It is known that for simple functions like the above (e.g. just returning
> a constant or an input argument), the clang compiler may still do inlining
> for a noinline function. Adding 'asm volatile ("")' in the beginning of the
> bpf_fentry_test7() can prevent inlining.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>

Fixes: d923021c2ce12 ("bpf: Add tests for PTR_TO_BTF_ID vs. null
comparison")

...might help this land in stable trees too. Thanks!

> ---
>  net/bpf/test_run.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 57a7a64b84ed..0841f8d82419 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -543,6 +543,7 @@ struct bpf_fentry_test_t {
>  
>  int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>  {
> +	asm volatile ("");
>  	return (long)arg;
>  }
>  
Is there a risk bpf_fentry_test8/9 might get inlined too?