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@xxxxxxxxxx>
> > ---
> > .../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
