[PATCH V2] perf tools: Fix maps__find_symbol_by_name()

[PATCH V2] perf tools: Fix maps__find_symbol_by_name()

[Adrian Hunter]

Commit 1c5aae7710bb ("perf machine: Create maps for x86 PTI entry
trampolines") revealed a problem with maps__find_symbol_by_name() that
resulted in probes not being found e.g.

	$ sudo perf probe xsk_mmap
	xsk_mmap is out of .text, skip it.
	Probe point 'xsk_mmap' not found.
	   Error: Failed to add events.

maps__find_symbol_by_name() can optionally return the map of the found
symbol. It can get the map wrong because, in fact, the symbol is found
on the map's dso, not allowing for the possibility that the dso has more
than one map. Fix by always checking the map contains the symbol.

Reported-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Björn Töpel <bjorn.topel@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:

	Expanded commit message
	Corrected email address


 tools/perf/util/map.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 3f07a587c8e6..354e54550d2b 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -574,6 +574,13 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
 	return NULL;
 }
 
+static bool map__contains_symbol(struct map *map, struct symbol *sym)
+{
+	u64 ip = map->unmap_ip(map, sym->start);
+
+	return ip >= map->start && ip < map->end;
+}
+
 struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
 					 struct map **mapp)
 {
@@ -589,6 +596,10 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
 
 		if (sym == NULL)
 			continue;
+		if (!map__contains_symbol(pos, sym)) {
+			sym = NULL;
+			continue;
+		}
 		if (mapp != NULL)
 			*mapp = pos;
 		goto out;
-- 
2.17.1

Re: [PATCH V2] perf tools: Fix maps__find_symbol_by_name()

[Jiri Olsa]

On Fri, Sep 07, 2018 at 11:51:16AM +0300, Adrian Hunter wrote:
> Commit 1c5aae7710bb ("perf machine: Create maps for x86 PTI entry
> trampolines") revealed a problem with maps__find_symbol_by_name() that
> resulted in probes not being found e.g.
> 
> 	$ sudo perf probe xsk_mmap
> 	xsk_mmap is out of .text, skip it.
> 	Probe point 'xsk_mmap' not found.
> 	   Error: Failed to add events.
> 
> maps__find_symbol_by_name() can optionally return the map of the found
> symbol. It can get the map wrong because, in fact, the symbol is found
> on the map's dso, not allowing for the possibility that the dso has more
> than one map. Fix by always checking the map contains the symbol.
> 
> Reported-by: Björn Töpel <bjorn.topel@intel.com>
> Tested-by: Björn Töpel <bjorn.topel@intel.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

Re: [PATCH V2] perf tools: Fix maps__find_symbol_by_name()

[Arnaldo Carvalho de Melo]

Em Fri, Sep 07, 2018 at 11:51:16AM +0300, Adrian Hunter escreveu:
> Commit 1c5aae7710bb ("perf machine: Create maps for x86 PTI entry
> trampolines") revealed a problem with maps__find_symbol_by_name() that

Can we have this with a Fixes: 1c5aae7710bb?

So that that, combined with the CC: stable, tells which stable kernels
should get that fix, I think there are scripts harvesting Fixes: tags to
help stable maintainers :-)

- Arnaldo

> resulted in probes not being found e.g.
> 
> 	$ sudo perf probe xsk_mmap
> 	xsk_mmap is out of .text, skip it.
> 	Probe point 'xsk_mmap' not found.
> 	   Error: Failed to add events.
> 
> maps__find_symbol_by_name() can optionally return the map of the found
> symbol. It can get the map wrong because, in fact, the symbol is found
> on the map's dso, not allowing for the possibility that the dso has more
> than one map. Fix by always checking the map contains the symbol.
> 
> Reported-by: Björn Töpel <bjorn.topel@intel.com>
> Tested-by: Björn Töpel <bjorn.topel@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 
> 
> Changes in V2:
> 
> 	Expanded commit message
> 	Corrected email address
> 
> 
>  tools/perf/util/map.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 3f07a587c8e6..354e54550d2b 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -574,6 +574,13 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
>  	return NULL;
>  }
>  
> +static bool map__contains_symbol(struct map *map, struct symbol *sym)
> +{
> +	u64 ip = map->unmap_ip(map, sym->start);
> +
> +	return ip >= map->start && ip < map->end;
> +}
> +
>  struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
>  					 struct map **mapp)
>  {
> @@ -589,6 +596,10 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
>  
>  		if (sym == NULL)
>  			continue;
> +		if (!map__contains_symbol(pos, sym)) {
> +			sym = NULL;
> +			continue;
> +		}
>  		if (mapp != NULL)
>  			*mapp = pos;
>  		goto out;
> -- 
> 2.17.1

Re: [PATCH V2] perf tools: Fix maps__find_symbol_by_name()

[Adrian Hunter]

On 10/09/18 17:34, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 07, 2018 at 11:51:16AM +0300, Adrian Hunter escreveu:
>> Commit 1c5aae7710bb ("perf machine: Create maps for x86 PTI entry
>> trampolines") revealed a problem with maps__find_symbol_by_name() that
> 
> Can we have this with a Fixes: 1c5aae7710bb?
> 
> So that that, combined with the CC: stable, tells which stable kernels
> should get that fix, I think there are scripts harvesting Fixes: tags to
> help stable maintainers :-)

It seemed like potentially an existing issue so I did not want to limit how
far back it got applied.

> 
> - Arnaldo
> 
>> resulted in probes not being found e.g.
>>
>> 	$ sudo perf probe xsk_mmap
>> 	xsk_mmap is out of .text, skip it.
>> 	Probe point 'xsk_mmap' not found.
>> 	   Error: Failed to add events.
>>
>> maps__find_symbol_by_name() can optionally return the map of the found
>> symbol. It can get the map wrong because, in fact, the symbol is found
>> on the map's dso, not allowing for the possibility that the dso has more
>> than one map. Fix by always checking the map contains the symbol.
>>
>> Reported-by: Björn Töpel <bjorn.topel@intel.com>
>> Tested-by: Björn Töpel <bjorn.topel@intel.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>
>>
>> Changes in V2:
>>
>> 	Expanded commit message
>> 	Corrected email address
>>
>>
>>  tools/perf/util/map.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>> index 3f07a587c8e6..354e54550d2b 100644
>> --- a/tools/perf/util/map.c
>> +++ b/tools/perf/util/map.c
>> @@ -574,6 +574,13 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
>>  	return NULL;
>>  }
>>  
>> +static bool map__contains_symbol(struct map *map, struct symbol *sym)
>> +{
>> +	u64 ip = map->unmap_ip(map, sym->start);
>> +
>> +	return ip >= map->start && ip < map->end;
>> +}
>> +
>>  struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
>>  					 struct map **mapp)
>>  {
>> @@ -589,6 +596,10 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
>>  
>>  		if (sym == NULL)
>>  			continue;
>> +		if (!map__contains_symbol(pos, sym)) {
>> +			sym = NULL;
>> +			continue;
>> +		}
>>  		if (mapp != NULL)
>>  			*mapp = pos;
>>  		goto out;
>> -- 
>> 2.17.1
> 

[tip:perf/urgent] perf tools: Fix maps__find_symbol_by_name()

[tip-bot for Adrian Hunter]

Commit-ID:  03db8b583d1c3c84963e08e2abf6c79081da5c31
Gitweb:     https://git.kernel.org/tip/03db8b583d1c3c84963e08e2abf6c79081da5c31
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 7 Sep 2018 11:51:16 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 11 Sep 2018 14:12:51 -0300

perf tools: Fix maps__find_symbol_by_name()

Commit 1c5aae7710bb ("perf machine: Create maps for x86 PTI entry
trampolines") revealed a problem with maps__find_symbol_by_name() that
resulted in probes not being found e.g.

	$ sudo perf probe xsk_mmap
	xsk_mmap is out of .text, skip it.
	Probe point 'xsk_mmap' not found.
	   Error: Failed to add events.

maps__find_symbol_by_name() can optionally return the map of the found
symbol. It can get the map wrong because, in fact, the symbol is found
on the map's dso, not allowing for the possibility that the dso has more
than one map. Fix by always checking the map contains the symbol.

Reported-by: Björn Töpel <bjorn.topel@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Björn Töpel <bjorn.topel@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: stable@vger.kernel.org
Fixes: 1c5aae7710bb ("perf machine: Create maps for x86 PTI entry trampolines")
Link: http://lkml.kernel.org/r/20180907085116.25782-1-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 36d0763311ef..6a6929f208b4 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -576,6 +576,13 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
 	return NULL;
 }
 
+static bool map__contains_symbol(struct map *map, struct symbol *sym)
+{
+	u64 ip = map->unmap_ip(map, sym->start);
+
+	return ip >= map->start && ip < map->end;
+}
+
 struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
 					 struct map **mapp)
 {
@@ -591,6 +598,10 @@ struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name,
 
 		if (sym == NULL)
 			continue;
+		if (!map__contains_symbol(pos, sym)) {
+			sym = NULL;
+			continue;
+		}
 		if (mapp != NULL)
 			*mapp = pos;
 		goto out;