linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] perf report: don't try to map ip to invalid map
@ 2018-09-26 13:52 Milian Wolff
  2018-09-26 13:52 ` [PATCH 2/3] perf report: use the offset address to find inline frames Milian Wolff
                   ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: Milian Wolff @ 2018-09-26 13:52 UTC (permalink / raw)
  To: acme, jolsa, yao.jin, namhyung
  Cc: Linux-kernel, linux-perf-users, Milian Wolff, Sandipan Das

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/3] perf report: use the offset address to find inline frames
  2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
@ 2018-09-26 13:52 ` Milian Wolff
  2018-09-27 16:00   ` Jiri Olsa
  2018-10-02  7:39   ` [PATCH] perf record: use unmapped IP for inline callchain cursors Milian Wolff
  2018-09-26 13:52 ` [PATCH 3/3] perf report: don't crash on invalid inline debug information Milian Wolff
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: Milian Wolff @ 2018-09-26 13:52 UTC (permalink / raw)
  To: acme, jolsa, yao.jin, namhyung
  Cc: Linux-kernel, linux-perf-users, Milian Wolff, Sandipan Das

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
  2018-09-26 13:52 ` [PATCH 2/3] perf report: use the offset address to find inline frames Milian Wolff
@ 2018-09-26 13:52 ` Milian Wolff
  2018-09-27 19:10   ` Arnaldo Carvalho de Melo
  2018-10-18  6:20   ` [tip:perf/urgent] perf report: Don't " tip-bot for Milian Wolff
  2018-09-26 14:18 ` [PATCH 1/3] perf report: don't try to map ip to invalid map Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: Milian Wolff @ 2018-09-26 13:52 UTC (permalink / raw)
  To: acme, jolsa, yao.jin, namhyung
  Cc: Linux-kernel, linux-perf-users, 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] perf report: don't try to map ip to invalid map
  2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
  2018-09-26 13:52 ` [PATCH 2/3] perf report: use the offset address to find inline frames Milian Wolff
  2018-09-26 13:52 ` [PATCH 3/3] perf report: don't crash on invalid inline debug information Milian Wolff
@ 2018-09-26 14:18 ` Arnaldo Carvalho de Melo
  2018-09-26 14:41   ` Milian Wolff
  2018-09-27  8:48 ` Sandipan Das
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-09-26 14:18 UTC (permalink / raw)
  To: Milian Wolff
  Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users, Sandipan Das

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] perf report: don't try to map ip to invalid map
  2018-09-26 14:18 ` [PATCH 1/3] perf report: don't try to map ip to invalid map Arnaldo Carvalho de Melo
@ 2018-09-26 14:41   ` Milian Wolff
  2018-09-27 19:07     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 28+ messages in thread
From: Milian Wolff @ 2018-09-26 14:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users, Sandipan Das

[-- 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 --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] perf report: don't try to map ip to invalid map
  2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
                   ` (2 preceding siblings ...)
  2018-09-26 14:18 ` [PATCH 1/3] perf report: don't try to map ip to invalid map Arnaldo Carvalho de Melo
@ 2018-09-27  8:48 ` Sandipan Das
  2018-09-27 19:11   ` Arnaldo Carvalho de Melo
  2018-09-27 16:02 ` Jiri Olsa
  2018-10-05 16:20 ` [tip:perf/urgent] perf report: Don't " tip-bot for Milian Wolff
  5 siblings, 1 reply; 28+ messages in thread
From: Sandipan Das @ 2018-09-27  8:48 UTC (permalink / raw)
  To: Milian Wolff
  Cc: acme, jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users



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>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/3] perf report: use the offset address to find inline frames
  2018-09-26 13:52 ` [PATCH 2/3] perf report: use the offset address to find inline frames Milian Wolff
@ 2018-09-27 16:00   ` Jiri Olsa
  2018-09-27 19:12     ` Arnaldo Carvalho de Melo
  2018-10-02  7:39   ` [PATCH] perf record: use unmapped IP for inline callchain cursors Milian Wolff
  1 sibling, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2018-09-27 16:00 UTC (permalink / raw)
  To: Milian Wolff
  Cc: acme, jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users,
	Sandipan Das

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] perf report: don't try to map ip to invalid map
  2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
                   ` (3 preceding siblings ...)
  2018-09-27  8:48 ` Sandipan Das
@ 2018-09-27 16:02 ` Jiri Olsa
  2018-10-05 16:20 ` [tip:perf/urgent] perf report: Don't " tip-bot for Milian Wolff
  5 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2018-09-27 16:02 UTC (permalink / raw)
  To: Milian Wolff
  Cc: acme, jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users,
	Sandipan Das

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] perf report: don't try to map ip to invalid map
  2018-09-26 14:41   ` Milian Wolff
@ 2018-09-27 19:07     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-09-27 19:07 UTC (permalink / raw)
  To: Milian Wolff
  Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users, Sandipan Das

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-09-26 13:52 ` [PATCH 3/3] perf report: don't crash on invalid inline debug information Milian Wolff
@ 2018-09-27 19:10   ` Arnaldo Carvalho de Melo
  2018-10-11 18:23     ` Milian Wolff
  2018-10-18  6:20   ` [tip:perf/urgent] perf report: Don't " tip-bot for Milian Wolff
  1 sibling, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-09-27 19:10 UTC (permalink / raw)
  To: Milian Wolff; +Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] perf report: don't try to map ip to invalid map
  2018-09-27  8:48 ` Sandipan Das
@ 2018-09-27 19:11   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-09-27 19:11 UTC (permalink / raw)
  To: Sandipan Das
  Cc: Milian Wolff, jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/3] perf report: use the offset address to find inline frames
  2018-09-27 16:00   ` Jiri Olsa
@ 2018-09-27 19:12     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-09-27 19:12 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Milian Wolff, jolsa, yao.jin, namhyung, Linux-kernel,
	linux-perf-users, Sandipan Das

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] perf record: use unmapped IP for inline callchain cursors
@ 2018-10-02  7:39   ` Milian Wolff
  2018-10-02 15:32     ` Arnaldo Carvalho de Melo
                       ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Milian Wolff @ 2018-10-02  7:39 UTC (permalink / raw)
  To: acme, jolsa, Jin Yao; +Cc: Linux-kernel, linux-perf-users, Milian Wolff

Only use the mapped IP to find inline frames, but keep
using the unmapped IP for the callchain cursor. This
ensures we properly show the unmapped IP when displaying
a frame we received via the dso__parse_addr_inlines API
for a module which does not contain sufficient debug symbols
to show the srcline.

Before:
$ perf record -e cycles:u --call-graph ls
$ perf script
...
ls 12853  2735.563911:      43354 cycles:u:
                   17878 __GI___tunables_init+0xffff01d1d63a0118 (/usr/lib/ld-2.28.so)
                   19ee9 _dl_sysdep_start+0xffff01d1d63a02e9 (/usr/lib/ld-2.28.so)
                    3087 _dl_start+0xffff01d1d63a0287 (/usr/lib/ld-2.28.so)
                    2007 _start+0xffff01d1d63a0007 (/usr/lib/ld-2.28.so)

After:

$ perf script
...
ls 12853  2735.563911:      43354 cycles:u:
            7f1714e46878 __GI___tunables_init+0x118 (/usr/lib/ld-2.28.so)
            7f1714e48ee9 _dl_sysdep_start+0x2e9 (/usr/lib/ld-2.28.so)
            7f1714e32087 _dl_start+0x287 (/usr/lib/ld-2.28.so)
            7f1714e31007 _start+0x7 (/usr/lib/ld-2.28.so)

For frames with sufficient debug symbols, the behavior is
still sane and works as expected in my tests.

This patch series shows that we desperately need
an automated test for inline frame resolution. I'll try to
come up with something for the various regressions in the future.

Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Reported-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
# Tested-by:
# Reviewed-by:
# Suggested-b:
Fixes: bfe16b0653 ("perf report: Don't crash on invalid inline debug information")
---
 tools/perf/util/machine.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 73a651f10a0f..111ae858cbcb 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2286,7 +2286,8 @@ static int append_inlines(struct callchain_cursor *cursor,
 	if (!symbol_conf.inline_name || !map || !sym)
 		return ret;
 
-	addr = map__rip_2objdump(map, ip);
+	addr = map__map_ip(map, ip);
+	addr = map__rip_2objdump(map, addr);
 
 	inline_node = inlines__tree_find(&map->dso->inlined_nodes, addr);
 	if (!inline_node) {
@@ -2317,6 +2318,9 @@ 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.
@@ -2324,9 +2328,6 @@ 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] perf record: use unmapped IP for inline callchain cursors
  2018-10-02  7:39   ` [PATCH] perf record: use unmapped IP for inline callchain cursors Milian Wolff
@ 2018-10-02 15:32     ` Arnaldo Carvalho de Melo
  2018-10-03  3:35       ` Ravi Bangoria
  2018-10-05 14:11     ` Arnaldo Carvalho de Melo
  2018-10-05 16:21     ` [tip:perf/urgent] perf record: Use " tip-bot for Milian Wolff
  2 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 15:32 UTC (permalink / raw)
  To: Milian Wolff, Ravi Bangoria
  Cc: Jiri Olsa, Jin Yao, Linux-kernel, linux-perf-users

Em Tue, Oct 02, 2018 at 09:39:49AM +0200, Milian Wolff escreveu:
> Only use the mapped IP to find inline frames, but keep
> using the unmapped IP for the callchain cursor. This
> ensures we properly show the unmapped IP when displaying
> a frame we received via the dso__parse_addr_inlines API
> for a module which does not contain sufficient debug symbols
> to show the srcline.

So, the patch came mangled, I fixed it up, please check, I'm adding Ravi
to the CC list, so that he can check as well and retest, right Ravi?

- Arnaldo

From d9ddee9653d5676130471de9bc688fc7ec7b4fc4 Mon Sep 17 00:00:00 2001
From: Milian Wolff <milian.wolff@kdab.com>
Date: Tue, 2 Oct 2018 09:39:49 +0200
Subject: [PATCH 1/1] perf record: use unmapped IP for inline callchain cursors

Only use the mapped IP to find inline frames, but keep using the unmapped IP
for the callchain cursor. This ensures we properly show the unmapped IP when
displaying a frame we received via the dso__parse_addr_inlines API for a module
which does not contain sufficient debug symbols to show the srcline.

Before:

  $ perf record -e cycles:u --call-graph ls
  $ perf script
  ...
  ls 12853  2735.563911:      43354 cycles:u:
                     17878 __GI___tunables_init+0xffff01d1d63a0118 (/usr/lib/ld-2.28.so)
                     19ee9 _dl_sysdep_start+0xffff01d1d63a02e9 (/usr/lib/ld-2.28.so)
                      3087 _dl_start+0xffff01d1d63a0287 (/usr/lib/ld-2.28.so)
                      2007 _start+0xffff01d1d63a0007 (/usr/lib/ld-2.28.so)

After:

  $ perf script
  ...
  ls 12853  2735.563911:      43354 cycles:u:
              7f1714e46878 __GI___tunables_init+0x118 (/usr/lib/ld-2.28.so)
              7f1714e48ee9 _dl_sysdep_start+0x2e9 (/usr/lib/ld-2.28.so)
              7f1714e32087 _dl_start+0x287 (/usr/lib/ld-2.28.so)
              7f1714e31007 _start+0x7 (/usr/lib/ld-2.28.so)

For frames with sufficient debug symbols, the behavior is still sane and works
as expected in my tests.

This patch series shows that we desperately need an automated test for inline
frame resolution. I'll try to come up with something for the various
regressions in the future.

Reported-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Fixes: bfe16b0653 ("perf report: Don't crash on invalid inline debug information")
Link: http://lkml.kernel.org/r/20181002073949.3297-1-milian.wolff@kdab.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 73a651f10a0f..111ae858cbcb 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2286,7 +2286,8 @@ static int append_inlines(struct callchain_cursor *cursor,
 	if (!symbol_conf.inline_name || !map || !sym)
 		return ret;
 
-	addr = map__rip_2objdump(map, ip);
+	addr = map__map_ip(map, ip);
+	addr = map__rip_2objdump(map, addr);
 
 	inline_node = inlines__tree_find(&map->dso->inlined_nodes, addr);
 	if (!inline_node) {
@@ -2317,6 +2318,9 @@ 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.
@@ -2324,9 +2328,6 @@ 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.14.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] perf record: use unmapped IP for inline callchain cursors
  2018-10-02 15:32     ` Arnaldo Carvalho de Melo
@ 2018-10-03  3:35       ` Ravi Bangoria
  2018-10-05 13:48         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 28+ messages in thread
From: Ravi Bangoria @ 2018-10-03  3:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Milian Wolff
  Cc: Ravi Bangoria, Jiri Olsa, Jin Yao, Linux-kernel, linux-perf-users



On 10/02/2018 09:02 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 02, 2018 at 09:39:49AM +0200, Milian Wolff escreveu:
>> Only use the mapped IP to find inline frames, but keep
>> using the unmapped IP for the callchain cursor. This
>> ensures we properly show the unmapped IP when displaying
>> a frame we received via the dso__parse_addr_inlines API
>> for a module which does not contain sufficient debug symbols
>> to show the srcline.
> 
> So, the patch came mangled, I fixed it up, please check, I'm adding Ravi
> to the CC list, so that he can check as well and retest, right Ravi?
> 
> - Arnaldo
> 
> From d9ddee9653d5676130471de9bc688fc7ec7b4fc4 Mon Sep 17 00:00:00 2001
> From: Milian Wolff <milian.wolff@kdab.com>
> Date: Tue, 2 Oct 2018 09:39:49 +0200
> Subject: [PATCH 1/1] perf record: use unmapped IP for inline callchain cursors
> 
> Only use the mapped IP to find inline frames, but keep using the unmapped IP
> for the callchain cursor. This ensures we properly show the unmapped IP when
> displaying a frame we received via the dso__parse_addr_inlines API for a module
> which does not contain sufficient debug symbols to show the srcline.
> 
> Before:
> 
>   $ perf record -e cycles:u --call-graph ls
>   $ perf script
>   ...
>   ls 12853  2735.563911:      43354 cycles:u:
>                      17878 __GI___tunables_init+0xffff01d1d63a0118 (/usr/lib/ld-2.28.so)
>                      19ee9 _dl_sysdep_start+0xffff01d1d63a02e9 (/usr/lib/ld-2.28.so)
>                       3087 _dl_start+0xffff01d1d63a0287 (/usr/lib/ld-2.28.so)
>                       2007 _start+0xffff01d1d63a0007 (/usr/lib/ld-2.28.so)
> 
> After:
> 
>   $ perf script
>   ...
>   ls 12853  2735.563911:      43354 cycles:u:
>               7f1714e46878 __GI___tunables_init+0x118 (/usr/lib/ld-2.28.so)
>               7f1714e48ee9 _dl_sysdep_start+0x2e9 (/usr/lib/ld-2.28.so)
>               7f1714e32087 _dl_start+0x287 (/usr/lib/ld-2.28.so)
>               7f1714e31007 _start+0x7 (/usr/lib/ld-2.28.so)

With current perf/urgent:

  $ sudo ./perf record --call-graph=dwarf -e probe_libc:inet_pton -- ping -6 -c 1 ::1
  $ sudo ./perf script
  ping  4109 [011] 767269.031273: probe_libc:inet_pton: (7fff944cb458)
                  15b458 __inet_pton+0xffff0000d7920008 (inlined)
                  10feb3 gaih_inet.constprop.7+0xffff0000d7920f43 (/usr/lib64/libc-2.26.
                  110a13 __GI_getaddrinfo+0xffff0000d7920163 (inlined)
               13c752d6f _init+0xbfb (/usr/bin/ping)
                   2371f generic_start_main.isra.0+0xffff0000d792013f (/usr/lib64/libc-2
                   2391b __libc_start_main+0xffff0000d79200bb (/usr/lib64/libc-2.26.so)

With this patch:

  $ sudo ./perf script
  ping  4109 [011] 767269.031273: probe_libc:inet_pton: (7fff944cb458)
            7fff944cb458 __inet_pton+0x8 (inlined)
            7fff9447feb3 gaih_inet.constprop.7+0xf43 (/usr/lib64/libc-2.26.so)
            7fff94480a13 __GI_getaddrinfo+0x163 (inlined)
               13c752d6f _init+0xbfb (/usr/bin/ping)
            7fff9439371f generic_start_main.isra.0+0x13f (/usr/lib64/libc-2.26.so)
            7fff9439391b __libc_start_main+0xbb (/usr/lib64/libc-2.26.so)

LGTM.

Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] perf record: use unmapped IP for inline callchain cursors
  2018-10-03  3:35       ` Ravi Bangoria
@ 2018-10-05 13:48         ` Arnaldo Carvalho de Melo
  2018-10-08 18:49           ` Milian Wolff
  0 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-05 13:48 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: Milian Wolff, Jiri Olsa, Jin Yao, Linux-kernel, linux-perf-users

Em Wed, Oct 03, 2018 at 09:05:37AM +0530, Ravi Bangoria escreveu:
> LGTM.
 
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

So, I've added this as a 'git rebase -i' 'fixup', i.e. kept the commit
log message for the patch this patch fixes, and combined the two into
just one patch so that we don't pollute the bisect history, since this
hasn't made it yet to tip, and I also added Ravi's Tested-by, since this
tests both.

- Arnaldo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] perf record: use unmapped IP for inline callchain cursors
  2018-10-02  7:39   ` [PATCH] perf record: use unmapped IP for inline callchain cursors Milian Wolff
  2018-10-02 15:32     ` Arnaldo Carvalho de Melo
@ 2018-10-05 14:11     ` Arnaldo Carvalho de Melo
  2018-10-05 16:21     ` [tip:perf/urgent] perf record: Use " tip-bot for Milian Wolff
  2 siblings, 0 replies; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-05 14:11 UTC (permalink / raw)
  To: Milian Wolff; +Cc: jolsa, Jin Yao, Linux-kernel, linux-perf-users

Em Tue, Oct 02, 2018 at 09:39:49AM +0200, Milian Wolff escreveu:
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Reported-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> # Tested-by:
> # Reviewed-by:
> # Suggested-b:
> Fixes: bfe16b0653 ("perf report: Don't crash on invalid inline debug information")

No, the patch that Ravi pointed out as causing the regression wasn't the
one above, it was this one:

"perf report: Use the offset address to find inline frames"

I'm reworking this again...

- Arnaldo

> ---
>  tools/perf/util/machine.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 73a651f10a0f..111ae858cbcb 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2286,7 +2286,8 @@ static int append_inlines(struct callchain_cursor *cursor,
>  	if (!symbol_conf.inline_name || !map || !sym)
>  		return ret;
>  
> -	addr = map__rip_2objdump(map, ip);
> +	addr = map__map_ip(map, ip);
> +	addr = map__rip_2objdump(map, addr);
>  
>  	inline_node = inlines__tree_find(&map->dso->inlined_nodes, addr);
>  	if (!inline_node) {
> @@ -2317,6 +2318,9 @@ 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.
> @@ -2324,9 +2328,6 @@ 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [tip:perf/urgent] perf report: Don't try to map ip to invalid map
  2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
                   ` (4 preceding siblings ...)
  2018-09-27 16:02 ` Jiri Olsa
@ 2018-10-05 16:20 ` tip-bot for Milian Wolff
  5 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Milian Wolff @ 2018-10-05 16:20 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: milian.wolff, jolsa, mingo, linux-kernel, yao.jin, tglx, acme,
	hpa, sandipan, namhyung

Commit-ID:  ff4ce2885af8f9e8e99864d78dbeb4673f089c76
Gitweb:     https://git.kernel.org/tip/ff4ce2885af8f9e8e99864d78dbeb4673f089c76
Author:     Milian Wolff <milian.wolff@kdab.com>
AuthorDate: Wed, 26 Sep 2018 15:52:05 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 27 Sep 2018 16:05:43 -0300

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: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>
Tested-by: Sandipan Das <sandipan@linux.ibm.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Fixes: 2a9d5050dc84 ("perf script: Show correct offsets for DWARF-based unwinding")
Link: http://lkml.kernel.org/r/20180926135207.30263-1-milian.wolff@kdab.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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,

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [tip:perf/urgent] perf record: Use unmapped IP for inline callchain cursors
  2018-10-02  7:39   ` [PATCH] perf record: use unmapped IP for inline callchain cursors Milian Wolff
  2018-10-02 15:32     ` Arnaldo Carvalho de Melo
  2018-10-05 14:11     ` Arnaldo Carvalho de Melo
@ 2018-10-05 16:21     ` tip-bot for Milian Wolff
  2 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Milian Wolff @ 2018-10-05 16:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: ravi.bangoria, acme, milian.wolff, jolsa, linux-kernel, tglx,
	hpa, yao.jin, namhyung, sandipan, mingo

Commit-ID:  7a8a8fcf7b860e4b2d4edc787c844d41cad9dfcf
Gitweb:     https://git.kernel.org/tip/7a8a8fcf7b860e4b2d4edc787c844d41cad9dfcf
Author:     Milian Wolff <milian.wolff@kdab.com>
AuthorDate: Wed, 26 Sep 2018 15:52:06 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 5 Oct 2018 11:18:09 -0300

perf record: Use unmapped IP for inline callchain cursors

Only use the mapped IP to find inline frames, but keep using the
unmapped IP for the callchain cursor. This ensures we properly show the
unmapped IP when displaying a frame we received via the
dso__parse_addr_inlines API for a module which does not contain
sufficient debug symbols to show the srcline.

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>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Sandipan Das <sandipan@linux.ibm.com>
Fixes: 19610184693c ("perf script: Show virtual addresses instead of offsets")
Link: http://lkml.kernel.org/r/20180926135207.30263-2-milian.wolff@kdab.com
Link: http://lkml.kernel.org/r/20181002073949.3297-1-milian.wolff@kdab.com
[ Squashed a fix from Milian for a problem reported by Ravi, fixed up space damage ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0cb4f8bf3ca7..111ae858cbcb 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2286,7 +2286,8 @@ static int append_inlines(struct callchain_cursor *cursor,
 	if (!symbol_conf.inline_name || !map || !sym)
 		return ret;
 
-	addr = map__rip_2objdump(map, ip);
+	addr = map__map_ip(map, ip);
+	addr = map__rip_2objdump(map, addr);
 
 	inline_node = inlines__tree_find(&map->dso->inlined_nodes, addr);
 	if (!inline_node) {

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] perf record: use unmapped IP for inline callchain cursors
  2018-10-05 13:48         ` Arnaldo Carvalho de Melo
@ 2018-10-08 18:49           ` Milian Wolff
  0 siblings, 0 replies; 28+ messages in thread
From: Milian Wolff @ 2018-10-08 18:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ravi Bangoria, Jiri Olsa, Jin Yao, Linux-kernel, linux-perf-users

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

On Freitag, 5. Oktober 2018 15:48:31 CEST Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 03, 2018 at 09:05:37AM +0530, Ravi Bangoria escreveu:
> > LGTM.
> > 
> > Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> 
> So, I've added this as a 'git rebase -i' 'fixup', i.e. kept the commit
> log message for the patch this patch fixes, and combined the two into
> just one patch so that we don't pollute the bisect history, since this
> hasn't made it yet to tip, and I also added Ravi's Tested-by, since this
> tests both.

Thanks a lot for the cleanup work Arnaldo.

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 --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-09-27 19:10   ` Arnaldo Carvalho de Melo
@ 2018-10-11 18:23     ` Milian Wolff
  2018-10-11 19:39       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 28+ messages in thread
From: Milian Wolff @ 2018-10-11 18:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users

[-- 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 --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-10-11 18:23     ` Milian Wolff
@ 2018-10-11 19:39       ` Arnaldo Carvalho de Melo
  2018-10-15 20:51         ` Milian Wolff
  0 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-11 19:39 UTC (permalink / raw)
  To: Milian Wolff; +Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-10-11 19:39       ` Arnaldo Carvalho de Melo
@ 2018-10-15 20:51         ` Milian Wolff
  2018-10-16 17:49           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 28+ messages in thread
From: Milian Wolff @ 2018-10-15 20:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, yao.jin, namhyung, Linux-kernel, linux-perf-users

[-- 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 --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-10-15 20:51         ` Milian Wolff
@ 2018-10-16 17:49           ` Arnaldo Carvalho de Melo
  2018-10-16 17:52             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-16 17:49 UTC (permalink / raw)
  To: Milian Wolff
  Cc: Jiri Olsa, Namhyung Kim, Linux-kernel, linux-perf-users, Jin Yao

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-10-16 17:49           ` Arnaldo Carvalho de Melo
@ 2018-10-16 17:52             ` Arnaldo Carvalho de Melo
  2018-10-16 19:00               ` Milian Wolff
  0 siblings, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-16 17:52 UTC (permalink / raw)
  To: Milian Wolff
  Cc: Jiri Olsa, Namhyung Kim, Linux-kernel, linux-perf-users, Jin Yao

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-10-16 17:52             ` Arnaldo Carvalho de Melo
@ 2018-10-16 19:00               ` Milian Wolff
  2018-10-16 20:06                 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 28+ messages in thread
From: Milian Wolff @ 2018-10-16 19:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Linux-kernel, linux-perf-users, Jin Yao

[-- 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 --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] perf report: don't crash on invalid inline debug information
  2018-10-16 19:00               ` Milian Wolff
@ 2018-10-16 20:06                 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-16 20:06 UTC (permalink / raw)
  To: Milian Wolff
  Cc: Jiri Olsa, Namhyung Kim, Linux-kernel, linux-perf-users, Jin Yao

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [tip:perf/urgent] perf report: Don't crash on invalid inline debug information
  2018-09-26 13:52 ` [PATCH 3/3] perf report: don't crash on invalid inline debug information Milian Wolff
  2018-09-27 19:10   ` Arnaldo Carvalho de Melo
@ 2018-10-18  6:20   ` tip-bot for Milian Wolff
  1 sibling, 0 replies; 28+ messages in thread
From: tip-bot for Milian Wolff @ 2018-10-18  6:20 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, grasland, acme, yao.jin, namhyung, tglx, mingo, jolsa,
	milian.wolff, linux-kernel

Commit-ID:  d4046e8e17b9f378cb861982ef71c63911b5dff3
Gitweb:     https://git.kernel.org/tip/d4046e8e17b9f378cb861982ef71c63911b5dff3
Author:     Milian Wolff <milian.wolff@kdab.com>
AuthorDate: Wed, 26 Sep 2018 15:52:07 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 16 Oct 2018 14:52:21 -0300

perf report: Don't crash on invalid inline debug information

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

Reported-by: Hadrien Grasland <grasland@lal.in2p3.fr>
Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Fixes: a64489c56c30 ("perf report: Find the inline stack for a given address")
[ The above 'Fixes:' cset is where originally the problem was
  introduced, i.e.  using a2l->funcname without checking if it is NULL,
  but this current patch fixes the current codebase, i.e. multiple csets
  were applied after a64489c56c30 before the problem was reported by Hadrien ]
Link: http://lkml.kernel.org/r/20180926135207.30263-3-milian.wolff@kdab.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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)

^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2018-10-18  6:21 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 13:52 [PATCH 1/3] perf report: don't try to map ip to invalid map Milian Wolff
2018-09-26 13:52 ` [PATCH 2/3] perf report: use the offset address to find inline frames Milian Wolff
2018-09-27 16:00   ` Jiri Olsa
2018-09-27 19:12     ` Arnaldo Carvalho de Melo
2018-10-02  7:39   ` [PATCH] perf record: use unmapped IP for inline callchain cursors Milian Wolff
2018-10-02 15:32     ` Arnaldo Carvalho de Melo
2018-10-03  3:35       ` Ravi Bangoria
2018-10-05 13:48         ` Arnaldo Carvalho de Melo
2018-10-08 18:49           ` Milian Wolff
2018-10-05 14:11     ` Arnaldo Carvalho de Melo
2018-10-05 16:21     ` [tip:perf/urgent] perf record: Use " tip-bot for Milian Wolff
2018-09-26 13:52 ` [PATCH 3/3] perf report: don't crash on invalid inline debug information Milian Wolff
2018-09-27 19:10   ` Arnaldo Carvalho de Melo
2018-10-11 18:23     ` Milian Wolff
2018-10-11 19:39       ` Arnaldo Carvalho de Melo
2018-10-15 20:51         ` Milian Wolff
2018-10-16 17:49           ` Arnaldo Carvalho de Melo
2018-10-16 17:52             ` Arnaldo Carvalho de Melo
2018-10-16 19:00               ` Milian Wolff
2018-10-16 20:06                 ` Arnaldo Carvalho de Melo
2018-10-18  6:20   ` [tip:perf/urgent] perf report: Don't " tip-bot for Milian Wolff
2018-09-26 14:18 ` [PATCH 1/3] perf report: don't try to map ip to invalid map Arnaldo Carvalho de Melo
2018-09-26 14:41   ` Milian Wolff
2018-09-27 19:07     ` Arnaldo Carvalho de Melo
2018-09-27  8:48 ` Sandipan Das
2018-09-27 19:11   ` Arnaldo Carvalho de Melo
2018-09-27 16:02 ` Jiri Olsa
2018-10-05 16:20 ` [tip:perf/urgent] perf report: Don't " tip-bot for Milian Wolff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).