Re: [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Mon, Jun 17, 2024 at 02:52:52PM +0100, James Clark wrote:
> 
> 
> On 01/06/2024 13:59, Chaitanya S Prakash wrote:
> > From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> > 
> > Strengthen the check to locate symbol map files in the tool's /tmp
> > directory. As the existing check allows non map files named in
> > "/tmp/perf-" pattern, it leads to possible dwarf library corruption
> > when perf is built with NO_DWARF=1. The try_to_find_probe_trace_events()
> > function behaves differently when built in that manner. Extending the
> > current check condition using the str_has_suffix() function to not only
> > validate the pattern but also ensure the file is of ".map" type allows
> > test perf probe of function from different CU to pass when built with
> > NO_DWARF=1.
> > 
> > Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> > ---
> >  tools/perf/util/symbol.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 9e5940b5bc59..fbcc189edd8d 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1799,7 +1799,8 @@ int dso__load(struct dso *dso, struct map *map)
> >  	const char *map_path = dso__long_name(dso);
> >  
> >  	mutex_lock(dso__lock(dso));
> > -	perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0;
> > +	perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0 &&
> > +		  str_has_prefix(dso__name(dso), ".map") != 0;
> 
> Does this work? Shouldn't it be str_has_suffix()?

Yep, I think it should be

  perfmap = str_has_prefix(dso__name(dso), "/tmp/perf-") &&
            str_has_suffix(dso__name(dso), ".map"));

> 
> For reference, there's another fix for the same issue here:
> https://lore.kernel.org/linux-perf-users/20240617130332.13427-1-atrajeev@linux.vnet.ibm.com/T/#m9b1e62f0cdbcd4caf019f2ad97d69b29df568522

Right, it seems this patchset is posted earlier than the above.  But I
prefer Athira's fix as it touches less codes basically and contains more
fixes.

Thanks,
Namhyung


> 
> >  	if (perfmap) {
> >  		if (dso__nsinfo(dso) &&
> >  		    (dso__find_perf_map(newmapname, sizeof(newmapname),