Re: [RFC] bpf: Issue with bpf_fentry_test7 call

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
> hi,   
> The bpf CI is broken due to clang emitting 2 functions for
> bpf_fentry_test7:
> 
>   # cat available_filter_functions | grep bpf_fentry_test7
>   bpf_fentry_test7
>   bpf_fentry_test7.specialized.1
> 
> The tests attach to 'bpf_fentry_test7' while the function with
> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
> 
> It looks like clang optimalization that comes from passing 0
> as argument and returning it directly in bpf_fentry_test7.
> 
> I'm not sure there's a way to disable this, so far I came
> up with solution below that passes real pointer, but I think
> that was not the original intention for the test.
> 
> We had issue with this function back in august:
>   32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
> 
> I'm not sure why it started to show now? was clang updated for CI?
> 
> I'll try to find out more, but any clang ideas are welcome ;-)

fyi also there's probably another related usse in global_func17 test:

	run_subtest:FAIL:unexpected_load_success unexpected success: 0
	#290/17  test_global_funcs/global_func17:FAIL

looks like clang optimized the call out and returns the value directly:

	Disassembly of section .text:

	0000000000000000 <foo>:
	       0:       b4 00 00 00 00 00 00 00 w0 = 0x0
	       1:       15 01 02 00 00 00 00 00 if r1 == 0x0 goto +0x2 <LBB0_2>
	       2:       b4 00 00 00 2a 00 00 00 w0 = 0x2a
	       3:       63 01 00 00 00 00 00 00 *(u32 *)(r1 + 0x0) = r0

	0000000000000020 <LBB0_2>:
	       4:       95 00 00 00 00 00 00 00 exit

	Disassembly of section tc:

	0000000000000000 <global_func17>:
	       0:       b4 00 00 00 2a 00 00 00 w0 = 0x2a
	       1:       95 00 00 00 00 00 00 00 exit

jirka


> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index c9fdcc5cdce1..33208eec9361 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
>  int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>  {
>  	asm volatile ("");
> -	return (long)arg;
> +	return 0;
>  }
>  
>  int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
>  		    bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
>  		    bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
>  		    bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
> -		    bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
> +		    bpf_fentry_test7(&arg) != 0 ||
>  		    bpf_fentry_test8(&arg) != 0 ||
>  		    bpf_fentry_test9(&retval) != 0)
>  			goto out;
> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> index 52a550d281d9..95c5c34ccaa8 100644
> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
>  SEC("fentry/bpf_fentry_test7")
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
> -	if (!arg)
> +	if (arg)
>  		test7_result = 1;
>  	return 0;
>  }
> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> index 8f1ccb7302e1..ffb30236ca02 100644
> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
>  SEC("fexit/bpf_fentry_test7")
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
> -	if (!arg)
> +	if (arg)
>  		test7_result = 1;
>  	return 0;
>  }