Re: [PATCH V4 1/3] tools/perf: Fix the string match for "/tmp/perf-$PID.map" files in dso__load

[Athira Rajeev <atrajeev@linux.vnet.ibm.com>]

> On 25 Jun 2024, at 11:29 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> 
> Hello,
> 
> On Tue, Jun 25, 2024 at 5:02 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>> 
>> On 25/06/24 14:57, Adrian Hunter wrote:
>>> On 23/06/24 09:48, Athira Rajeev wrote:
>>>> Perf test for perf probe of function from different CU fails
>>>> as below:
>>>> 
>>>>     ./perf test -vv "test perf probe of function from different CU"
>>>>     116: test perf probe of function from different CU:
>>>>     --- start ---
>>>>     test child forked, pid 2679
>>>>     Failed to find symbol foo in /tmp/perf-uprobe-different-cu-sh.Msa7iy89bx/testfile
>>>>       Error: Failed to add events.
>>>>     --- Cleaning up ---
>>>>     "foo" does not hit any event.
>>>>       Error: Failed to delete events.
>>>>     ---- end(-1) ----
>>>>     116: test perf probe of function from different CU                   : FAILED!
>>>> 
>>>> The test does below to probe function "foo" :
>>>> 
>>>>     # gcc -g -Og -flto -c /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile-foo.c
>>>>     -o /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile-foo.o
>>>>     # gcc -g -Og -c /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile-main.c
>>>>     -o /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile-main.o
>>>>     # gcc -g -Og -o /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile
>>>>     /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile-foo.o
>>>>     /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile-main.o
>>>> 
>>>>     # ./perf probe -x /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile foo
>>>>     Failed to find symbol foo in /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7/testfile
>>>>        Error: Failed to add events.
>>>> 
>>>> Perf probe fails to find symbol foo in the executable placed in
>>>> /tmp/perf-uprobe-different-cu-sh.XniNxNEVT7
>>>> 
>>>> Simple reproduce:
>>>> 
>>>> # mktemp -d /tmp/perf-checkXXXXXXXXXX
>>>>   /tmp/perf-checkcWpuLRQI8j
>>>> 
>>>> # gcc -g -o test test.c
>>>> # cp test /tmp/perf-checkcWpuLRQI8j/
>>>> # nm /tmp/perf-checkcWpuLRQI8j/test | grep foo
>>>>   00000000100006bc T foo
>>>> 
>>>> # ./perf probe -x /tmp/perf-checkcWpuLRQI8j/test foo
>>>>   Failed to find symbol foo in /tmp/perf-checkcWpuLRQI8j/test
>>>>      Error: Failed to add events.
>>>> 
>>>> But it works with any files like /tmp/perf/test. Only for
>>>> patterns with "/tmp/perf-", this fails.
>>>> 
>>>> Further debugging, commit 80d496be89ed ("perf report: Add support
>>>> for profiling JIT generated code") added support for profiling JIT
>>>> generated code. This patch handles dso's of form
>>>> "/tmp/perf-$PID.map" .
>>>> 
>>>> The check used "if (strncmp(self->name, "/tmp/perf-", 10) == 0)"
>>>> to match "/tmp/perf-$PID.map". With this commit, any dso in
>>>> /tmp/perf- folder will be considered separately for processing
>>>> (not only JIT created map files ). Fix this by changing the
>>>> string pattern to check for "/tmp/perf-%d.map". Add a helper
>>>> function is_perf_pid_map_name to do this check. In "struct dso",
>>>> dso->long_name holds the long name of the dso file. Since the
>>>> /tmp/perf-$PID.map check uses the complete name, use dso___long_name for
>>>> the string name.
>>>> 
>>>> With the fix,
>>>>     # ./perf test "test perf probe of function from different CU"
>>>>     117: test perf probe of function from different CU                   : Ok
>>>> 
>>>> Signed-off-by: Athira Rajeev<atrajeev@linux.vnet.ibm.com>
>>> 
>>> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
>>> 
>> 
>> Although it could use a Fixes tag
>> 
> 
> Thanks, I will add
> 
> Fixes: 56cbeacf1435 ("perf probe: Add test for regression introduced
> by switch to die_get_decl_file()")
> 
> Namhyung
> 
Hi Adrian,
Thanks for the review.

Thanks Namhyung for adding the Fixes tag and pulling the fix

Athira