2018-10-09 22:21:19

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf report segfault

On Tue, Oct 09, 2018 at 04:47:31PM -0500, Anthony LaTorre wrote:
> I can try building perf from the latest sources. I've attached the
> perf.data and perf.data.tar.bz2 from the test program I sent earlier.

cool, reproduced.. it seems to get introduced by:
2a9d5050dc84 perf script: Show correct offsets for DWARF-based unwinding

reverting that patch fixes the issue for me, but looks like
we could just make th attached check and prevent the crash

adding Sandipan Das to the loop, the author of that commit, any idea?

thanks,
jirka


---
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c4acd2001db0..ea68c805c7ac 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2312,7 +2312,6 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
{
struct callchain_cursor *cursor = arg;
const char *srcline = NULL;
- u64 addr;

if (symbol_conf.hide_unresolved && entry->sym == NULL)
return 0;
@@ -2320,13 +2319,15 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
if (append_inlines(cursor, entry->map, entry->sym, entry->ip) == 0)
return 0;

- /*
- * Convert entry->ip from a virtual address to an offset in
- * its corresponding binary.
- */
- addr = map__map_ip(entry->map, entry->ip);
+ if (entry->map) {
+ /*
+ * Convert entry->ip from a virtual address to an offset in
+ * its corresponding binary.
+ */
+ u64 addr = map__map_ip(entry->map, entry->ip);
+ srcline = callchain_srcline(entry->map, entry->sym, addr);
+ }

- srcline = callchain_srcline(entry->map, entry->sym, addr);
return callchain_cursor_append(cursor, entry->ip,
entry->map, entry->sym,
false, NULL, 0, 0, 0, srcline);


2018-10-10 05:45:04

by Sandipan Das

[permalink] [raw]
Subject: Re: perf report segfault

Hi Jiri,

Yes, this happens when entry->map is NULL. While your fix seems correct, the
following commit from Milian Wolff had already addressed this. I think this
was pulled in with one of Arnaldo's recent perf/urgent updates.

ff4ce2885af8 ("perf report: Don't try to map ip to invalid map")

Adding Milian to the loop as well.

With Regards,
Sandipan

On 10/10/18 3:50 AM, Jiri Olsa wrote:
> On Tue, Oct 09, 2018 at 04:47:31PM -0500, Anthony LaTorre wrote:
>> I can try building perf from the latest sources. I've attached the
>> perf.data and perf.data.tar.bz2 from the test program I sent earlier.
>
> cool, reproduced.. it seems to get introduced by:
> 2a9d5050dc84 perf script: Show correct offsets for DWARF-based unwinding
>
> reverting that patch fixes the issue for me, but looks like
> we could just make th attached check and prevent the crash
>
> adding Sandipan Das to the loop, the author of that commit, any idea?
>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index c4acd2001db0..ea68c805c7ac 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2312,7 +2312,6 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> {
> struct callchain_cursor *cursor = arg;
> const char *srcline = NULL;
> - u64 addr;
>
> if (symbol_conf.hide_unresolved && entry->sym == NULL)
> return 0;
> @@ -2320,13 +2319,15 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> if (append_inlines(cursor, entry->map, entry->sym, entry->ip) == 0)
> return 0;
>
> - /*
> - * Convert entry->ip from a virtual address to an offset in
> - * its corresponding binary.
> - */
> - addr = map__map_ip(entry->map, entry->ip);
> + if (entry->map) {
> + /*
> + * Convert entry->ip from a virtual address to an offset in
> + * its corresponding binary.
> + */
> + u64 addr = map__map_ip(entry->map, entry->ip);
> + srcline = callchain_srcline(entry->map, entry->sym, addr);
> + }
>
> - srcline = callchain_srcline(entry->map, entry->sym, addr);
> return callchain_cursor_append(cursor, entry->ip,
> entry->map, entry->sym,
> false, NULL, 0, 0, 0, srcline);
>


2018-10-10 06:20:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: perf report segfault


* Sandipan Das <[email protected]> wrote:

> Hi Jiri,
>
> Yes, this happens when entry->map is NULL. While your fix seems correct, the
> following commit from Milian Wolff had already addressed this. I think this
> was pulled in with one of Arnaldo's recent perf/urgent updates.
>
> ff4ce2885af8 ("perf report: Don't try to map ip to invalid map")

Ok, I'll send the pending perf/urgent tooling bits to Greg/Linus later today,
to make sure this is fixed upstream as well.

Thanks,

Ingo

2018-10-10 07:15:58

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf report segfault

On Wed, Oct 10, 2018 at 11:13:47AM +0530, Sandipan Das wrote:
> Hi Jiri,
>
> Yes, this happens when entry->map is NULL. While your fix seems correct, the
> following commit from Milian Wolff had already addressed this. I think this
> was pulled in with one of Arnaldo's recent perf/urgent updates.
>
> ff4ce2885af8 ("perf report: Don't try to map ip to invalid map")
>
> Adding Milian to the loop as well.

yea.. I knew I saw that code just recently ;-)

thanks,
jirka

>
> With Regards,
> Sandipan
>
> On 10/10/18 3:50 AM, Jiri Olsa wrote:
> > On Tue, Oct 09, 2018 at 04:47:31PM -0500, Anthony LaTorre wrote:
> >> I can try building perf from the latest sources. I've attached the
> >> perf.data and perf.data.tar.bz2 from the test program I sent earlier.
> >
> > cool, reproduced.. it seems to get introduced by:
> > 2a9d5050dc84 perf script: Show correct offsets for DWARF-based unwinding
> >
> > reverting that patch fixes the issue for me, but looks like
> > we could just make th attached check and prevent the crash
> >
> > adding Sandipan Das to the loop, the author of that commit, any idea?
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index c4acd2001db0..ea68c805c7ac 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -2312,7 +2312,6 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> > {
> > struct callchain_cursor *cursor = arg;
> > const char *srcline = NULL;
> > - u64 addr;
> >
> > if (symbol_conf.hide_unresolved && entry->sym == NULL)
> > return 0;
> > @@ -2320,13 +2319,15 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> > if (append_inlines(cursor, entry->map, entry->sym, entry->ip) == 0)
> > return 0;
> >
> > - /*
> > - * Convert entry->ip from a virtual address to an offset in
> > - * its corresponding binary.
> > - */
> > - addr = map__map_ip(entry->map, entry->ip);
> > + if (entry->map) {
> > + /*
> > + * Convert entry->ip from a virtual address to an offset in
> > + * its corresponding binary.
> > + */
> > + u64 addr = map__map_ip(entry->map, entry->ip);
> > + srcline = callchain_srcline(entry->map, entry->sym, addr);
> > + }
> >
> > - srcline = callchain_srcline(entry->map, entry->sym, addr);
> > return callchain_cursor_append(cursor, entry->ip,
> > entry->map, entry->sym,
> > false, NULL, 0, 0, 0, srcline);
> >
>

2018-10-10 12:55:03

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf report segfault

Em Wed, Oct 10, 2018 at 12:20:04AM +0200, Jiri Olsa escreveu:
> On Tue, Oct 09, 2018 at 04:47:31PM -0500, Anthony LaTorre wrote:
> > I can try building perf from the latest sources. I've attached the
> > perf.data and perf.data.tar.bz2 from the test program I sent earlier.
>
> cool, reproduced.. it seems to get introduced by:
> 2a9d5050dc84 perf script: Show correct offsets for DWARF-based unwinding
>
> reverting that patch fixes the issue for me, but looks like
> we could just make th attached check and prevent the crash
>
> adding Sandipan Das to the loop, the author of that commit, any idea?

commit ff4ce2885af8f9e8e99864d78dbeb4673f089c76
Author: Milian Wolff <[email protected]>
Date: Wed Sep 26 15:52:05 2018 +0200

perf report: Don't try to map ip to invalid map

Fixes a crash when the report encounters an address that could not be
associated with an mmaped region:

#0 0x00005555557bdc4a in callchain_srcline (ip=<error reading variable: Cannot access memory at address 0x38>, sym=0x0, map=0x0) at util/machine.c:2329
#1 unwind_entry (entry=entry@entry=0x7fffffff9180, arg=arg@entry=0x7ffff5642498) at util/machine.c:2329
#2 0x00005555558370af in entry (arg=0x7ffff5642498, cb=0x5555557bdb50 <unwind_entry>, thread=<optimized out>, ip=18446744073709551615) at util/unwind-libunwind-local.c:586
#3 get_entries (ui=ui@entry=0x7fffffff9620, cb=0x5555557bdb50 <unwind_entry>, arg=0x7ffff5642498, max_stack=<optimized out>) at util/unwind-libunwind-local.c:703
#4 0x0000555555837192 in _unwind__get_entries (cb=<optimized out>, arg=<optimized out>, thread=<optimized out>, data=<optimized out>, max_stack=<optimized out>) at util/unwind-libunwind-
local.c:725
#5 0x00005555557c310f in thread__resolve_callchain_unwind (max_stack=127, sample=0x7fffffff9830, evsel=0x555555c7b3b0, cursor=0x7ffff5642498, thread=0x555555c7f6f0) at util/machine.c:235
1
#6 thread__resolve_callchain (thread=0x555555c7f6f0, cursor=0x7ffff5642498, evsel=0x555555c7b3b0, sample=0x7fffffff9830, parent=0x7fffffff97b8, root_al=0x7fffffff9750, max_stack=127) at
util/machine.c:2378
#7 0x00005555557ba4ee in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=parent@entry=0x7fffffff97b8, evsel=<optimized out>, al=al@entry=0x7fffffff9750,
max_stack=<optimized out>) at util/callchain.c:1085

Signed-off-by: Milian Wolff <[email protected]>
Tested-by: Sandipan Das <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Jin Yao <[email protected]>
Cc: Namhyung Kim <[email protected]>
Fixes: 2a9d5050dc84 ("perf script: Show correct offsets for DWARF-based unwinding")
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>