* Re: [PATCH V2] perf tools: Fix maps__find_symbol_by_name()
2018-09-07 8:51 [PATCH V2] perf tools: Fix maps__find_symbol_by_name() Adrian Hunter
@ 2018-09-07 9:58 ` Jiri Olsa
2018-09-10 14:34 ` Arnaldo Carvalho de Melo
2018-09-12 19:22 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
2 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2018-09-07 9:58 UTC (permalink / raw)
To: Adrian Hunter
Cc: Arnaldo Carvalho de Melo, Björn Töpel, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V2] perf tools: Fix maps__find_symbol_by_name()
2018-09-07 8:51 [PATCH V2] perf tools: Fix maps__find_symbol_by_name() Adrian Hunter
2018-09-07 9:58 ` Jiri Olsa
@ 2018-09-10 14:34 ` Arnaldo Carvalho de Melo
2018-09-11 6:30 ` Adrian Hunter
2018-09-12 19:22 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
2 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-09-10 14:34 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Jiri Olsa, Björn Töpel, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V2] perf tools: Fix maps__find_symbol_by_name()
2018-09-10 14:34 ` Arnaldo Carvalho de Melo
@ 2018-09-11 6:30 ` Adrian Hunter
0 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2018-09-11 6:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Björn Töpel, linux-kernel
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/urgent] perf tools: Fix maps__find_symbol_by_name()
2018-09-07 8:51 [PATCH V2] perf tools: Fix maps__find_symbol_by_name() Adrian Hunter
2018-09-07 9:58 ` Jiri Olsa
2018-09-10 14:34 ` Arnaldo Carvalho de Melo
@ 2018-09-12 19:22 ` tip-bot for Adrian Hunter
2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Adrian Hunter @ 2018-09-12 19:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, acme, tglx, linux-kernel, bjorn.topel, jolsa, hpa, 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;
^ permalink raw reply related [flat|nested] 5+ messages in thread