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

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

[Milian Wolff]

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

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

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

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c4acd2001db0..0cb4f8bf3ca7 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2312,7 +2312,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 {
 	struct callchain_cursor *cursor = arg;
 	const char *srcline = NULL;
-	u64 addr;
+	u64 addr = entry->ip;
 
 	if (symbol_conf.hide_unresolved && entry->sym == NULL)
 		return 0;
@@ -2324,7 +2324,8 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	 * Convert entry->ip from a virtual address to an offset in
 	 * its corresponding binary.
 	 */
-	addr = map__map_ip(entry->map, entry->ip);
+	if (entry->map)
+		addr = map__map_ip(entry->map, entry->ip);
 
 	srcline = callchain_srcline(entry->map, entry->sym, addr);
 	return callchain_cursor_append(cursor, entry->ip,
-- 
2.19.0

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

[Milian Wolff]

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

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

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

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0cb4f8bf3ca7..73a651f10a0f 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2317,9 +2317,6 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	if (symbol_conf.hide_unresolved && entry->sym == NULL)
 		return 0;
 
-	if (append_inlines(cursor, entry->map, entry->sym, entry->ip) == 0)
-		return 0;
-
 	/*
 	 * Convert entry->ip from a virtual address to an offset in
 	 * its corresponding binary.
@@ -2327,6 +2324,9 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	if (entry->map)
 		addr = map__map_ip(entry->map, entry->ip);
 
+	if (append_inlines(cursor, entry->map, entry->sym, addr) == 0)
+		return 0;
+
 	srcline = callchain_srcline(entry->map, entry->sym, addr);
 	return callchain_cursor_append(cursor, entry->ip,
 				       entry->map, entry->sym,
-- 
2.19.0

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

[Jiri Olsa]

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

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

thanks,
jirka

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

[Arnaldo Carvalho de Melo]

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

Thanks, applied.

- Arnaldo

[PATCH] perf record: use unmapped IP for inline callchain cursors

[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

Re: [PATCH] perf record: use unmapped IP for inline callchain cursors

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf record: use unmapped IP for inline callchain cursors

[Ravi Bangoria]

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>

Re: [PATCH] perf record: use unmapped IP for inline callchain cursors

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf record: use unmapped IP for inline callchain cursors

[Milian Wolff]

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

Re: [PATCH] perf record: use unmapped IP for inline callchain cursors

[Arnaldo Carvalho de Melo]

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

[tip:perf/urgent] perf record: Use unmapped IP for inline callchain cursors

[tip-bot for Milian Wolff]

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) {

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

[Milian Wolff]

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

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

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

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

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

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

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

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

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

[Arnaldo Carvalho de Melo]

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

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

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

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

[Milian Wolff]

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

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

Hey Arnaldo,

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

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

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

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

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

[Arnaldo Carvalho de Melo]

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

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

- Arnaldo

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

[Milian Wolff]

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

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

Ping?

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

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

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

[Arnaldo Carvalho de Melo]

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

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

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

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

      perf report: Find the inline stack for a given address

Agreed?

- Arnaldo

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

[Arnaldo Carvalho de Melo]

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

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

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

- Arnaldo

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

[Milian Wolff]

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

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

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

Cheers

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

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

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

[Arnaldo Carvalho de Melo]

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

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

- Arnaldo

[tip:perf/urgent] perf report: Don't crash on invalid inline debug information

[tip-bot for Milian Wolff]

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)

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

[Arnaldo Carvalho de Melo]

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

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

Thanks for working on this!

- Arnaldo
 
> #0  0x00005555557bdc4a in callchain_srcline (ip=<error reading variable: Cannot access memory at address 0x38>, sym=0x0, map=0x0) at util/machine.c:2329
> #1  unwind_entry (entry=entry@entry=0x7fffffff9180, arg=arg@entry=0x7ffff5642498) at util/machine.c:2329
> #2  0x00005555558370af in entry (arg=0x7ffff5642498, cb=0x5555557bdb50 <unwind_entry>, thread=<optimized out>, ip=18446744073709551615) at util/unwind-libunwind-local.c:586
> #3  get_entries (ui=ui@entry=0x7fffffff9620, cb=0x5555557bdb50 <unwind_entry>, arg=0x7ffff5642498, max_stack=<optimized out>) at util/unwind-libunwind-local.c:703
> #4  0x0000555555837192 in _unwind__get_entries (cb=<optimized out>, arg=<optimized out>, thread=<optimized out>, data=<optimized out>, max_stack=<optimized out>) at util/unwind-libunwind-local.c:725
> #5  0x00005555557c310f in thread__resolve_callchain_unwind (max_stack=127, sample=0x7fffffff9830, evsel=0x555555c7b3b0, cursor=0x7ffff5642498, thread=0x555555c7f6f0) at util/machine.c:2351
> #6  thread__resolve_callchain (thread=0x555555c7f6f0, cursor=0x7ffff5642498, evsel=0x555555c7b3b0, sample=0x7fffffff9830, parent=0x7fffffff97b8, root_al=0x7fffffff9750, max_stack=127) at util/machine.c:2378
> #7  0x00005555557ba4ee in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=parent@entry=0x7fffffff97b8, evsel=<optimized out>, al=al@entry=0x7fffffff9750,
>     max_stack=<optimized out>) at util/callchain.c:1085
> 
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> Cc: Sandipan Das <sandipan@linux.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> ---
>  tools/perf/util/machine.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index c4acd2001db0..0cb4f8bf3ca7 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2312,7 +2312,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
>  {
>  	struct callchain_cursor *cursor = arg;
>  	const char *srcline = NULL;
> -	u64 addr;
> +	u64 addr = entry->ip;
>  
>  	if (symbol_conf.hide_unresolved && entry->sym == NULL)
>  		return 0;
> @@ -2324,7 +2324,8 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
>  	 * Convert entry->ip from a virtual address to an offset in
>  	 * its corresponding binary.
>  	 */
> -	addr = map__map_ip(entry->map, entry->ip);
> +	if (entry->map)
> +		addr = map__map_ip(entry->map, entry->ip);
>  
>  	srcline = callchain_srcline(entry->map, entry->sym, addr);
>  	return callchain_cursor_append(cursor, entry->ip,
> -- 
> 2.19.0

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

[Milian Wolff]

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

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

The issue was introduced by

perf script: Show correct offsets for DWARF-based unwinding

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

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

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

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

[Arnaldo Carvalho de Melo]

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

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

- Arnaldo

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

[Sandipan Das]

On 26/09/18 7:22 PM, Milian Wolff wrote:
> Fixes a crash when the report encounters an address that
> could not be associated with an mmaped region:
> 
> #0  0x00005555557bdc4a in callchain_srcline (ip=<error reading variable: Cannot access memory at address 0x38>, sym=0x0, map=0x0) at util/machine.c:2329
> #1  unwind_entry (entry=entry@entry=0x7fffffff9180, arg=arg@entry=0x7ffff5642498) at util/machine.c:2329
> #2  0x00005555558370af in entry (arg=0x7ffff5642498, cb=0x5555557bdb50 <unwind_entry>, thread=<optimized out>, ip=18446744073709551615) at util/unwind-libunwind-local.c:586
> #3  get_entries (ui=ui@entry=0x7fffffff9620, cb=0x5555557bdb50 <unwind_entry>, arg=0x7ffff5642498, max_stack=<optimized out>) at util/unwind-libunwind-local.c:703
> #4  0x0000555555837192 in _unwind__get_entries (cb=<optimized out>, arg=<optimized out>, thread=<optimized out>, data=<optimized out>, max_stack=<optimized out>) at util/unwind-libunwind-local.c:725
> #5  0x00005555557c310f in thread__resolve_callchain_unwind (max_stack=127, sample=0x7fffffff9830, evsel=0x555555c7b3b0, cursor=0x7ffff5642498, thread=0x555555c7f6f0) at util/machine.c:2351
> #6  thread__resolve_callchain (thread=0x555555c7f6f0, cursor=0x7ffff5642498, evsel=0x555555c7b3b0, sample=0x7fffffff9830, parent=0x7fffffff97b8, root_al=0x7fffffff9750, max_stack=127) at util/machine.c:2378
> #7  0x00005555557ba4ee in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=parent@entry=0x7fffffff97b8, evsel=<optimized out>, al=al@entry=0x7fffffff9750,
>     max_stack=<optimized out>) at util/callchain.c:1085
> 

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

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

[Arnaldo Carvalho de Melo]

Em Thu, Sep 27, 2018 at 02:18:47PM +0530, Sandipan Das escreveu:
> 
> 
> On 26/09/18 7:22 PM, Milian Wolff wrote:
> > Fixes a crash when the report encounters an address that
> > could not be associated with an mmaped region:
> > 
> > #0  0x00005555557bdc4a in callchain_srcline (ip=<error reading variable: Cannot access memory at address 0x38>, sym=0x0, map=0x0) at util/machine.c:2329
> > #1  unwind_entry (entry=entry@entry=0x7fffffff9180, arg=arg@entry=0x7ffff5642498) at util/machine.c:2329
> > #2  0x00005555558370af in entry (arg=0x7ffff5642498, cb=0x5555557bdb50 <unwind_entry>, thread=<optimized out>, ip=18446744073709551615) at util/unwind-libunwind-local.c:586
> > #3  get_entries (ui=ui@entry=0x7fffffff9620, cb=0x5555557bdb50 <unwind_entry>, arg=0x7ffff5642498, max_stack=<optimized out>) at util/unwind-libunwind-local.c:703
> > #4  0x0000555555837192 in _unwind__get_entries (cb=<optimized out>, arg=<optimized out>, thread=<optimized out>, data=<optimized out>, max_stack=<optimized out>) at util/unwind-libunwind-local.c:725
> > #5  0x00005555557c310f in thread__resolve_callchain_unwind (max_stack=127, sample=0x7fffffff9830, evsel=0x555555c7b3b0, cursor=0x7ffff5642498, thread=0x555555c7f6f0) at util/machine.c:2351
> > #6  thread__resolve_callchain (thread=0x555555c7f6f0, cursor=0x7ffff5642498, evsel=0x555555c7b3b0, sample=0x7fffffff9830, parent=0x7fffffff97b8, root_al=0x7fffffff9750, max_stack=127) at util/machine.c:2378
> > #7  0x00005555557ba4ee in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=parent@entry=0x7fffffff97b8, evsel=<optimized out>, al=al@entry=0x7fffffff9750,
> >     max_stack=<optimized out>) at util/callchain.c:1085
> > 
> 
> Tested-by: Sandipan Das <sandipan@linux.ibm.com>

Thanks, applied.

- Arnaldo

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

[Jiri Olsa]

On Wed, Sep 26, 2018 at 03:52:05PM +0200, Milian Wolff wrote:
> Fixes a crash when the report encounters an address that
> could not be associated with an mmaped region:
> 
> #0  0x00005555557bdc4a in callchain_srcline (ip=<error reading variable: Cannot access memory at address 0x38>, sym=0x0, map=0x0) at util/machine.c:2329
> #1  unwind_entry (entry=entry@entry=0x7fffffff9180, arg=arg@entry=0x7ffff5642498) at util/machine.c:2329
> #2  0x00005555558370af in entry (arg=0x7ffff5642498, cb=0x5555557bdb50 <unwind_entry>, thread=<optimized out>, ip=18446744073709551615) at util/unwind-libunwind-local.c:586
> #3  get_entries (ui=ui@entry=0x7fffffff9620, cb=0x5555557bdb50 <unwind_entry>, arg=0x7ffff5642498, max_stack=<optimized out>) at util/unwind-libunwind-local.c:703
> #4  0x0000555555837192 in _unwind__get_entries (cb=<optimized out>, arg=<optimized out>, thread=<optimized out>, data=<optimized out>, max_stack=<optimized out>) at util/unwind-libunwind-local.c:725
> #5  0x00005555557c310f in thread__resolve_callchain_unwind (max_stack=127, sample=0x7fffffff9830, evsel=0x555555c7b3b0, cursor=0x7ffff5642498, thread=0x555555c7f6f0) at util/machine.c:2351
> #6  thread__resolve_callchain (thread=0x555555c7f6f0, cursor=0x7ffff5642498, evsel=0x555555c7b3b0, sample=0x7fffffff9830, parent=0x7fffffff97b8, root_al=0x7fffffff9750, max_stack=127) at util/machine.c:2378
> #7  0x00005555557ba4ee in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=parent@entry=0x7fffffff97b8, evsel=<optimized out>, al=al@entry=0x7fffffff9750,
>     max_stack=<optimized out>) at util/callchain.c:1085
> 
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> Cc: Sandipan Das <sandipan@linux.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

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

thanks,
jirka

[tip:perf/urgent] perf report: Don't try to map ip to invalid map

[tip-bot for Milian Wolff]

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,