[PATCH 1/3] perf report: don't try to map ip to invalid map

[PATCH 1/3] perf report: don't try to map ip to invalid map

[Milian Wolff]

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:2351
#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 <milian.wolff@kdab.com>
Cc: Sandipan Das <sandipan@linux.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
 tools/perf/util/machine.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c4acd2001db0..0cb4f8bf3ca7 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2312,7 +2312,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 {
 	struct callchain_cursor *cursor = arg;
 	const char *srcline = NULL;
-	u64 addr;
+	u64 addr = entry->ip;
 
 	if (symbol_conf.hide_unresolved && entry->sym == NULL)
 		return 0;
@@ -2324,7 +2324,8 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	 * 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)
+		addr = map__map_ip(entry->map, entry->ip);
 
 	srcline = callchain_srcline(entry->map, entry->sym, addr);
 	return callchain_cursor_append(cursor, entry->ip,
-- 
2.19.0

[PATCH 2/3] perf report: use the offset address to find inline frames

[Milian Wolff]

To correctly find inlined frames, we have to use the file offset
instead of the virtual memory address. This was already fixed for
displaying srcline information while displaying in commit
2a9d5050dc84fa20 ("perf script: Show correct offsets for DWARF-based
unwinding"). We just need to use the same corrected address also when
trying to find inline frames.

This is another follow-up to commit 19610184693c ("perf script: Show
virtual addresses instead of offsets").

Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
Cc: Sandipan Das <sandipan@linux.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
 tools/perf/util/machine.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0cb4f8bf3ca7..73a651f10a0f 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2317,9 +2317,6 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	if (symbol_conf.hide_unresolved && entry->sym == NULL)
 		return 0;
 
-	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.
@@ -2327,6 +2324,9 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	if (entry->map)
 		addr = map__map_ip(entry->map, entry->ip);
 
+	if (append_inlines(cursor, entry->map, entry->sym, addr) == 0)
+		return 0;
+
 	srcline = callchain_srcline(entry->map, entry->sym, addr);
 	return callchain_cursor_append(cursor, entry->ip,
 				       entry->map, entry->sym,
-- 
2.19.0

[PATCH 3/3] perf report: don't crash on invalid inline debug information

[Milian Wolff]

When the function name for an inline frame is invalid, we must
not try to demangle this symbol, otherwise we crash with:

#0  0x0000555555895c01 in bfd_demangle ()
#1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90, elf_name=0x0, kmodule=0) at util/symbol-elf.c:215
#2  dso__demangle_sym (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>, kmodule@entry=0, elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400
#3  0x00005555557fef4b in new_inline_sym (funcname=0x0, base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89
#4  inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00, node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at util/srcline.c:264
#5  0x00005555557ff27f in addr2line (dso_name=dso_name@entry=0x555555d92430 "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/elf", addr=addr@entry=2888, file=file@entry=0x0,
    line=line@entry=0x0, dso=dso@entry=0x555555c7bb00, unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810, sym=0x555555d92b90) at util/srcline.c:313
#6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90, dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430 "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/elf")
    at util/srcline.c:358

So instead handle the case where we get invalid function names
for inlined frames and use a fallback '??' function name instead.

While this crash was originally reported by Hadrien for rust code,
I can now also reproduce it with trivial C++ code. Indeed, it seems
like libbfd fails to interpret the debug information for the inline
frame symbol name:

$ addr2line -e /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/elf -if b48
main
/usr/include/c++/8.2.1/complex:610
??
/usr/include/c++/8.2.1/complex:618
??
/usr/include/c++/8.2.1/complex:675
??
/usr/include/c++/8.2.1/complex:685
main
/home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39

I've reported this bug upstream and also attached a patch there
which should fix this issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=23715

Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Reported-by: Hadrien Grasland <grasland@lal.in2p3.fr>
---
 tools/perf/util/srcline.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index 09d6746e6ec8..e767c4a9d4d2 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -85,6 +85,9 @@ static struct symbol *new_inline_sym(struct dso *dso,
 	struct symbol *inline_sym;
 	char *demangled = NULL;
 
+	if (!funcname)
+		funcname = "??";
+
 	if (dso) {
 		demangled = dso__demangle_sym(dso, 0, funcname);
 		if (demangled)
-- 
2.19.0

Re: [PATCH 1/3] perf report: don't try to map ip to invalid map

[Arnaldo Carvalho de Melo]

Em Wed, Sep 26, 2018 at 03:52:05PM +0200, Milian Wolff escreveu:
> Fixes a crash when the report encounters an address that
> could not be associated with an mmaped region:

Milian, can you spot which cset introduced this problem? So that we can
add a "Fixes: sha" tag in this (and the others, if needed) to help the
stable kernel maintainers to find which kernels this has to be
backported to?

Thanks for working on this!

- Arnaldo
 
> #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:2351
> #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 <milian.wolff@kdab.com>
> Cc: Sandipan Das <sandipan@linux.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> ---
>  tools/perf/util/machine.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index c4acd2001db0..0cb4f8bf3ca7 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2312,7 +2312,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
>  {
>  	struct callchain_cursor *cursor = arg;
>  	const char *srcline = NULL;
> -	u64 addr;
> +	u64 addr = entry->ip;
>  
>  	if (symbol_conf.hide_unresolved && entry->sym == NULL)
>  		return 0;
> @@ -2324,7 +2324,8 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
>  	 * 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)
> +		addr = map__map_ip(entry->map, entry->ip);
>  
>  	srcline = callchain_srcline(entry->map, entry->sym, addr);
>  	return callchain_cursor_append(cursor, entry->ip,
> -- 
> 2.19.0

Re: [PATCH 1/3] perf report: don't try to map ip to invalid map

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

On Wednesday, September 26, 2018 4:18:19 PM CEST Arnaldo Carvalho de Melo 
wrote:
> Em Wed, Sep 26, 2018 at 03:52:05PM +0200, Milian Wolff escreveu:
> > Fixes a crash when the report encounters an address that
> 
> > could not be associated with an mmaped region:
> Milian, can you spot which cset introduced this problem? So that we can
> add a "Fixes: sha" tag in this (and the others, if needed) to help the
> stable kernel maintainers to find which kernels this has to be
> backported to?

The issue was introduced by

perf script: Show correct offsets for DWARF-based unwinding

This in turn got backported already a few times, at which point the 
2a9d5050dc84fa2060f08a52f632976923e0fa7e sha was used when referencing the 
"Upstream commit".

Is that enough, or do you need me to find all the backported shas too?
-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH 1/3] perf report: don't try to map ip to invalid map

[Sandipan Das]

On 26/09/18 7:22 PM, Milian Wolff wrote:
> 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:2351
> #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
> 

Tested-by: Sandipan Das <sandipan@linux.ibm.com>

Re: [PATCH 2/3] perf report: use the offset address to find inline frames

[Jiri Olsa]

On Wed, Sep 26, 2018 at 03:52:06PM +0200, Milian Wolff wrote:
> To correctly find inlined frames, we have to use the file offset
> instead of the virtual memory address. This was already fixed for
> displaying srcline information while displaying in commit
> 2a9d5050dc84fa20 ("perf script: Show correct offsets for DWARF-based
> unwinding"). We just need to use the same corrected address also when
> trying to find inline frames.
> 
> This is another follow-up to commit 19610184693c ("perf script: Show
> virtual addresses instead of offsets").
> 
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> Cc: Sandipan Das <sandipan@linux.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

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

thanks,
jirka

Re: [PATCH 1/3] perf report: don't try to map ip to invalid map

[Jiri Olsa]

On Wed, Sep 26, 2018 at 03:52:05PM +0200, Milian Wolff wrote:
> 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:2351
> #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 <milian.wolff@kdab.com>
> Cc: Sandipan Das <sandipan@linux.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

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

thanks,
jirka

Re: [PATCH 1/3] perf report: don't try to map ip to invalid map

[Arnaldo Carvalho de Melo]

Em Wed, Sep 26, 2018 at 04:41:23PM +0200, Milian Wolff escreveu:
> On Wednesday, September 26, 2018 4:18:19 PM CEST Arnaldo Carvalho de Melo 
> wrote:
> > Em Wed, Sep 26, 2018 at 03:52:05PM +0200, Milian Wolff escreveu:
> > > Fixes a crash when the report encounters an address that
> > 
> > > could not be associated with an mmaped region:
> > Milian, can you spot which cset introduced this problem? So that we can
> > add a "Fixes: sha" tag in this (and the others, if needed) to help the
> > stable kernel maintainers to find which kernels this has to be
> > backported to?
> 
> The issue was introduced by
> 
> perf script: Show correct offsets for DWARF-based unwinding
> 
> This in turn got backported already a few times, at which point the 
> 2a9d5050dc84fa2060f08a52f632976923e0fa7e sha was used when referencing the 
> "Upstream commit".
> 
> Is that enough, or do you need me to find all the backported shas too?

I think it is enough, and I hope that the stable guys already handle
fixes to fixes :-)

- Arnaldo

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Arnaldo Carvalho de Melo]

Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> When the function name for an inline frame is invalid, we must
> not try to demangle this symbol, otherwise we crash with:
> 
> #0  0x0000555555895c01 in bfd_demangle ()
> #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90, elf_name=0x0, kmodule=0) at util/symbol-elf.c:215
> #2  dso__demangle_sym (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>, kmodule@entry=0, elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400
> #3  0x00005555557fef4b in new_inline_sym (funcname=0x0, base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89
> #4  inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00, node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at util/srcline.c:264
> #5  0x00005555557ff27f in addr2line (dso_name=dso_name@entry=0x555555d92430 "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/elf", addr=addr@entry=2888, file=file@entry=0x0,
>     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00, unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810, sym=0x555555d92b90) at util/srcline.c:313
> #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90, dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430 "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/elf")
>     at util/srcline.c:358
> 
> So instead handle the case where we get invalid function names
> for inlined frames and use a fallback '??' function name instead.
> 
> While this crash was originally reported by Hadrien for rust code,
> I can now also reproduce it with trivial C++ code. Indeed, it seems
> like libbfd fails to interpret the debug information for the inline
> frame symbol name:
> 
> $ addr2line -e /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/elf -if b48
> main
> /usr/include/c++/8.2.1/complex:610
> ??
> /usr/include/c++/8.2.1/complex:618
> ??
> /usr/include/c++/8.2.1/complex:675
> ??
> /usr/include/c++/8.2.1/complex:685
> main
> /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39
> 
> I've reported this bug upstream and also attached a patch there
> which should fix this issue:
> https://sourceware.org/bugzilla/show_bug.cgi?id=23715

Millian, what about this one, which is the cset it is fixing?

- Arnaldo
 
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Reported-by: Hadrien Grasland <grasland@lal.in2p3.fr>
> ---
>  tools/perf/util/srcline.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
> index 09d6746e6ec8..e767c4a9d4d2 100644
> --- a/tools/perf/util/srcline.c
> +++ b/tools/perf/util/srcline.c
> @@ -85,6 +85,9 @@ static struct symbol *new_inline_sym(struct dso *dso,
>  	struct symbol *inline_sym;
>  	char *demangled = NULL;
>  
> +	if (!funcname)
> +		funcname = "??";
> +
>  	if (dso) {
>  		demangled = dso__demangle_sym(dso, 0, funcname);
>  		if (demangled)
> -- 
> 2.19.0

Re: [PATCH 1/3] perf report: don't try to map ip to invalid map

[Arnaldo Carvalho de Melo]

Em Thu, Sep 27, 2018 at 02:18:47PM +0530, Sandipan Das escreveu:
> 
> 
> On 26/09/18 7:22 PM, Milian Wolff wrote:
> > 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:2351
> > #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
> > 
> 
> Tested-by: Sandipan Das <sandipan@linux.ibm.com>

Thanks, applied.

- Arnaldo

Re: [PATCH 2/3] perf report: use the offset address to find inline frames

[Arnaldo Carvalho de Melo]

Em Thu, Sep 27, 2018 at 06:00:42PM +0200, Jiri Olsa escreveu:
> On Wed, Sep 26, 2018 at 03:52:06PM +0200, Milian Wolff wrote:
> > To correctly find inlined frames, we have to use the file offset
> > instead of the virtual memory address. This was already fixed for
> > displaying srcline information while displaying in commit
> > 2a9d5050dc84fa20 ("perf script: Show correct offsets for DWARF-based
> > unwinding"). We just need to use the same corrected address also when
> > trying to find inline frames.
> > 
> > This is another follow-up to commit 19610184693c ("perf script: Show
> > virtual addresses instead of offsets").
> > 
> > Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> > Cc: Sandipan Das <sandipan@linux.ibm.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.

- Arnaldo

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]

On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de Melo 
wrote:
> Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > When the function name for an inline frame is invalid, we must
> > not try to demangle this symbol, otherwise we crash with:
> > 
> > #0  0x0000555555895c01 in bfd_demangle ()
> > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90, elf_name=0x0,
> > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>, kmodule@entry=0,
> > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3 
> > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89 #4 
> > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > (dso_name=dso_name@entry=0x555555d92430
> > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/
> > elf", addr=addr@entry=2888, file=file@entry=0x0,> 
> >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> >     unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810,
> >     sym=0x555555d92b90) at util/srcline.c:313> 
> > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/
> > elf")> 
> >     at util/srcline.c:358
> > 
> > So instead handle the case where we get invalid function names
> > for inlined frames and use a fallback '??' function name instead.
> > 
> > While this crash was originally reported by Hadrien for rust code,
> > I can now also reproduce it with trivial C++ code. Indeed, it seems
> > like libbfd fails to interpret the debug information for the inline
> > frame symbol name:
> > 
> > $ addr2line -e
> > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/e
> > lf -if b48 main
> > /usr/include/c++/8.2.1/complex:610
> > ??
> > /usr/include/c++/8.2.1/complex:618
> > ??
> > /usr/include/c++/8.2.1/complex:675
> > ??
> > /usr/include/c++/8.2.1/complex:685
> > main
> > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/mai
> > n.cpp:39
> > 
> > I've reported this bug upstream and also attached a patch there
> > which should fix this issue:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> 
> Millian, what about this one, which is the cset it is fixing?

Hey Arnaldo,

just noticed this email and that the corresponding patch hasn't landed in 
perf/core yet. The patch set which introduced this is a64489c56c307 ("perf 
report: Find the inline stack for a given address"). Note that the code was 
introduced by this patch, but then subsequently touched and moved by follow up 
patches. So, is this the patch you want to see referenced? Otherwise, the 
latest patch which gets fixed is afaik: 7285cf3325b4a ("perf srcline: Show 
correct function name for srcline of callchains").

Can you please pick either of these patches and amend the commit message of my 
patch and push it to perf/urgent and perf/core?

Cheers
-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Arnaldo Carvalho de Melo]

Em Thu, Oct 11, 2018 at 08:23:31PM +0200, Milian Wolff escreveu:
> On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de Melo 
> wrote:
> > Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > > When the function name for an inline frame is invalid, we must
> > > not try to demangle this symbol, otherwise we crash with:
> > > 
> > > #0  0x0000555555895c01 in bfd_demangle ()
> > > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90, elf_name=0x0,
> > > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>, kmodule@entry=0,
> > > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3 
> > > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > > base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89 #4 
> > > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > > (dso_name=dso_name@entry=0x555555d92430
> > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/
> > > elf", addr=addr@entry=2888, file=file@entry=0x0,> 
> > >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> > >     unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810,
> > >     sym=0x555555d92b90) at util/srcline.c:313> 
> > > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/
> > > elf")> 
> > >     at util/srcline.c:358
> > > 
> > > So instead handle the case where we get invalid function names
> > > for inlined frames and use a fallback '??' function name instead.
> > > 
> > > While this crash was originally reported by Hadrien for rust code,
> > > I can now also reproduce it with trivial C++ code. Indeed, it seems
> > > like libbfd fails to interpret the debug information for the inline
> > > frame symbol name:
> > > 
> > > $ addr2line -e
> > > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce5/e
> > > lf -if b48 main
> > > /usr/include/c++/8.2.1/complex:610
> > > ??
> > > /usr/include/c++/8.2.1/complex:618
> > > ??
> > > /usr/include/c++/8.2.1/complex:675
> > > ??
> > > /usr/include/c++/8.2.1/complex:685
> > > main
> > > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/mai
> > > n.cpp:39
> > > 
> > > I've reported this bug upstream and also attached a patch there
> > > which should fix this issue:
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> > 
> > Millian, what about this one, which is the cset it is fixing?
> 
> Hey Arnaldo,
> 
> just noticed this email and that the corresponding patch hasn't landed in 
> perf/core yet. The patch set which introduced this is a64489c56c307 ("perf 
> report: Find the inline stack for a given address"). Note that the code was 
> introduced by this patch, but then subsequently touched and moved by follow up 
> patches. So, is this the patch you want to see referenced? Otherwise, the 
> latest patch which gets fixed is afaik: 7285cf3325b4a ("perf srcline: Show 
> correct function name for srcline of callchains").
> 
> Can you please pick either of these patches and amend the commit message of my 
> patch and push it to perf/urgent and perf/core?

I'll reread all this later or tomorrow and continue, going AFK now.

- Arnaldo

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 3946 bytes --]

On Donnerstag, 11. Oktober 2018 21:39:20 CEST Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 11, 2018 at 08:23:31PM +0200, Milian Wolff escreveu:
> > On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de Melo
> > 
> > wrote:
> > > Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > > > When the function name for an inline frame is invalid, we must
> > > > not try to demangle this symbol, otherwise we crash with:
> > > > 
> > > > #0  0x0000555555895c01 in bfd_demangle ()
> > > > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90,
> > > > elf_name=0x0,
> > > > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > > > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>,
> > > > kmodule@entry=0,
> > > > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3
> > > > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > > > base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89 #4
> > > > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > > > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > > > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > > > (dso_name=dso_name@entry=0x555555d92430
> > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fc
> > > > e5/
> > > > elf", addr=addr@entry=2888, file=file@entry=0x0,>
> > > > 
> > > >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> > > >     unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810,
> > > >     sym=0x555555d92b90) at util/srcline.c:313>
> > > > 
> > > > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > > > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fc
> > > > e5/
> > > > elf")>
> > > > 
> > > >     at util/srcline.c:358
> > > > 
> > > > So instead handle the case where we get invalid function names
> > > > for inlined frames and use a fallback '??' function name instead.
> > > > 
> > > > While this crash was originally reported by Hadrien for rust code,
> > > > I can now also reproduce it with trivial C++ code. Indeed, it seems
> > > > like libbfd fails to interpret the debug information for the inline
> > > > frame symbol name:
> > > > 
> > > > $ addr2line -e
> > > > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce
> > > > 5/e
> > > > lf -if b48 main
> > > > /usr/include/c++/8.2.1/complex:610
> > > > ??
> > > > /usr/include/c++/8.2.1/complex:618
> > > > ??
> > > > /usr/include/c++/8.2.1/complex:675
> > > > ??
> > > > /usr/include/c++/8.2.1/complex:685
> > > > main
> > > > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining
> > > > /mai
> > > > n.cpp:39
> > > > 
> > > > I've reported this bug upstream and also attached a patch there
> > > > which should fix this issue:
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> > > 
> > > Millian, what about this one, which is the cset it is fixing?
> > 
> > Hey Arnaldo,
> > 
> > just noticed this email and that the corresponding patch hasn't landed in
> > perf/core yet. The patch set which introduced this is a64489c56c307 ("perf
> > report: Find the inline stack for a given address"). Note that the code
> > was
> > introduced by this patch, but then subsequently touched and moved by
> > follow up patches. So, is this the patch you want to see referenced?
> > Otherwise, the latest patch which gets fixed is afaik: 7285cf3325b4a
> > ("perf srcline: Show correct function name for srcline of callchains").
> > 
> > Can you please pick either of these patches and amend the commit message
> > of my patch and push it to perf/urgent and perf/core?
> 
> I'll reread all this later or tomorrow and continue, going AFK now.

Ping?

-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Arnaldo Carvalho de Melo]

Em Mon, Oct 15, 2018 at 10:51:36PM +0200, Milian Wolff escreveu:
> On Donnerstag, 11. Oktober 2018 21:39:20 CEST Arnaldo Carvalho de Melo wrote:
> > Em Thu, Oct 11, 2018 at 08:23:31PM +0200, Milian Wolff escreveu:
> > > On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de Melo
> > > 
> > > wrote:
> > > > Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > > > > When the function name for an inline frame is invalid, we must
> > > > > not try to demangle this symbol, otherwise we crash with:
> > > > > 
> > > > > #0  0x0000555555895c01 in bfd_demangle ()
> > > > > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90,
> > > > > elf_name=0x0,
> > > > > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > > > > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>,
> > > > > kmodule@entry=0,
> > > > > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3
> > > > > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > > > > base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89 #4
> > > > > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > > > > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > > > > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > > > > (dso_name=dso_name@entry=0x555555d92430
> > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fc
> > > > > e5/
> > > > > elf", addr=addr@entry=2888, file=file@entry=0x0,>
> > > > > 
> > > > >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> > > > >     unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810,
> > > > >     sym=0x555555d92b90) at util/srcline.c:313>
> > > > > 
> > > > > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > > > > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fc
> > > > > e5/
> > > > > elf")>
> > > > > 
> > > > >     at util/srcline.c:358
> > > > > 
> > > > > So instead handle the case where we get invalid function names
> > > > > for inlined frames and use a fallback '??' function name instead.
> > > > > 
> > > > > While this crash was originally reported by Hadrien for rust code,
> > > > > I can now also reproduce it with trivial C++ code. Indeed, it seems
> > > > > like libbfd fails to interpret the debug information for the inline
> > > > > frame symbol name:
> > > > > 
> > > > > $ addr2line -e
> > > > > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce
> > > > > 5/e
> > > > > lf -if b48 main
> > > > > /usr/include/c++/8.2.1/complex:610
> > > > > ??
> > > > > /usr/include/c++/8.2.1/complex:618
> > > > > ??
> > > > > /usr/include/c++/8.2.1/complex:675
> > > > > ??
> > > > > /usr/include/c++/8.2.1/complex:685
> > > > > main
> > > > > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining
> > > > > /mai
> > > > > n.cpp:39
> > > > > 
> > > > > I've reported this bug upstream and also attached a patch there
> > > > > which should fix this issue:
> > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> > > > 
> > > > Millian, what about this one, which is the cset it is fixing?
> > > 
> > > Hey Arnaldo,
> > > 
> > > just noticed this email and that the corresponding patch hasn't landed in
> > > perf/core yet. The patch set which introduced this is a64489c56c307 ("perf
> > > report: Find the inline stack for a given address"). Note that the code
> > > was
> > > introduced by this patch, but then subsequently touched and moved by
> > > follow up patches. So, is this the patch you want to see referenced?
> > > Otherwise, the latest patch which gets fixed is afaik: 7285cf3325b4a
> > > ("perf srcline: Show correct function name for srcline of callchains").
> > > 
> > > Can you please pick either of these patches and amend the commit message
> > > of my patch and push it to perf/urgent and perf/core?
> > 
> > I'll reread all this later or tomorrow and continue, going AFK now.
> 
> Ping?

Applied, seems simple enough, makes this code a bit more robust.

With regards to the cset where the problem originally was introduced,
i.e. not checking if a2l->funcname was NULL before either passing it to
strdup() or all the way to bfd_demangle(), that would cause the crash in
either place, I think this is the cset:

  commit a64489c56c307bf0955f0489158c5ecf6aa10fe2
  Author: Jin Yao <yao.jin@linux.intel.com>
  Date:   Sun Mar 26 04:34:26 2017 +0800

      perf report: Find the inline stack for a given address

Agreed?

- Arnaldo

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Arnaldo Carvalho de Melo]

Em Tue, Oct 16, 2018 at 02:49:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Oct 15, 2018 at 10:51:36PM +0200, Milian Wolff escreveu:
> > On Donnerstag, 11. Oktober 2018 21:39:20 CEST Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Oct 11, 2018 at 08:23:31PM +0200, Milian Wolff escreveu:
> > > > On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de Melo
> > > > 
> > > > wrote:
> > > > > Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > > > > > When the function name for an inline frame is invalid, we must
> > > > > > not try to demangle this symbol, otherwise we crash with:
> > > > > > 
> > > > > > #0  0x0000555555895c01 in bfd_demangle ()
> > > > > > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90,
> > > > > > elf_name=0x0,
> > > > > > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > > > > > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>,
> > > > > > kmodule@entry=0,
> > > > > > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3
> > > > > > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > > > > > base_sym=0x555555d92b90, dso=0x555555d92b90) at util/srcline.c:89 #4
> > > > > > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > > > > > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > > > > > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > > > > > (dso_name=dso_name@entry=0x555555d92430
> > > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fc
> > > > > > e5/
> > > > > > elf", addr=addr@entry=2888, file=file@entry=0x0,>
> > > > > > 
> > > > > >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> > > > > >     unwind_inlines=unwind_inlines@entry=true, node=0x555555e31810,
> > > > > >     sym=0x555555d92b90) at util/srcline.c:313>
> > > > > > 
> > > > > > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > > > > > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fc
> > > > > > e5/
> > > > > > elf")>
> > > > > > 
> > > > > >     at util/srcline.c:358
> > > > > > 
> > > > > > So instead handle the case where we get invalid function names
> > > > > > for inlined frames and use a fallback '??' function name instead.
> > > > > > 
> > > > > > While this crash was originally reported by Hadrien for rust code,
> > > > > > I can now also reproduce it with trivial C++ code. Indeed, it seems
> > > > > > like libbfd fails to interpret the debug information for the inline
> > > > > > frame symbol name:
> > > > > > 
> > > > > > $ addr2line -e
> > > > > > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033d24fce
> > > > > > 5/e
> > > > > > lf -if b48 main
> > > > > > /usr/include/c++/8.2.1/complex:610
> > > > > > ??
> > > > > > /usr/include/c++/8.2.1/complex:618
> > > > > > ??
> > > > > > /usr/include/c++/8.2.1/complex:675
> > > > > > ??
> > > > > > /usr/include/c++/8.2.1/complex:685
> > > > > > main
> > > > > > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining
> > > > > > /mai
> > > > > > n.cpp:39
> > > > > > 
> > > > > > I've reported this bug upstream and also attached a patch there
> > > > > > which should fix this issue:
> > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> > > > > 
> > > > > Millian, what about this one, which is the cset it is fixing?
> > > > 
> > > > Hey Arnaldo,
> > > > 
> > > > just noticed this email and that the corresponding patch hasn't landed in
> > > > perf/core yet. The patch set which introduced this is a64489c56c307 ("perf
> > > > report: Find the inline stack for a given address"). Note that the code
> > > > was
> > > > introduced by this patch, but then subsequently touched and moved by
> > > > follow up patches. So, is this the patch you want to see referenced?
> > > > Otherwise, the latest patch which gets fixed is afaik: 7285cf3325b4a
> > > > ("perf srcline: Show correct function name for srcline of callchains").
> > > > 
> > > > Can you please pick either of these patches and amend the commit message
> > > > of my patch and push it to perf/urgent and perf/core?
> > > 
> > > I'll reread all this later or tomorrow and continue, going AFK now.
> > 
> > Ping?
> 
> Applied, seems simple enough, makes this code a bit more robust.
> 
> With regards to the cset where the problem originally was introduced,
> i.e. not checking if a2l->funcname was NULL before either passing it to
> strdup() or all the way to bfd_demangle(), that would cause the crash in
> either place, I think this is the cset:
> 
>   commit a64489c56c307bf0955f0489158c5ecf6aa10fe2
>   Author: Jin Yao <yao.jin@linux.intel.com>
>   Date:   Sun Mar 26 04:34:26 2017 +0800
> 
>       perf report: Find the inline stack for a given address
> 
>> Agreed?

But I'm not sure this will be worth for doing backports, as before
applying this patch a series of other patches touching this code would
have to be applied :-\

I can leave it there, so that we know when the problem was introduced,
i.e. I _guess_ that if this rust or C++ reproducers would be used with
perf built with a64489c56c307bf0955f0489158c5ecf6aa10fe2 as head, we
would see a crash as well.

- Arnaldo

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 6018 bytes --]

On Dienstag, 16. Oktober 2018 19:52:04 CEST Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 16, 2018 at 02:49:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Oct 15, 2018 at 10:51:36PM +0200, Milian Wolff escreveu:
> > > On Donnerstag, 11. Oktober 2018 21:39:20 CEST Arnaldo Carvalho de Melo 
wrote:
> > > > Em Thu, Oct 11, 2018 at 08:23:31PM +0200, Milian Wolff escreveu:
> > > > > On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de
> > > > > Melo
> > > > > 
> > > > > wrote:
> > > > > > Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > > > > > > When the function name for an inline frame is invalid, we must
> > > > > > > not try to demangle this symbol, otherwise we crash with:
> > > > > > > 
> > > > > > > #0  0x0000555555895c01 in bfd_demangle ()
> > > > > > > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90,
> > > > > > > elf_name=0x0,
> > > > > > > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > > > > > > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>,
> > > > > > > kmodule@entry=0,
> > > > > > > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3
> > > > > > > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > > > > > > base_sym=0x555555d92b90, dso=0x555555d92b90) at
> > > > > > > util/srcline.c:89 #4
> > > > > > > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > > > > > > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > > > > > > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > > > > > > (dso_name=dso_name@entry=0x555555d92430
> > > > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da6603
> > > > > > > 3d24fc
> > > > > > > e5/
> > > > > > > elf", addr=addr@entry=2888, file=file@entry=0x0,>
> > > > > > > 
> > > > > > >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> > > > > > >     unwind_inlines=unwind_inlines@entry=true,
> > > > > > >     node=0x555555e31810,
> > > > > > >     sym=0x555555d92b90) at util/srcline.c:313>
> > > > > > > 
> > > > > > > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > > > > > > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > > > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da6603
> > > > > > > 3d24fc
> > > > > > > e5/
> > > > > > > elf")>
> > > > > > > 
> > > > > > >     at util/srcline.c:358
> > > > > > > 
> > > > > > > So instead handle the case where we get invalid function names
> > > > > > > for inlined frames and use a fallback '??' function name
> > > > > > > instead.
> > > > > > > 
> > > > > > > While this crash was originally reported by Hadrien for rust
> > > > > > > code,
> > > > > > > I can now also reproduce it with trivial C++ code. Indeed, it
> > > > > > > seems
> > > > > > > like libbfd fails to interpret the debug information for the
> > > > > > > inline
> > > > > > > frame symbol name:
> > > > > > > 
> > > > > > > $ addr2line -e
> > > > > > > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033
> > > > > > > d24fce
> > > > > > > 5/e
> > > > > > > lf -if b48 main
> > > > > > > /usr/include/c++/8.2.1/complex:610
> > > > > > > ??
> > > > > > > /usr/include/c++/8.2.1/complex:618
> > > > > > > ??
> > > > > > > /usr/include/c++/8.2.1/complex:675
> > > > > > > ??
> > > > > > > /usr/include/c++/8.2.1/complex:685
> > > > > > > main
> > > > > > > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-in
> > > > > > > lining
> > > > > > > /mai
> > > > > > > n.cpp:39
> > > > > > > 
> > > > > > > I've reported this bug upstream and also attached a patch there
> > > > > > > which should fix this issue:
> > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> > > > > > 
> > > > > > Millian, what about this one, which is the cset it is fixing?
> > > > > 
> > > > > Hey Arnaldo,
> > > > > 
> > > > > just noticed this email and that the corresponding patch hasn't
> > > > > landed in
> > > > > perf/core yet. The patch set which introduced this is a64489c56c307
> > > > > ("perf
> > > > > report: Find the inline stack for a given address"). Note that the
> > > > > code
> > > > > was
> > > > > introduced by this patch, but then subsequently touched and moved by
> > > > > follow up patches. So, is this the patch you want to see referenced?
> > > > > Otherwise, the latest patch which gets fixed is afaik: 7285cf3325b4a
> > > > > ("perf srcline: Show correct function name for srcline of
> > > > > callchains").
> > > > > 
> > > > > Can you please pick either of these patches and amend the commit
> > > > > message
> > > > > of my patch and push it to perf/urgent and perf/core?
> > > > 
> > > > I'll reread all this later or tomorrow and continue, going AFK now.
> > > 
> > > Ping?
> > 
> > Applied, seems simple enough, makes this code a bit more robust.
> > 
> > With regards to the cset where the problem originally was introduced,
> > i.e. not checking if a2l->funcname was NULL before either passing it to
> > strdup() or all the way to bfd_demangle(), that would cause the crash in
> > 
> > either place, I think this is the cset:
> >   commit a64489c56c307bf0955f0489158c5ecf6aa10fe2
> >   Author: Jin Yao <yao.jin@linux.intel.com>
> >   Date:   Sun Mar 26 04:34:26 2017 +0800
> >   
> >       perf report: Find the inline stack for a given address
> >> 
> >> Agreed?
> 
> But I'm not sure this will be worth for doing backports, as before
> applying this patch a series of other patches touching this code would
> have to be applied :-\
> 
> I can leave it there, so that we know when the problem was introduced,
> i.e. I _guess_ that if this rust or C++ reproducers would be used with
> perf built with a64489c56c307bf0955f0489158c5ecf6aa10fe2 as head, we
> would see a crash as well.

Yes, probably. And backporting this patch should be easily doable for anyone 
with a little C knowledge ;-)

Cheers

-- 
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information

[Arnaldo Carvalho de Melo]

Em Tue, Oct 16, 2018 at 09:00:48PM +0200, Milian Wolff escreveu:
> On Dienstag, 16. Oktober 2018 19:52:04 CEST Arnaldo Carvalho de Melo wrote:
> > Em Tue, Oct 16, 2018 at 02:49:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Mon, Oct 15, 2018 at 10:51:36PM +0200, Milian Wolff escreveu:
> > > > On Donnerstag, 11. Oktober 2018 21:39:20 CEST Arnaldo Carvalho de Melo 
> wrote:
> > > > > Em Thu, Oct 11, 2018 at 08:23:31PM +0200, Milian Wolff escreveu:
> > > > > > On Donnerstag, 27. September 2018 21:10:37 CEST Arnaldo Carvalho de
> > > > > > Melo
> > > > > > 
> > > > > > wrote:
> > > > > > > Em Wed, Sep 26, 2018 at 03:52:07PM +0200, Milian Wolff escreveu:
> > > > > > > > When the function name for an inline frame is invalid, we must
> > > > > > > > not try to demangle this symbol, otherwise we crash with:
> > > > > > > > 
> > > > > > > > #0  0x0000555555895c01 in bfd_demangle ()
> > > > > > > > #1  0x0000555555823262 in demangle_sym (dso=0x555555d92b90,
> > > > > > > > elf_name=0x0,
> > > > > > > > kmodule=0) at util/symbol-elf.c:215 #2  dso__demangle_sym
> > > > > > > > (dso=dso@entry=0x555555d92b90, kmodule=<optimized out>,
> > > > > > > > kmodule@entry=0,
> > > > > > > > elf_name=elf_name@entry=0x0) at util/symbol-elf.c:400 #3
> > > > > > > > 0x00005555557fef4b in new_inline_sym (funcname=0x0,
> > > > > > > > base_sym=0x555555d92b90, dso=0x555555d92b90) at
> > > > > > > > util/srcline.c:89 #4
> > > > > > > > inline_list__append_dso_a2l (dso=dso@entry=0x555555c7bb00,
> > > > > > > > node=node@entry=0x555555e31810, sym=sym@entry=0x555555d92b90) at
> > > > > > > > util/srcline.c:264 #5  0x00005555557ff27f in addr2line
> > > > > > > > (dso_name=dso_name@entry=0x555555d92430
> > > > > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da6603
> > > > > > > > 3d24fc
> > > > > > > > e5/
> > > > > > > > elf", addr=addr@entry=2888, file=file@entry=0x0,>
> > > > > > > > 
> > > > > > > >     line=line@entry=0x0, dso=dso@entry=0x555555c7bb00,
> > > > > > > >     unwind_inlines=unwind_inlines@entry=true,
> > > > > > > >     node=0x555555e31810,
> > > > > > > >     sym=0x555555d92b90) at util/srcline.c:313>
> > > > > > > > 
> > > > > > > > #6  0x00005555557ffe7c in addr2inlines (sym=0x555555d92b90,
> > > > > > > > dso=0x555555c7bb00, addr=2888, dso_name=0x555555d92430
> > > > > > > > "/home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da6603
> > > > > > > > 3d24fc
> > > > > > > > e5/
> > > > > > > > elf")>
> > > > > > > > 
> > > > > > > >     at util/srcline.c:358
> > > > > > > > 
> > > > > > > > So instead handle the case where we get invalid function names
> > > > > > > > for inlined frames and use a fallback '??' function name
> > > > > > > > instead.
> > > > > > > > 
> > > > > > > > While this crash was originally reported by Hadrien for rust
> > > > > > > > code,
> > > > > > > > I can now also reproduce it with trivial C++ code. Indeed, it
> > > > > > > > seems
> > > > > > > > like libbfd fails to interpret the debug information for the
> > > > > > > > inline
> > > > > > > > frame symbol name:
> > > > > > > > 
> > > > > > > > $ addr2line -e
> > > > > > > > /home/milian/.debug/.build-id/f7/186d14bb94f3c6161c010926da66033
> > > > > > > > d24fce
> > > > > > > > 5/e
> > > > > > > > lf -if b48 main
> > > > > > > > /usr/include/c++/8.2.1/complex:610
> > > > > > > > ??
> > > > > > > > /usr/include/c++/8.2.1/complex:618
> > > > > > > > ??
> > > > > > > > /usr/include/c++/8.2.1/complex:675
> > > > > > > > ??
> > > > > > > > /usr/include/c++/8.2.1/complex:685
> > > > > > > > main
> > > > > > > > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-in
> > > > > > > > lining
> > > > > > > > /mai
> > > > > > > > n.cpp:39
> > > > > > > > 
> > > > > > > > I've reported this bug upstream and also attached a patch there
> > > > > > > > which should fix this issue:
> > > > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=23715
> > > > > > > 
> > > > > > > Millian, what about this one, which is the cset it is fixing?
> > > > > > 
> > > > > > Hey Arnaldo,
> > > > > > 
> > > > > > just noticed this email and that the corresponding patch hasn't
> > > > > > landed in
> > > > > > perf/core yet. The patch set which introduced this is a64489c56c307
> > > > > > ("perf
> > > > > > report: Find the inline stack for a given address"). Note that the
> > > > > > code
> > > > > > was
> > > > > > introduced by this patch, but then subsequently touched and moved by
> > > > > > follow up patches. So, is this the patch you want to see referenced?
> > > > > > Otherwise, the latest patch which gets fixed is afaik: 7285cf3325b4a
> > > > > > ("perf srcline: Show correct function name for srcline of
> > > > > > callchains").
> > > > > > 
> > > > > > Can you please pick either of these patches and amend the commit
> > > > > > message
> > > > > > of my patch and push it to perf/urgent and perf/core?
> > > > > 
> > > > > I'll reread all this later or tomorrow and continue, going AFK now.
> > > > 
> > > > Ping?
> > > 
> > > Applied, seems simple enough, makes this code a bit more robust.
> > > 
> > > With regards to the cset where the problem originally was introduced,
> > > i.e. not checking if a2l->funcname was NULL before either passing it to
> > > strdup() or all the way to bfd_demangle(), that would cause the crash in
> > > 
> > > either place, I think this is the cset:
> > >   commit a64489c56c307bf0955f0489158c5ecf6aa10fe2
> > >   Author: Jin Yao <yao.jin@linux.intel.com>
> > >   Date:   Sun Mar 26 04:34:26 2017 +0800
> > >   
> > >       perf report: Find the inline stack for a given address
> > >> 
> > >> Agreed?
> > 
> > But I'm not sure this will be worth for doing backports, as before
> > applying this patch a series of other patches touching this code would
> > have to be applied :-\
> > 
> > I can leave it there, so that we know when the problem was introduced,
> > i.e. I _guess_ that if this rust or C++ reproducers would be used with
> > perf built with a64489c56c307bf0955f0489158c5ecf6aa10fe2 as head, we
> > would see a crash as well.
 
> Yes, probably. And backporting this patch should be easily doable for anyone 
> with a little C knowledge ;-)

This specific one, yes, I kept the Fixes: tag :-)

- Arnaldo