On 02/08/2023 13:26, Jiri Olsa wrote:
> 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
>
sure, will do! thanks!
Alan
