Re: [PATCH bpf-next 3/3] selftests/bpf: Add bpf_get_func_ip test for uprobe inside function

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Aug 02, 2023 at 12:30:36PM +0100, Alan Maguire wrote:
> On 01/08/2023 08:30, Jiri Olsa wrote:
> > Adding get_func_ip test for uprobe inside function that validates
> > the get_func_ip helper returns correct probe address value.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  .../bpf/prog_tests/get_func_ip_test.c         | 40 ++++++++++++++++++-
> >  .../bpf/progs/get_func_ip_uprobe_test.c       | 18 +++++++++
> >  2 files changed, 57 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/get_func_ip_uprobe_test.c
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c b/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c
> > index 114cdbc04caf..f199220ad6de 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c
> > @@ -55,7 +55,16 @@ static void test_function_entry(void)
> >   * offset, disabling it for all other archs
> 
> nit: comment here
> 
> /* test6 is x86_64 specific because of the instruction
>  * offset, disabling it for all other archs
> 
> ...should probably be updated now multiple tests are gated by the
> #ifdef __x86_64__.

right will update that

> 
> BTW I tested if these tests would pass on aarch64 with a few tweaks
> to instruction offsets, and they do. Something like the following
> gets all of the tests running and passing on aarch64:

nice, thanks a lot for testing that

SNIP

> diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_uprobe_test.c
> b/tools/testing/selftests/bpf/progs/get_func_ip_uprobe_test.c
> index 052f8a4345a8..56af4a8447b9 100644
> --- a/tools/testing/selftests/bpf/progs/get_func_ip_uprobe_test.c
> +++ b/tools/testing/selftests/bpf/progs/get_func_ip_uprobe_test.c
> @@ -8,11 +8,17 @@ char _license[] SEC("license") = "GPL";
>  unsigned long uprobe_trigger_body;
> 
>  __u64 test1_result = 0;
> +#if defined(__TARGET_ARCH_x86)
> +#define OFFSET 1
>  SEC("uprobe//proc/self/exe:uprobe_trigger_body+1")
> +#elif defined(__TARGET_ARCH_arm64)
> +#define OFFSET 4
> +SEC("uprobe//proc/self/exe:uprobe_trigger_body+4")
> +#endif
>  int BPF_UPROBE(test1)
>  {
>         __u64 addr = bpf_get_func_ip(ctx);
> 
> -       test1_result = (const void *) addr == (const void *)
> uprobe_trigger_body + 1;
> +       test1_result = (const void *) addr == (const void *)
> uprobe_trigger_body + OFFSET;
>         return 0;
>  }
> 
> 
> Anyway if you're doing a later version and want to roll something like
> the above in feel free, otherwise I can send a followup patch later on.
> Regardless, for the series on aarch64:

I'd preffer if you could send follow up for arm, because I have
no easy way to test that change

thanks,
jirka