linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] perf report: Support annotation of code without symbols
@ 2020-02-27  4:39 Jin Yao
  2020-02-27  4:39 ` [PATCH v5 1/3] perf util: Print al_addr when symbol is not found Jin Yao
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Jin Yao @ 2020-02-27  4:39 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

For perf report on stripped binaries it is currently impossible to do
annotation. The annotation state is all tied to symbols, but there are
either no symbols, or symbols are not covering all the code.

We should support the annotation functionality even without symbols.

The first patch uses al_addr to print because it's easy to dump
the instructions from this address in binary for branch mode.

The second patch supports the annotation on stripped binary.

The third patch supports the hotkey 'a' on address for annotation.

 v5:
 ---
 Separate the hotkey 'a' implementation to a new patch
 "perf report: Support hotkey 'a' on address for annotation"

 v4:
 ---
 1. Support the hotkey 'a'. When we press 'a' on address,
    now it supports the annotation.

 2. Change the patch title from
    "Support interactive annotation of code without symbols" to
    "perf report: Support interactive annotation of code without symbols"

 v3:
 ---
 Keep just the ANNOTATION_DUMMY_LEN, and remove the
 opts->annotate_dummy_len since it's the "maybe in future
 we will provide" feature.

 v2:
 ---
 Fix a crash issue when annotating an address in "unknown" object.

Jin Yao (3):
  perf util: Print al_addr when symbol is not found
  perf report: Support interactive annotation of code without symbols
  perf report: Support hotkey 'a' on address for annotation

 tools/perf/ui/browsers/hists.c | 90 +++++++++++++++++++++++++++-------
 tools/perf/util/annotate.h     |  1 +
 tools/perf/util/sort.c         |  6 ++-
 3 files changed, 78 insertions(+), 19 deletions(-)

-- 
2.17.1


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

* [PATCH v5 1/3] perf util: Print al_addr when symbol is not found
  2020-02-27  4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
@ 2020-02-27  4:39 ` Jin Yao
  2020-04-04  8:42   ` [tip: perf/urgent] perf report: " tip-bot2 for Jin Yao
  2020-02-27  4:39 ` [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols Jin Yao
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Jin Yao @ 2020-02-27  4:39 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

For branch mode, if the symbol is not found, it prints
the address.

For example, 0x0000555eee0365a0 in below output.

Overhead  Command  Source Shared Object  Source Symbol                            Target Symbol
  17.55%  div      libc-2.27.so          [.] __random                             [.] __random
   6.11%  div      div                   [.] 0x0000555eee0365a0                   [.] rand
   6.10%  div      libc-2.27.so          [.] rand                                 [.] 0x0000555eee036769
   5.80%  div      libc-2.27.so          [.] __random_r                           [.] __random
   5.72%  div      libc-2.27.so          [.] __random                             [.] __random_r
   5.62%  div      libc-2.27.so          [.] __random_r                           [.] __random_r
   5.38%  div      libc-2.27.so          [.] __random                             [.] rand
   4.56%  div      libc-2.27.so          [.] __random                             [.] __random
   4.49%  div      div                   [.] 0x0000555eee036779                   [.] 0x0000555eee0365ff
   4.25%  div      div                   [.] 0x0000555eee0365fa                   [.] 0x0000555eee036760

But it's not very easy to understand what the instructions
are in the binary. So this patch uses the al_addr instead.

With this patch, the output is

Overhead  Command  Source Shared Object  Source Symbol                            Target Symbol
  17.55%  div      libc-2.27.so          [.] __random                             [.] __random
   6.11%  div      div                   [.] 0x00000000000005a0                   [.] rand
   6.10%  div      libc-2.27.so          [.] rand                                 [.] 0x0000000000000769
   5.80%  div      libc-2.27.so          [.] __random_r                           [.] __random
   5.72%  div      libc-2.27.so          [.] __random                             [.] __random_r
   5.62%  div      libc-2.27.so          [.] __random_r                           [.] __random_r
   5.38%  div      libc-2.27.so          [.] __random                             [.] rand
   4.56%  div      libc-2.27.so          [.] __random                             [.] __random
   4.49%  div      div                   [.] 0x0000000000000779                   [.] 0x00000000000005ff
   4.25%  div      div                   [.] 0x00000000000005fa                   [.] 0x0000000000000760

Now we can use objdump to dump the object starting from 0x5a0.

For example,
objdump -d --start-address 0x5a0 div

00000000000005a0 <rand@plt>:
 5a0:   ff 25 2a 0a 20 00       jmpq   *0x200a2a(%rip)        # 200fd0 <__cxa_finalize@plt+0x200a20>
 5a6:   68 02 00 00 00          pushq  $0x2
 5ab:   e9 c0 ff ff ff          jmpq   570 <srand@plt-0x10>
 ...

 v2:
 ---
 No change

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/util/sort.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ab0cfd790ad0..e860595576c2 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -869,7 +869,8 @@ static int hist_entry__sym_from_snprintf(struct hist_entry *he, char *bf,
 	if (he->branch_info) {
 		struct addr_map_symbol *from = &he->branch_info->from;
 
-		return _hist_entry__sym_snprintf(&from->ms, from->addr, he->level, bf, size, width);
+		return _hist_entry__sym_snprintf(&from->ms, from->al_addr,
+						 he->level, bf, size, width);
 	}
 
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, "N/A");
@@ -881,7 +882,8 @@ static int hist_entry__sym_to_snprintf(struct hist_entry *he, char *bf,
 	if (he->branch_info) {
 		struct addr_map_symbol *to = &he->branch_info->to;
 
-		return _hist_entry__sym_snprintf(&to->ms, to->addr, he->level, bf, size, width);
+		return _hist_entry__sym_snprintf(&to->ms, to->al_addr,
+						 he->level, bf, size, width);
 	}
 
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, "N/A");
-- 
2.17.1


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

* [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-02-27  4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
  2020-02-27  4:39 ` [PATCH v5 1/3] perf util: Print al_addr when symbol is not found Jin Yao
@ 2020-02-27  4:39 ` Jin Yao
  2020-03-18 15:42   ` Arnaldo Carvalho de Melo
  2020-04-04  8:42   ` [tip: perf/urgent] " tip-bot2 for Jin Yao
  2020-02-27  4:39 ` [PATCH v5 3/3] perf report: Support hotkey 'a' on address for annotation Jin Yao
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Jin Yao @ 2020-02-27  4:39 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

For perf report on stripped binaries it is currently impossible to do
annotation. The annotation state is all tied to symbols, but there are
either no symbols, or symbols are not covering all the code.

We should support the annotation functionality even without symbols.

This patch fakes a symbol and the symbol name is the string of address.
After that, we just follow current annotation working flow.

For example,

1. perf report

Overhead  Command  Shared Object     Symbol
  20.67%  div      libc-2.27.so      [.] __random_r
  17.29%  div      libc-2.27.so      [.] __random
  10.59%  div      div               [.] 0x0000000000000628
   9.25%  div      div               [.] 0x0000000000000612
   6.11%  div      div               [.] 0x0000000000000645

2. Select the line of "10.59%  div      div               [.] 0x0000000000000628" and ENTER.

Annotate 0x0000000000000628
Zoom into div thread
Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
Browse map details
Run scripts for samples of symbol [0x0000000000000628]
Run scripts for all samples
Switch to another data file in PWD
Exit

3. Select the "Annotate 0x0000000000000628" and ENTER.

Percent│
       │
       │
       │     Disassembly of section .text:
       │
       │     0000000000000628 <.text+0x68>:
       │       divsd %xmm4,%xmm0
       │       divsd %xmm3,%xmm1
       │       movsd (%rsp),%xmm2
       │       addsd %xmm1,%xmm0
       │       addsd %xmm2,%xmm0
       │       movsd %xmm0,(%rsp)

Now we can see the dump of object starting from 0x628.

 v5:
 ---
 Remove the hotkey 'a' implementation from this patch. It
 will be moved to a separate patch.

 v4:
 ---
 1. Support the hotkey 'a'. When we press 'a' on address,
    now it supports the annotation.

 2. Change the patch title from
    "Support interactive annotation of code without symbols" to
    "perf report: Support interactive annotation of code without symbols"

 v3:
 ---
 Keep just the ANNOTATION_DUMMY_LEN, and remove the
 opts->annotate_dummy_len since it's the "maybe in future
 we will provide" feature.

 v2:
 ---
 Fix a crash issue when annotating an address in "unknown" object.

 The steps to reproduce this issue:

 perf record -e cycles:u ls
 perf report

    75.29%  ls       ld-2.27.so        [.] do_lookup_x
    23.64%  ls       ld-2.27.so        [.] __GI___tunables_init
     1.04%  ls       [unknown]         [k] 0xffffffff85c01210
     0.03%  ls       ld-2.27.so        [.] _start

 When annotating 0xffffffff85c01210, the crash happens.

 v2 adds checking for ms->map in add_annotate_opt(). If the object is
 "unknown", ms->map is NULL.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/ui/browsers/hists.c | 43 +++++++++++++++++++++++++++++-----
 tools/perf/util/annotate.h     |  1 +
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f36dee499320..2f07680559c4 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2465,13 +2465,41 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
 	return 0;
 }
 
+static struct symbol *new_annotate_sym(u64 addr, struct map *map)
+{
+	struct symbol *sym;
+	struct annotated_source *src;
+	char name[64];
+
+	snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
+
+	sym = symbol__new(addr, ANNOTATION_DUMMY_LEN, 0, 0, name);
+	if (sym) {
+		src = symbol__hists(sym, 1);
+		if (!src) {
+			symbol__delete(sym);
+			return NULL;
+		}
+
+		dso__insert_symbol(map->dso, sym);
+	}
+
+	return sym;
+}
+
 static int
 add_annotate_opt(struct hist_browser *browser __maybe_unused,
 		 struct popup_action *act, char **optstr,
-		 struct map_symbol *ms)
+		 struct map_symbol *ms,
+		 u64 addr)
 {
-	if (ms->sym == NULL || ms->map->dso->annotate_warned ||
-	    symbol__annotation(ms->sym)->src == NULL)
+	if (!ms->map || !ms->map->dso || ms->map->dso->annotate_warned)
+		return 0;
+
+	if (!ms->sym)
+		ms->sym = new_annotate_sym(addr, ms->map);
+
+	if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
 		return 0;
 
 	if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
@@ -3219,17 +3247,20 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 			nr_options += add_annotate_opt(browser,
 						       &actions[nr_options],
 						       &options[nr_options],
-						       &bi->from.ms);
+						       &bi->from.ms,
+						       bi->from.al_addr);
 			if (bi->to.ms.sym != bi->from.ms.sym)
 				nr_options += add_annotate_opt(browser,
 							&actions[nr_options],
 							&options[nr_options],
-							&bi->to.ms);
+							&bi->to.ms,
+							bi->to.al_addr);
 		} else {
 			nr_options += add_annotate_opt(browser,
 						       &actions[nr_options],
 						       &options[nr_options],
-						       browser->selection);
+						       browser->selection,
+						       browser->he_selection->ip);
 		}
 skip_annotation:
 		nr_options += add_thread_opt(browser, &actions[nr_options],
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 455403e8fede..c8463c48adf2 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -74,6 +74,7 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2);
 #define ANNOTATION__CYCLES_WIDTH 6
 #define ANNOTATION__MINMAX_CYCLES_WIDTH 19
 #define ANNOTATION__AVG_IPC_WIDTH 36
+#define ANNOTATION_DUMMY_LEN	256
 
 struct annotation_options {
 	bool hide_src_code,
-- 
2.17.1


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

* [PATCH v5 3/3] perf report: Support hotkey 'a' on address for annotation
  2020-02-27  4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
  2020-02-27  4:39 ` [PATCH v5 1/3] perf util: Print al_addr when symbol is not found Jin Yao
  2020-02-27  4:39 ` [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols Jin Yao
@ 2020-02-27  4:39 ` Jin Yao
  2020-04-04  8:42   ` [tip: perf/urgent] perf report/top TUI: Support hotkey 'a' for annotation of unresolved addresses tip-bot2 for Jin Yao
  2020-03-02  4:08 ` [PATCH v5 0/3] perf report: Support annotation of code without symbols Ravi Bangoria
  2020-03-02 12:51 ` Jiri Olsa
  4 siblings, 1 reply; 15+ messages in thread
From: Jin Yao @ 2020-02-27  4:39 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

In previous patch, we have supported the annotation functionality
even without symbols.

For this patch, it supports the hotkey 'a' on address in report
view. Note that, for branch mode, we only support the annotation
for "branch to" address.

 v5:
 ---
 New patch in v5.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/ui/browsers/hists.c | 47 ++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 2f07680559c4..7f9a8fd24e5c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -3061,21 +3061,46 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 				continue;
 			}
 
-			if (browser->selection == NULL ||
-			    browser->selection->sym == NULL ||
-			    browser->selection->map->dso->annotate_warned)
+			if (!browser->selection ||
+			    !browser->selection->map ||
+			    !browser->selection->map->dso ||
+			    browser->selection->map->dso->annotate_warned) {
 				continue;
+			}
 
-			if (symbol__annotation(browser->selection->sym)->src == NULL) {
-				ui_browser__warning(&browser->b, delay_secs * 2,
-						    "No samples for the \"%s\" symbol.\n\n"
-						    "Probably appeared just in a callchain",
-						    browser->selection->sym->name);
-				continue;
+			if (!browser->selection->sym) {
+				if (!browser->he_selection)
+					continue;
+
+				if (sort__mode == SORT_MODE__BRANCH) {
+					bi = browser->he_selection->branch_info;
+					if (!bi || !bi->to.ms.map)
+						continue;
+
+					actions->ms.sym = new_annotate_sym(bi->to.al_addr,
+									   bi->to.ms.map);
+					actions->ms.map = bi->to.ms.map;
+				} else {
+					actions->ms.sym = new_annotate_sym(browser->he_selection->ip,
+									   browser->selection->map);
+					actions->ms.map = browser->selection->map;
+				}
+
+				if (!actions->ms.sym)
+					continue;
+			} else {
+				if (symbol__annotation(browser->selection->sym)->src == NULL) {
+					ui_browser__warning(&browser->b, delay_secs * 2,
+						"No samples for the \"%s\" symbol.\n\n"
+						"Probably appeared just in a callchain",
+						browser->selection->sym->name);
+					continue;
+				}
+
+				actions->ms.map = browser->selection->map;
+				actions->ms.sym = browser->selection->sym;
 			}
 
-			actions->ms.map = browser->selection->map;
-			actions->ms.sym = browser->selection->sym;
 			do_annotate(browser, actions);
 			continue;
 		case 'P':
-- 
2.17.1


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

* Re: [PATCH v5 0/3] perf report: Support annotation of code without symbols
  2020-02-27  4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
                   ` (2 preceding siblings ...)
  2020-02-27  4:39 ` [PATCH v5 3/3] perf report: Support hotkey 'a' on address for annotation Jin Yao
@ 2020-03-02  4:08 ` Ravi Bangoria
  2020-03-02 12:51 ` Jiri Olsa
  4 siblings, 0 replies; 15+ messages in thread
From: Ravi Bangoria @ 2020-03-02  4:08 UTC (permalink / raw)
  To: Jin Yao, acme
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin, Ravi Bangoria



On 2/27/20 10:09 AM, Jin Yao wrote:
> For perf report on stripped binaries it is currently impossible to do
> annotation. The annotation state is all tied to symbols, but there are
> either no symbols, or symbols are not covering all the code.
> 
> We should support the annotation functionality even without symbols.
> 
> The first patch uses al_addr to print because it's easy to dump
> the instructions from this address in binary for branch mode.
> 
> The second patch supports the annotation on stripped binary.
> 
> The third patch supports the hotkey 'a' on address for annotation.

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

Ravi


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

* Re: [PATCH v5 0/3] perf report: Support annotation of code without symbols
  2020-02-27  4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
                   ` (3 preceding siblings ...)
  2020-03-02  4:08 ` [PATCH v5 0/3] perf report: Support annotation of code without symbols Ravi Bangoria
@ 2020-03-02 12:51 ` Jiri Olsa
  4 siblings, 0 replies; 15+ messages in thread
From: Jiri Olsa @ 2020-03-02 12:51 UTC (permalink / raw)
  To: Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Thu, Feb 27, 2020 at 12:39:36PM +0800, Jin Yao wrote:
> For perf report on stripped binaries it is currently impossible to do
> annotation. The annotation state is all tied to symbols, but there are
> either no symbols, or symbols are not covering all the code.
> 
> We should support the annotation functionality even without symbols.
> 
> The first patch uses al_addr to print because it's easy to dump
> the instructions from this address in binary for branch mode.
> 
> The second patch supports the annotation on stripped binary.
> 
> The third patch supports the hotkey 'a' on address for annotation.
> 
>  v5:
>  ---
>  Separate the hotkey 'a' implementation to a new patch
>  "perf report: Support hotkey 'a' on address for annotation"

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> 
>  v4:
>  ---
>  1. Support the hotkey 'a'. When we press 'a' on address,
>     now it supports the annotation.
> 
>  2. Change the patch title from
>     "Support interactive annotation of code without symbols" to
>     "perf report: Support interactive annotation of code without symbols"
> 
>  v3:
>  ---
>  Keep just the ANNOTATION_DUMMY_LEN, and remove the
>  opts->annotate_dummy_len since it's the "maybe in future
>  we will provide" feature.
> 
>  v2:
>  ---
>  Fix a crash issue when annotating an address in "unknown" object.
> 
> Jin Yao (3):
>   perf util: Print al_addr when symbol is not found
>   perf report: Support interactive annotation of code without symbols
>   perf report: Support hotkey 'a' on address for annotation
> 
>  tools/perf/ui/browsers/hists.c | 90 +++++++++++++++++++++++++++-------
>  tools/perf/util/annotate.h     |  1 +
>  tools/perf/util/sort.c         |  6 ++-
>  3 files changed, 78 insertions(+), 19 deletions(-)
> 
> -- 
> 2.17.1
> 


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

* Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-02-27  4:39 ` [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols Jin Yao
@ 2020-03-18 15:42   ` Arnaldo Carvalho de Melo
  2020-03-18 15:43     ` Arnaldo Carvalho de Melo
  2020-03-19  0:49     ` Jin, Yao
  2020-04-04  8:42   ` [tip: perf/urgent] " tip-bot2 for Jin Yao
  1 sibling, 2 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-18 15:42 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
> For perf report on stripped binaries it is currently impossible to do
> annotation. The annotation state is all tied to symbols, but there are
> either no symbols, or symbols are not covering all the code.
> 
> We should support the annotation functionality even without symbols.
> 
> This patch fakes a symbol and the symbol name is the string of address.
> After that, we just follow current annotation working flow.
> 
> For example,
> 
> 1. perf report
> 
> Overhead  Command  Shared Object     Symbol
>   20.67%  div      libc-2.27.so      [.] __random_r
>   17.29%  div      libc-2.27.so      [.] __random
>   10.59%  div      div               [.] 0x0000000000000628
>    9.25%  div      div               [.] 0x0000000000000612
>    6.11%  div      div               [.] 0x0000000000000645
> 
> 2. Select the line of "10.59%  div      div               [.] 0x0000000000000628" and ENTER.
> 
> Annotate 0x0000000000000628
> Zoom into div thread
> Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
> Browse map details
> Run scripts for samples of symbol [0x0000000000000628]
> Run scripts for all samples
> Switch to another data file in PWD
> Exit
> 
> 3. Select the "Annotate 0x0000000000000628" and ENTER.
> 
> Percent│
>        │
>        │
>        │     Disassembly of section .text:
>        │
>        │     0000000000000628 <.text+0x68>:
>        │       divsd %xmm4,%xmm0
>        │       divsd %xmm3,%xmm1
>        │       movsd (%rsp),%xmm2
>        │       addsd %xmm1,%xmm0
>        │       addsd %xmm2,%xmm0
>        │       movsd %xmm0,(%rsp)
> 
> Now we can see the dump of object starting from 0x628.

Testing this I noticed this discrepancy when using 'o' in the annotate
view to see the address columns:

Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
0x0000000000ea8b97  /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
Percent│                                                                                                                                                                                                                                                  ▒
       │                                                                                                                                                                                                                                                  ▒
       │                                                                                                                                                                                                                                                  ▒
       │        Disassembly of section .text:                                                                                                                                                                                                             ▒
       │                                                                                                                                                                                                                                                  ◆
       │        00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>:                                                                                                                                                                                ▒
       │12a8b97:   cmp     %rax,(%rdi)                                                                                                                                                                                                                    ▒
       │12a8b9a: ↓ je      12a8ba0 <linemap_get_expansion_line@@Base+0x230>                                                                                                                                                                               ▒
       │12a8b9c:   xor     %eax,%eax                                                                                                                                                                                                                      ▒
       │12a8b9e: ← retq                                                                                                                                                                                                                                   ▒
       │12a8b9f:   nop                                                                                                                                                                                                                                    ▒
       │12a8ba0:   mov     0x8(%rsi),%edx



 See that 0x0000000000ea8b97 != 12a8b97

How can we explain that?

- Arnaldo
 
>  v5:
>  ---
>  Remove the hotkey 'a' implementation from this patch. It
>  will be moved to a separate patch.
> 
>  v4:
>  ---
>  1. Support the hotkey 'a'. When we press 'a' on address,
>     now it supports the annotation.
> 
>  2. Change the patch title from
>     "Support interactive annotation of code without symbols" to
>     "perf report: Support interactive annotation of code without symbols"
> 
>  v3:
>  ---
>  Keep just the ANNOTATION_DUMMY_LEN, and remove the
>  opts->annotate_dummy_len since it's the "maybe in future
>  we will provide" feature.
> 
>  v2:
>  ---
>  Fix a crash issue when annotating an address in "unknown" object.
> 
>  The steps to reproduce this issue:
> 
>  perf record -e cycles:u ls
>  perf report
> 
>     75.29%  ls       ld-2.27.so        [.] do_lookup_x
>     23.64%  ls       ld-2.27.so        [.] __GI___tunables_init
>      1.04%  ls       [unknown]         [k] 0xffffffff85c01210
>      0.03%  ls       ld-2.27.so        [.] _start
> 
>  When annotating 0xffffffff85c01210, the crash happens.
> 
>  v2 adds checking for ms->map in add_annotate_opt(). If the object is
>  "unknown", ms->map is NULL.
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  tools/perf/ui/browsers/hists.c | 43 +++++++++++++++++++++++++++++-----
>  tools/perf/util/annotate.h     |  1 +
>  2 files changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index f36dee499320..2f07680559c4 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2465,13 +2465,41 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
>  	return 0;
>  }
>  
> +static struct symbol *new_annotate_sym(u64 addr, struct map *map)
> +{
> +	struct symbol *sym;
> +	struct annotated_source *src;
> +	char name[64];
> +
> +	snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
> +
> +	sym = symbol__new(addr, ANNOTATION_DUMMY_LEN, 0, 0, name);
> +	if (sym) {
> +		src = symbol__hists(sym, 1);
> +		if (!src) {
> +			symbol__delete(sym);
> +			return NULL;
> +		}
> +
> +		dso__insert_symbol(map->dso, sym);
> +	}
> +
> +	return sym;
> +}
> +
>  static int
>  add_annotate_opt(struct hist_browser *browser __maybe_unused,
>  		 struct popup_action *act, char **optstr,
> -		 struct map_symbol *ms)
> +		 struct map_symbol *ms,
> +		 u64 addr)
>  {
> -	if (ms->sym == NULL || ms->map->dso->annotate_warned ||
> -	    symbol__annotation(ms->sym)->src == NULL)
> +	if (!ms->map || !ms->map->dso || ms->map->dso->annotate_warned)
> +		return 0;
> +
> +	if (!ms->sym)
> +		ms->sym = new_annotate_sym(addr, ms->map);
> +
> +	if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
>  		return 0;
>  
>  	if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
> @@ -3219,17 +3247,20 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>  			nr_options += add_annotate_opt(browser,
>  						       &actions[nr_options],
>  						       &options[nr_options],
> -						       &bi->from.ms);
> +						       &bi->from.ms,
> +						       bi->from.al_addr);
>  			if (bi->to.ms.sym != bi->from.ms.sym)
>  				nr_options += add_annotate_opt(browser,
>  							&actions[nr_options],
>  							&options[nr_options],
> -							&bi->to.ms);
> +							&bi->to.ms,
> +							bi->to.al_addr);
>  		} else {
>  			nr_options += add_annotate_opt(browser,
>  						       &actions[nr_options],
>  						       &options[nr_options],
> -						       browser->selection);
> +						       browser->selection,
> +						       browser->he_selection->ip);
>  		}
>  skip_annotation:
>  		nr_options += add_thread_opt(browser, &actions[nr_options],
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 455403e8fede..c8463c48adf2 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -74,6 +74,7 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2);
>  #define ANNOTATION__CYCLES_WIDTH 6
>  #define ANNOTATION__MINMAX_CYCLES_WIDTH 19
>  #define ANNOTATION__AVG_IPC_WIDTH 36
> +#define ANNOTATION_DUMMY_LEN	256
>  
>  struct annotation_options {
>  	bool hide_src_code,
> -- 
> 2.17.1
> 

-- 

- Arnaldo

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

* Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-03-18 15:42   ` Arnaldo Carvalho de Melo
@ 2020-03-18 15:43     ` Arnaldo Carvalho de Melo
  2020-03-18 15:46       ` Arnaldo Carvalho de Melo
  2020-03-19  0:56       ` Jin, Yao
  2020-03-19  0:49     ` Jin, Yao
  1 sibling, 2 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-18 15:43 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Wed, Mar 18, 2020 at 12:42:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
> > For perf report on stripped binaries it is currently impossible to do
> > annotation. The annotation state is all tied to symbols, but there are
> > either no symbols, or symbols are not covering all the code.
> > 
> > We should support the annotation functionality even without symbols.
> > 
> > This patch fakes a symbol and the symbol name is the string of address.
> > After that, we just follow current annotation working flow.
> > 
> > For example,
> > 
> > 1. perf report
> > 
> > Overhead  Command  Shared Object     Symbol
> >   20.67%  div      libc-2.27.so      [.] __random_r
> >   17.29%  div      libc-2.27.so      [.] __random
> >   10.59%  div      div               [.] 0x0000000000000628
> >    9.25%  div      div               [.] 0x0000000000000612
> >    6.11%  div      div               [.] 0x0000000000000645
> > 
> > 2. Select the line of "10.59%  div      div               [.] 0x0000000000000628" and ENTER.
> > 
> > Annotate 0x0000000000000628
> > Zoom into div thread
> > Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
> > Browse map details
> > Run scripts for samples of symbol [0x0000000000000628]
> > Run scripts for all samples
> > Switch to another data file in PWD
> > Exit
> > 
> > 3. Select the "Annotate 0x0000000000000628" and ENTER.
> > 
> > Percent│
> >        │
> >        │
> >        │     Disassembly of section .text:
> >        │
> >        │     0000000000000628 <.text+0x68>:
> >        │       divsd %xmm4,%xmm0
> >        │       divsd %xmm3,%xmm1
> >        │       movsd (%rsp),%xmm2
> >        │       addsd %xmm1,%xmm0
> >        │       addsd %xmm2,%xmm0
> >        │       movsd %xmm0,(%rsp)
> > 
> > Now we can see the dump of object starting from 0x628.
> 
> Testing this I noticed this discrepancy when using 'o' in the annotate
> view to see the address columns:
> 
> Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
> 0x0000000000ea8b97  /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
> Percent│                                                                                                                                                                                                                                                  ▒
>        │                                                                                                                                                                                                                                                  ▒
>        │                                                                                                                                                                                                                                                  ▒
>        │        Disassembly of section .text:                                                                                                                                                                                                             ▒
>        │                                                                                                                                                                                                                                                  ◆
>        │        00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>:                                                                                                                                                                                ▒
>        │12a8b97:   cmp     %rax,(%rdi)                                                                                                                                                                                                                    ▒
>        │12a8b9a: ↓ je      12a8ba0 <linemap_get_expansion_line@@Base+0x230>                                                                                                                                                                               ▒
>        │12a8b9c:   xor     %eax,%eax                                                                                                                                                                                                                      ▒
>        │12a8b9e: ← retq                                                                                                                                                                                                                                   ▒
>        │12a8b9f:   nop                                                                                                                                                                                                                                    ▒
>        │12a8ba0:   mov     0x8(%rsi),%edx
> 
> 
> 
>  See that 0x0000000000ea8b97 != 12a8b97
> 
> How can we explain that?

On another machine, in 'perf top', its ok, the same address appears on
the second line and in the first line in the disassembled code.

I'm applying the patch,

- Arnaldo

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

* Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-03-18 15:43     ` Arnaldo Carvalho de Melo
@ 2020-03-18 15:46       ` Arnaldo Carvalho de Melo
  2020-03-19  1:11         ` Jin, Yao
  2020-03-19  0:56       ` Jin, Yao
  1 sibling, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-03-18 15:46 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Wed, Mar 18, 2020 at 12:43:58PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 18, 2020 at 12:42:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
> > > Now we can see the dump of object starting from 0x628.

> > Testing this I noticed this discrepancy when using 'o' in the annotate
> > view to see the address columns:

> > Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
> > 0x0000000000ea8b97  /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
> > Percent│
> >        │        Disassembly of section .text:
> >        │
> >        │        00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>:
> >        │12a8b97:   cmp     %rax,(%rdi)
> >        │12a8b9a: ↓ je      12a8ba0 <linemap_get_expansion_line@@Base+0x230>
> >        │12a8b9c:   xor     %eax,%eax
> >        │12a8b9e: ← retq
> >        │12a8b9f:   nop
> >        │12a8ba0:   mov     0x8(%rsi),%edx

> >  See that 0x0000000000ea8b97 != 12a8b97

> > How can we explain that?

> On another machine, in 'perf top', its ok, the same address appears on
> the second line and in the first line in the disassembled code.

> I'm applying the patch,

With this adjustments, ok?

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 2f07680559c4..c2556901f7b6 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2465,10 +2465,10 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
 	return 0;
 }
 
-static struct symbol *new_annotate_sym(u64 addr, struct map *map)
+static struct symbol *symbol__new_unresolved(u64 addr, struct map *map)
 {
-	struct symbol *sym;
 	struct annotated_source *src;
+	struct symbol *sym;
 	char name[64];
 
 	snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
@@ -2497,7 +2497,7 @@ add_annotate_opt(struct hist_browser *browser __maybe_unused,
 		return 0;
 
 	if (!ms->sym)
-		ms->sym = new_annotate_sym(addr, ms->map);
+		ms->sym = symbol__new_unresolved(addr, ms->map);
 
 	if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
 		return 0;

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

* Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-03-18 15:42   ` Arnaldo Carvalho de Melo
  2020-03-18 15:43     ` Arnaldo Carvalho de Melo
@ 2020-03-19  0:49     ` Jin, Yao
  1 sibling, 0 replies; 15+ messages in thread
From: Jin, Yao @ 2020-03-19  0:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 3/18/2020 11:42 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
>> For perf report on stripped binaries it is currently impossible to do
>> annotation. The annotation state is all tied to symbols, but there are
>> either no symbols, or symbols are not covering all the code.
>>
>> We should support the annotation functionality even without symbols.
>>
>> This patch fakes a symbol and the symbol name is the string of address.
>> After that, we just follow current annotation working flow.
>>
>> For example,
>>
>> 1. perf report
>>
>> Overhead  Command  Shared Object     Symbol
>>    20.67%  div      libc-2.27.so      [.] __random_r
>>    17.29%  div      libc-2.27.so      [.] __random
>>    10.59%  div      div               [.] 0x0000000000000628
>>     9.25%  div      div               [.] 0x0000000000000612
>>     6.11%  div      div               [.] 0x0000000000000645
>>
>> 2. Select the line of "10.59%  div      div               [.] 0x0000000000000628" and ENTER.
>>
>> Annotate 0x0000000000000628
>> Zoom into div thread
>> Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
>> Browse map details
>> Run scripts for samples of symbol [0x0000000000000628]
>> Run scripts for all samples
>> Switch to another data file in PWD
>> Exit
>>
>> 3. Select the "Annotate 0x0000000000000628" and ENTER.
>>
>> Percent│
>>         │
>>         │
>>         │     Disassembly of section .text:
>>         │
>>         │     0000000000000628 <.text+0x68>:
>>         │       divsd %xmm4,%xmm0
>>         │       divsd %xmm3,%xmm1
>>         │       movsd (%rsp),%xmm2
>>         │       addsd %xmm1,%xmm0
>>         │       addsd %xmm2,%xmm0
>>         │       movsd %xmm0,(%rsp)
>>
>> Now we can see the dump of object starting from 0x628.
> 
> Testing this I noticed this discrepancy when using 'o' in the annotate
> view to see the address columns:
> 
> Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
> 0x0000000000ea8b97  /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
> Percent│                                                                                                                                                                                                                                                  ▒
>         │                                                                                                                                                                                                                                                  ▒
>         │                                                                                                                                                                                                                                                  ▒
>         │        Disassembly of section .text:                                                                                                                                                                                                             ▒
>         │                                                                                                                                                                                                                                                  ◆
>         │        00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>:                                                                                                                                                                                ▒
>         │12a8b97:   cmp     %rax,(%rdi)                                                                                                                                                                                                                    ▒
>         │12a8b9a: ↓ je      12a8ba0 <linemap_get_expansion_line@@Base+0x230>                                                                                                                                                                               ▒
>         │12a8b9c:   xor     %eax,%eax                                                                                                                                                                                                                      ▒
>         │12a8b9e: ← retq                                                                                                                                                                                                                                   ▒
>         │12a8b9f:   nop                                                                                                                                                                                                                                    ▒
>         │12a8ba0:   mov     0x8(%rsi),%edx
> 
> 
> 
>   See that 0x0000000000ea8b97 != 12a8b97
> 
> How can we explain that?
> 
> - Arnaldo
>   

Not very clear about the reason. Actually the patch doesn't change the 
annotate view, it only fakes a dummy symbol.

I want to debug it but I'm failed to reproduce this issue on my machine. :(

Thanks
Jin Yao

>>   v5:
>>   ---
>>   Remove the hotkey 'a' implementation from this patch. It
>>   will be moved to a separate patch.
>>
>>   v4:
>>   ---
>>   1. Support the hotkey 'a'. When we press 'a' on address,
>>      now it supports the annotation.
>>
>>   2. Change the patch title from
>>      "Support interactive annotation of code without symbols" to
>>      "perf report: Support interactive annotation of code without symbols"
>>
>>   v3:
>>   ---
>>   Keep just the ANNOTATION_DUMMY_LEN, and remove the
>>   opts->annotate_dummy_len since it's the "maybe in future
>>   we will provide" feature.
>>
>>   v2:
>>   ---
>>   Fix a crash issue when annotating an address in "unknown" object.
>>
>>   The steps to reproduce this issue:
>>
>>   perf record -e cycles:u ls
>>   perf report
>>
>>      75.29%  ls       ld-2.27.so        [.] do_lookup_x
>>      23.64%  ls       ld-2.27.so        [.] __GI___tunables_init
>>       1.04%  ls       [unknown]         [k] 0xffffffff85c01210
>>       0.03%  ls       ld-2.27.so        [.] _start
>>
>>   When annotating 0xffffffff85c01210, the crash happens.
>>
>>   v2 adds checking for ms->map in add_annotate_opt(). If the object is
>>   "unknown", ms->map is NULL.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>>   tools/perf/ui/browsers/hists.c | 43 +++++++++++++++++++++++++++++-----
>>   tools/perf/util/annotate.h     |  1 +
>>   2 files changed, 38 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index f36dee499320..2f07680559c4 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -2465,13 +2465,41 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
>>   	return 0;
>>   }
>>   
>> +static struct symbol *new_annotate_sym(u64 addr, struct map *map)
>> +{
>> +	struct symbol *sym;
>> +	struct annotated_source *src;
>> +	char name[64];
>> +
>> +	snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
>> +
>> +	sym = symbol__new(addr, ANNOTATION_DUMMY_LEN, 0, 0, name);
>> +	if (sym) {
>> +		src = symbol__hists(sym, 1);
>> +		if (!src) {
>> +			symbol__delete(sym);
>> +			return NULL;
>> +		}
>> +
>> +		dso__insert_symbol(map->dso, sym);
>> +	}
>> +
>> +	return sym;
>> +}
>> +
>>   static int
>>   add_annotate_opt(struct hist_browser *browser __maybe_unused,
>>   		 struct popup_action *act, char **optstr,
>> -		 struct map_symbol *ms)
>> +		 struct map_symbol *ms,
>> +		 u64 addr)
>>   {
>> -	if (ms->sym == NULL || ms->map->dso->annotate_warned ||
>> -	    symbol__annotation(ms->sym)->src == NULL)
>> +	if (!ms->map || !ms->map->dso || ms->map->dso->annotate_warned)
>> +		return 0;
>> +
>> +	if (!ms->sym)
>> +		ms->sym = new_annotate_sym(addr, ms->map);
>> +
>> +	if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
>>   		return 0;
>>   
>>   	if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
>> @@ -3219,17 +3247,20 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>>   			nr_options += add_annotate_opt(browser,
>>   						       &actions[nr_options],
>>   						       &options[nr_options],
>> -						       &bi->from.ms);
>> +						       &bi->from.ms,
>> +						       bi->from.al_addr);
>>   			if (bi->to.ms.sym != bi->from.ms.sym)
>>   				nr_options += add_annotate_opt(browser,
>>   							&actions[nr_options],
>>   							&options[nr_options],
>> -							&bi->to.ms);
>> +							&bi->to.ms,
>> +							bi->to.al_addr);
>>   		} else {
>>   			nr_options += add_annotate_opt(browser,
>>   						       &actions[nr_options],
>>   						       &options[nr_options],
>> -						       browser->selection);
>> +						       browser->selection,
>> +						       browser->he_selection->ip);
>>   		}
>>   skip_annotation:
>>   		nr_options += add_thread_opt(browser, &actions[nr_options],
>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>> index 455403e8fede..c8463c48adf2 100644
>> --- a/tools/perf/util/annotate.h
>> +++ b/tools/perf/util/annotate.h
>> @@ -74,6 +74,7 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2);
>>   #define ANNOTATION__CYCLES_WIDTH 6
>>   #define ANNOTATION__MINMAX_CYCLES_WIDTH 19
>>   #define ANNOTATION__AVG_IPC_WIDTH 36
>> +#define ANNOTATION_DUMMY_LEN	256
>>   
>>   struct annotation_options {
>>   	bool hide_src_code,
>> -- 
>> 2.17.1
>>
> 

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

* Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-03-18 15:43     ` Arnaldo Carvalho de Melo
  2020-03-18 15:46       ` Arnaldo Carvalho de Melo
@ 2020-03-19  0:56       ` Jin, Yao
  1 sibling, 0 replies; 15+ messages in thread
From: Jin, Yao @ 2020-03-19  0:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 3/18/2020 11:43 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 18, 2020 at 12:42:06PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
>>> For perf report on stripped binaries it is currently impossible to do
>>> annotation. The annotation state is all tied to symbols, but there are
>>> either no symbols, or symbols are not covering all the code.
>>>
>>> We should support the annotation functionality even without symbols.
>>>
>>> This patch fakes a symbol and the symbol name is the string of address.
>>> After that, we just follow current annotation working flow.
>>>
>>> For example,
>>>
>>> 1. perf report
>>>
>>> Overhead  Command  Shared Object     Symbol
>>>    20.67%  div      libc-2.27.so      [.] __random_r
>>>    17.29%  div      libc-2.27.so      [.] __random
>>>    10.59%  div      div               [.] 0x0000000000000628
>>>     9.25%  div      div               [.] 0x0000000000000612
>>>     6.11%  div      div               [.] 0x0000000000000645
>>>
>>> 2. Select the line of "10.59%  div      div               [.] 0x0000000000000628" and ENTER.
>>>
>>> Annotate 0x0000000000000628
>>> Zoom into div thread
>>> Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
>>> Browse map details
>>> Run scripts for samples of symbol [0x0000000000000628]
>>> Run scripts for all samples
>>> Switch to another data file in PWD
>>> Exit
>>>
>>> 3. Select the "Annotate 0x0000000000000628" and ENTER.
>>>
>>> Percent│
>>>         │
>>>         │
>>>         │     Disassembly of section .text:
>>>         │
>>>         │     0000000000000628 <.text+0x68>:
>>>         │       divsd %xmm4,%xmm0
>>>         │       divsd %xmm3,%xmm1
>>>         │       movsd (%rsp),%xmm2
>>>         │       addsd %xmm1,%xmm0
>>>         │       addsd %xmm2,%xmm0
>>>         │       movsd %xmm0,(%rsp)
>>>
>>> Now we can see the dump of object starting from 0x628.
>>
>> Testing this I noticed this discrepancy when using 'o' in the annotate
>> view to see the address columns:
>>
>> Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
>> 0x0000000000ea8b97  /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
>> Percent│                                                                                                                                                                                                                                                  ▒
>>         │                                                                                                                                                                                                                                                  ▒
>>         │                                                                                                                                                                                                                                                  ▒
>>         │        Disassembly of section .text:                                                                                                                                                                                                             ▒
>>         │                                                                                                                                                                                                                                                  ◆
>>         │        00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>:                                                                                                                                                                                ▒
>>         │12a8b97:   cmp     %rax,(%rdi)                                                                                                                                                                                                                    ▒
>>         │12a8b9a: ↓ je      12a8ba0 <linemap_get_expansion_line@@Base+0x230>                                                                                                                                                                               ▒
>>         │12a8b9c:   xor     %eax,%eax                                                                                                                                                                                                                      ▒
>>         │12a8b9e: ← retq                                                                                                                                                                                                                                   ▒
>>         │12a8b9f:   nop                                                                                                                                                                                                                                    ▒
>>         │12a8ba0:   mov     0x8(%rsi),%edx
>>
>>
>>
>>   See that 0x0000000000ea8b97 != 12a8b97
>>
>> How can we explain that?
> 
> On another machine, in 'perf top', its ok, the same address appears on
> the second line and in the first line in the disassembled code.
> 
> I'm applying the patch,
> 
> - Arnaldo
> 

Yes, it looks strange. On my test machines, perf report and perf top 
both work fine. I'm using ubuntu 18.04.

Thanks
Jin Yao


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

* Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
  2020-03-18 15:46       ` Arnaldo Carvalho de Melo
@ 2020-03-19  1:11         ` Jin, Yao
  0 siblings, 0 replies; 15+ messages in thread
From: Jin, Yao @ 2020-03-19  1:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 3/18/2020 11:46 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 18, 2020 at 12:43:58PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Wed, Mar 18, 2020 at 12:42:06PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
>>>> Now we can see the dump of object starting from 0x628.
> 
>>> Testing this I noticed this discrepancy when using 'o' in the annotate
>>> view to see the address columns:
> 
>>> Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
>>> 0x0000000000ea8b97  /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
>>> Percent│
>>>         │        Disassembly of section .text:
>>>         │
>>>         │        00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>:
>>>         │12a8b97:   cmp     %rax,(%rdi)
>>>         │12a8b9a: ↓ je      12a8ba0 <linemap_get_expansion_line@@Base+0x230>
>>>         │12a8b9c:   xor     %eax,%eax
>>>         │12a8b9e: ← retq
>>>         │12a8b9f:   nop
>>>         │12a8ba0:   mov     0x8(%rsi),%edx
> 
>>>   See that 0x0000000000ea8b97 != 12a8b97
> 
>>> How can we explain that?
> 
>> On another machine, in 'perf top', its ok, the same address appears on
>> the second line and in the first line in the disassembled code.
> 
>> I'm applying the patch,
> 
> With this adjustments, ok?
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 2f07680559c4..c2556901f7b6 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2465,10 +2465,10 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
>   	return 0;
>   }
>   
> -static struct symbol *new_annotate_sym(u64 addr, struct map *map)
> +static struct symbol *symbol__new_unresolved(u64 addr, struct map *map)
>   {
> -	struct symbol *sym;
>   	struct annotated_source *src;
> +	struct symbol *sym;
>   	char name[64];
>   
>   	snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
> @@ -2497,7 +2497,7 @@ add_annotate_opt(struct hist_browser *browser __maybe_unused,
>   		return 0;
>   
>   	if (!ms->sym)
> -		ms->sym = new_annotate_sym(addr, ms->map);
> +		ms->sym = symbol__new_unresolved(addr, ms->map);
>   
>   	if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
>   		return 0;
> 

Sure, that's OK, thanks! The name "symbol__new_unresolved" is much 
better than "new_annotate_sym".

Thanks
Jin Yao

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

* [tip: perf/urgent] perf report/top TUI: Support hotkey 'a' for annotation of unresolved addresses
  2020-02-27  4:39 ` [PATCH v5 3/3] perf report: Support hotkey 'a' on address for annotation Jin Yao
@ 2020-04-04  8:42   ` tip-bot2 for Jin Yao
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Jin Yao @ 2020-04-04  8:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Jin Yao, Arnaldo Carvalho de Melo, Jiri Olsa, Alexander Shishkin,
	Andi Kleen, Kan Liang, Peter Zijlstra, x86, LKML

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     ec0479a63b7657e57665a9fc81d11a99131cfc62
Gitweb:        https://git.kernel.org/tip/ec0479a63b7657e57665a9fc81d11a99131cfc62
Author:        Jin Yao <yao.jin@linux.intel.com>
AuthorDate:    Thu, 27 Feb 2020 12:39:39 +08:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 24 Mar 2020 09:37:27 -03:00

perf report/top TUI: Support hotkey 'a' for annotation of unresolved addresses

In previous patch, we have supported the annotation functionality even
without symbols.

For this patch, it supports the hotkey 'a' on address in report view.
Note that, for branch mode, we only support the annotation for "branch
to" address.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200227043939.4403-4-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 46 +++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d0f9745..1103a01 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -3061,21 +3061,45 @@ do_hotkey:		 // key came straight from options ui__popup_menu()
 				continue;
 			}
 
-			if (browser->selection == NULL ||
-			    browser->selection->sym == NULL ||
-			    browser->selection->map->dso->annotate_warned)
+			if (!browser->selection ||
+			    !browser->selection->map ||
+			    !browser->selection->map->dso ||
+			    browser->selection->map->dso->annotate_warned) {
 				continue;
+			}
 
-			if (symbol__annotation(browser->selection->sym)->src == NULL) {
-				ui_browser__warning(&browser->b, delay_secs * 2,
-						    "No samples for the \"%s\" symbol.\n\n"
-						    "Probably appeared just in a callchain",
-						    browser->selection->sym->name);
-				continue;
+			if (!browser->selection->sym) {
+				if (!browser->he_selection)
+					continue;
+
+				if (sort__mode == SORT_MODE__BRANCH) {
+					bi = browser->he_selection->branch_info;
+					if (!bi || !bi->to.ms.map)
+						continue;
+
+					actions->ms.sym = symbol__new_unresolved(bi->to.al_addr, bi->to.ms.map);
+					actions->ms.map = bi->to.ms.map;
+				} else {
+					actions->ms.sym = symbol__new_unresolved(browser->he_selection->ip,
+										 browser->selection->map);
+					actions->ms.map = browser->selection->map;
+				}
+
+				if (!actions->ms.sym)
+					continue;
+			} else {
+				if (symbol__annotation(browser->selection->sym)->src == NULL) {
+					ui_browser__warning(&browser->b, delay_secs * 2,
+						"No samples for the \"%s\" symbol.\n\n"
+						"Probably appeared just in a callchain",
+						browser->selection->sym->name);
+					continue;
+				}
+
+				actions->ms.map = browser->selection->map;
+				actions->ms.sym = browser->selection->sym;
 			}
 
-			actions->ms.map = browser->selection->map;
-			actions->ms.sym = browser->selection->sym;
 			do_annotate(browser, actions);
 			continue;
 		case 'P':

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

* [tip: perf/urgent] perf report: Print al_addr when symbol is not found
  2020-02-27  4:39 ` [PATCH v5 1/3] perf util: Print al_addr when symbol is not found Jin Yao
@ 2020-04-04  8:42   ` tip-bot2 for Jin Yao
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Jin Yao @ 2020-04-04  8:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Jin Yao, Arnaldo Carvalho de Melo, Ravi Bangoria, Jiri Olsa,
	Alexander Shishkin, Andi Kleen, Kan Liang, Peter Zijlstra, x86,
	LKML

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     443bc639e518c6c4e8fc2e0456cccf3a04f6e77f
Gitweb:        https://git.kernel.org/tip/443bc639e518c6c4e8fc2e0456cccf3a04f6e77f
Author:        Jin Yao <yao.jin@linux.intel.com>
AuthorDate:    Thu, 27 Feb 2020 12:39:37 +08:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 23 Mar 2020 11:08:29 -03:00

perf report: Print al_addr when symbol is not found

For branch mode, if the symbol is not found, it prints
the address.

For example, 0x0000555eee0365a0 in below output.

  Overhead  Command  Source Shared Object  Source Symbol                            Target Symbol
    17.55%  div      libc-2.27.so          [.] __random                             [.] __random
     6.11%  div      div                   [.] 0x0000555eee0365a0                   [.] rand
     6.10%  div      libc-2.27.so          [.] rand                                 [.] 0x0000555eee036769
     5.80%  div      libc-2.27.so          [.] __random_r                           [.] __random
     5.72%  div      libc-2.27.so          [.] __random                             [.] __random_r
     5.62%  div      libc-2.27.so          [.] __random_r                           [.] __random_r
     5.38%  div      libc-2.27.so          [.] __random                             [.] rand
     4.56%  div      libc-2.27.so          [.] __random                             [.] __random
     4.49%  div      div                   [.] 0x0000555eee036779                   [.] 0x0000555eee0365ff
     4.25%  div      div                   [.] 0x0000555eee0365fa                   [.] 0x0000555eee036760

But it's not very easy to understand what the instructions
are in the binary. So this patch uses the al_addr instead.

With this patch, the output is

  Overhead  Command  Source Shared Object  Source Symbol                            Target Symbol
    17.55%  div      libc-2.27.so          [.] __random                             [.] __random
     6.11%  div      div                   [.] 0x00000000000005a0                   [.] rand
     6.10%  div      libc-2.27.so          [.] rand                                 [.] 0x0000000000000769
     5.80%  div      libc-2.27.so          [.] __random_r                           [.] __random
     5.72%  div      libc-2.27.so          [.] __random                             [.] __random_r
     5.62%  div      libc-2.27.so          [.] __random_r                           [.] __random_r
     5.38%  div      libc-2.27.so          [.] __random                             [.] rand
     4.56%  div      libc-2.27.so          [.] __random                             [.] __random
     4.49%  div      div                   [.] 0x0000000000000779                   [.] 0x00000000000005ff
     4.25%  div      div                   [.] 0x00000000000005fa                   [.] 0x0000000000000760

Now we can use objdump to dump the object starting from 0x5a0.

For example,
objdump -d --start-address 0x5a0 div

00000000000005a0 <rand@plt>:
 5a0:   ff 25 2a 0a 20 00       jmpq   *0x200a2a(%rip)        # 200fd0 <__cxa_finalize@plt+0x200a20>
 5a6:   68 02 00 00 00          pushq  $0x2
 5ab:   e9 c0 ff ff ff          jmpq   570 <srand@plt-0x10>
 ...

Committer testing:

  [root@seventh ~]# perf record -a -b sleep 1
  [root@seventh ~]# perf report --header-only | grep cpudesc
  # cpudesc : Intel(R) Core(TM) i5-7500 CPU @ 3.40GHz
  [root@seventh ~]# perf evlist -v
  cycles: size: 120, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CPU|PERIOD|BRANCH_STACK, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1, branch_sample_type: ANY
  [root@seventh ~]#

Before:

  [root@seventh ~]# perf report --stdio --dso libsystemd-shared-241.so | head -20
  # To display the perf.data header info, please use --header/--header-only options.
  #
  #
  # Total Lost Samples: 0
  #
  # Samples: 2K of event 'cycles'
  # Event count (approx.): 2240
  #
  # Overhead  Command          Source Shared Object      Source Symbol           Target Symbol           Basic Block Cycles
  # ........  ...............  ........................  ......................  ......................  ..................
  #
       0.13%  systemd-journal  libc-2.29.so              [.] cfree@GLIBC_2.2.5   [.] _int_free           1
       0.09%  systemd          libsystemd-shared-241.so  [.] 0x00007fe406465c82  [.] 0x00007fe406465d80  1
       0.09%  systemd          libsystemd-shared-241.so  [.] 0x00007fe406465ded  [.] 0x00007fe406465c30  1
       0.09%  systemd          libsystemd-shared-241.so  [.] 0x00007fe406465e4e  [.] 0x00007fe406465de0  1
       0.09%  systemd-journal  systemd-journald          [.] free@plt            [.] cfree@GLIBC_2.2.5   1
       0.09%  systemd-journal  libc-2.29.so              [.] _int_free           [.] _int_free           18
       0.09%  systemd-journal  libc-2.29.so              [.] _int_free           [.] _int_free           2
       0.04%  systemd          libsystemd-shared-241.so  [.] bus_resolve@plt     [.] bus_resolve         204
       0.04%  systemd          libsystemd-shared-241.so  [.] getpid_cached@plt   [.] getpid_cached       7
  [root@seventh ~]#

After:

  [root@seventh ~]# perf report --stdio --dso libsystemd-shared-241.so | head -20
  # To display the perf.data header info, please use --header/--header-only options.
  #
  #
  # Total Lost Samples: 0
  #
  # Samples: 2K of event 'cycles'
  # Event count (approx.): 2240
  #
  # Overhead  Command          Source Shared Object      Source Symbol           Target Symbol           Basic Block Cycles
  # ........  ...............  ........................  ......................  ......................  ..................
  #
       0.13%  systemd-journal  libc-2.29.so              [.] cfree@GLIBC_2.2.5   [.] _int_free           1
       0.09%  systemd          libsystemd-shared-241.so  [.] 0x00000000000f7c82  [.] 0x00000000000f7d80  1
       0.09%  systemd          libsystemd-shared-241.so  [.] 0x00000000000f7ded  [.] 0x00000000000f7c30  1
       0.09%  systemd          libsystemd-shared-241.so  [.] 0x00000000000f7e4e  [.] 0x00000000000f7de0  1
       0.09%  systemd-journal  systemd-journald          [.] free@plt            [.] cfree@GLIBC_2.2.5   1
       0.09%  systemd-journal  libc-2.29.so              [.] _int_free           [.] _int_free           18
       0.09%  systemd-journal  libc-2.29.so              [.] _int_free           [.] _int_free           2
       0.04%  systemd          libsystemd-shared-241.so  [.] bus_resolve@plt     [.] bus_resolve         204
       0.04%  systemd          libsystemd-shared-241.so  [.] getpid_cached@plt   [.] getpid_cached       7
  [root@seventh ~]#

Lets use -v to get full paths and then try objdump on the unresolved address:

  [root@seventh ~]# perf report -v --stdio --dso libsystemd-shared-241.so |& grep libsystemd-shared-241.so | tail -1
     0.04% systemd-journal /usr/lib/systemd/libsystemd-shared-241.so 0x80c1a B [.] 0x0000000000080c1a 0x80a95 B [.] 0x0000000000080a95 61
  [root@seventh ~]#

  [root@seventh ~]# objdump -d --start-address 0x00000000000f7d80 /usr/lib/systemd/libsystemd-shared-241.so | head -20

  /usr/lib/systemd/libsystemd-shared-241.so:     file format elf64-x86-64

  Disassembly of section .text:

  00000000000f7d80 <proc_cmdline_parse_given@@SD_SHARED+0x330>:
     f7d80:	41 39 11             	cmp    %edx,(%r9)
     f7d83:	0f 84 ff fe ff ff    	je     f7c88 <proc_cmdline_parse_given@@SD_SHARED+0x238>
     f7d89:	4c 8d 05 97 09 0c 00 	lea    0xc0997(%rip),%r8        # 1b8727 <utf8_skip_data@@SD_SHARED+0x3147>
     f7d90:	b9 49 00 00 00       	mov    $0x49,%ecx
     f7d95:	48 8d 15 c9 f5 0b 00 	lea    0xbf5c9(%rip),%rdx        # 1b7365 <utf8_skip_data@@SD_SHARED+0x1d85>
     f7d9c:	31 ff                	xor    %edi,%edi
     f7d9e:	48 8d 35 9b ff 0b 00 	lea    0xbff9b(%rip),%rsi        # 1b7d40 <utf8_skip_data@@SD_SHARED+0x2760>
     f7da5:	e8 a6 d6 f4 ff       	callq  45450 <log_assert_failed_realm@plt>
     f7daa:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)
     f7db0:	41 56                	push   %r14
     f7db2:	41 55                	push   %r13
     f7db4:	41 54                	push   %r12
     f7db6:	55                   	push   %rbp
  [root@seventh ~]#

If we tried the the reported address before this patch:

  [root@seventh ~]# objdump -d --start-address 0x00007fe406465d80 /usr/lib/systemd/libsystemd-shared-241.so | head -20

  /usr/lib/systemd/libsystemd-shared-241.so:     file format elf64-x86-64

  [root@seventh ~]#

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200227043939.4403-2-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ab0cfd7..e860595 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -869,7 +869,8 @@ static int hist_entry__sym_from_snprintf(struct hist_entry *he, char *bf,
 	if (he->branch_info) {
 		struct addr_map_symbol *from = &he->branch_info->from;
 
-		return _hist_entry__sym_snprintf(&from->ms, from->addr, he->level, bf, size, width);
+		return _hist_entry__sym_snprintf(&from->ms, from->al_addr,
+						 he->level, bf, size, width);
 	}
 
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, "N/A");
@@ -881,7 +882,8 @@ static int hist_entry__sym_to_snprintf(struct hist_entry *he, char *bf,
 	if (he->branch_info) {
 		struct addr_map_symbol *to = &he->branch_info->to;
 
-		return _hist_entry__sym_snprintf(&to->ms, to->addr, he->level, bf, size, width);
+		return _hist_entry__sym_snprintf(&to->ms, to->al_addr,
+						 he->level, bf, size, width);
 	}
 
 	return repsep_snprintf(bf, size, "%-*.*s", width, width, "N/A");

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

* [tip: perf/urgent] perf report: Support interactive annotation of code without symbols
  2020-02-27  4:39 ` [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols Jin Yao
  2020-03-18 15:42   ` Arnaldo Carvalho de Melo
@ 2020-04-04  8:42   ` tip-bot2 for Jin Yao
  1 sibling, 0 replies; 15+ messages in thread
From: tip-bot2 for Jin Yao @ 2020-04-04  8:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Jin Yao, Arnaldo Carvalho de Melo, Ravi Bangoria, Jiri Olsa,
	Alexander Shishkin, Andi Kleen, Kan Liang, Peter Zijlstra, x86,
	LKML

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     7b0a0dcb64705d1dbed46d33c9810251667469b9
Gitweb:        https://git.kernel.org/tip/7b0a0dcb64705d1dbed46d33c9810251667469b9
Author:        Jin Yao <yao.jin@linux.intel.com>
AuthorDate:    Thu, 27 Feb 2020 12:39:38 +08:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 24 Mar 2020 09:36:33 -03:00

perf report: Support interactive annotation of code without symbols

For perf report on stripped binaries it is currently impossible to do
annotation. The annotation state is all tied to symbols, but there are
either no symbols, or symbols are not covering all the code.

We should support the annotation functionality even without symbols.

This patch fakes a symbol and the symbol name is the string of address.
After that, we just follow current annotation working flow.

For example,

1. perf report

  Overhead  Command  Shared Object     Symbol
    20.67%  div      libc-2.27.so      [.] __random_r
    17.29%  div      libc-2.27.so      [.] __random
    10.59%  div      div               [.] 0x0000000000000628
     9.25%  div      div               [.] 0x0000000000000612
     6.11%  div      div               [.] 0x0000000000000645

2. Select the line of "10.59%  div      div               [.] 0x0000000000000628" and ENTER.

  Annotate 0x0000000000000628
  Zoom into div thread
  Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
  Browse map details
  Run scripts for samples of symbol [0x0000000000000628]
  Run scripts for all samples
  Switch to another data file in PWD
  Exit

3. Select the "Annotate 0x0000000000000628" and ENTER.

Percent│
       │
       │
       │     Disassembly of section .text:
       │
       │     0000000000000628 <.text+0x68>:
       │       divsd %xmm4,%xmm0
       │       divsd %xmm3,%xmm1
       │       movsd (%rsp),%xmm2
       │       addsd %xmm1,%xmm0
       │       addsd %xmm2,%xmm0
       │       movsd %xmm0,(%rsp)

Now we can see the dump of object starting from 0x628.

 v5:
 ---
 Remove the hotkey 'a' implementation from this patch. It
 will be moved to a separate patch.

 v4:
 ---
 1. Support the hotkey 'a'. When we press 'a' on address,
    now it supports the annotation.

 2. Change the patch title from
    "Support interactive annotation of code without symbols" to
    "perf report: Support interactive annotation of code without symbols"

 v3:
 ---
 Keep just the ANNOTATION_DUMMY_LEN, and remove the
 opts->annotate_dummy_len since it's the "maybe in future
 we will provide" feature.

 v2:
 ---
 Fix a crash issue when annotating an address in "unknown" object.

 The steps to reproduce this issue:

 perf record -e cycles:u ls
 perf report

    75.29%  ls       ld-2.27.so        [.] do_lookup_x
    23.64%  ls       ld-2.27.so        [.] __GI___tunables_init
     1.04%  ls       [unknown]         [k] 0xffffffff85c01210
     0.03%  ls       ld-2.27.so        [.] _start

 When annotating 0xffffffff85c01210, the crash happens.

 v2 adds checking for ms->map in add_annotate_opt(). If the object is
 "unknown", ms->map is NULL.

Committer notes:

Renamed new_annotate_sym() to symbol__new_unresolved().

Use PRIx64 to fix this issue in some 32-bit arches:

  ui/browsers/hists.c: In function 'symbol__new_unresolved':
  ui/browsers/hists.c:2474:38: error: format '%lx' expects argument of type 'long unsigned int', but argument 5 has type 'u64' {aka 'long long unsigned int'} [-Werror=format=]
    snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
                                  ~~~~~~^                      ~~~~
                                  %-#.*llx

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200227043939.4403-3-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 43 ++++++++++++++++++++++++++++-----
 tools/perf/util/annotate.h     |  1 +-
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f36dee4..d0f9745 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2465,13 +2465,41 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
 	return 0;
 }
 
+static struct symbol *symbol__new_unresolved(u64 addr, struct map *map)
+{
+	struct annotated_source *src;
+	struct symbol *sym;
+	char name[64];
+
+	snprintf(name, sizeof(name), "%.*" PRIx64, BITS_PER_LONG / 4, addr);
+
+	sym = symbol__new(addr, ANNOTATION_DUMMY_LEN, 0, 0, name);
+	if (sym) {
+		src = symbol__hists(sym, 1);
+		if (!src) {
+			symbol__delete(sym);
+			return NULL;
+		}
+
+		dso__insert_symbol(map->dso, sym);
+	}
+
+	return sym;
+}
+
 static int
 add_annotate_opt(struct hist_browser *browser __maybe_unused,
 		 struct popup_action *act, char **optstr,
-		 struct map_symbol *ms)
+		 struct map_symbol *ms,
+		 u64 addr)
 {
-	if (ms->sym == NULL || ms->map->dso->annotate_warned ||
-	    symbol__annotation(ms->sym)->src == NULL)
+	if (!ms->map || !ms->map->dso || ms->map->dso->annotate_warned)
+		return 0;
+
+	if (!ms->sym)
+		ms->sym = symbol__new_unresolved(addr, ms->map);
+
+	if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
 		return 0;
 
 	if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
@@ -3219,17 +3247,20 @@ do_hotkey:		 // key came straight from options ui__popup_menu()
 			nr_options += add_annotate_opt(browser,
 						       &actions[nr_options],
 						       &options[nr_options],
-						       &bi->from.ms);
+						       &bi->from.ms,
+						       bi->from.al_addr);
 			if (bi->to.ms.sym != bi->from.ms.sym)
 				nr_options += add_annotate_opt(browser,
 							&actions[nr_options],
 							&options[nr_options],
-							&bi->to.ms);
+							&bi->to.ms,
+							bi->to.al_addr);
 		} else {
 			nr_options += add_annotate_opt(browser,
 						       &actions[nr_options],
 						       &options[nr_options],
-						       browser->selection);
+						       browser->selection,
+						       browser->he_selection->ip);
 		}
 skip_annotation:
 		nr_options += add_thread_opt(browser, &actions[nr_options],
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 07c7759..2d88069 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -74,6 +74,7 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2);
 #define ANNOTATION__CYCLES_WIDTH 6
 #define ANNOTATION__MINMAX_CYCLES_WIDTH 19
 #define ANNOTATION__AVG_IPC_WIDTH 36
+#define ANNOTATION_DUMMY_LEN	256
 
 struct annotation_options {
 	bool hide_src_code,

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

end of thread, other threads:[~2020-04-04  8:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27  4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
2020-02-27  4:39 ` [PATCH v5 1/3] perf util: Print al_addr when symbol is not found Jin Yao
2020-04-04  8:42   ` [tip: perf/urgent] perf report: " tip-bot2 for Jin Yao
2020-02-27  4:39 ` [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols Jin Yao
2020-03-18 15:42   ` Arnaldo Carvalho de Melo
2020-03-18 15:43     ` Arnaldo Carvalho de Melo
2020-03-18 15:46       ` Arnaldo Carvalho de Melo
2020-03-19  1:11         ` Jin, Yao
2020-03-19  0:56       ` Jin, Yao
2020-03-19  0:49     ` Jin, Yao
2020-04-04  8:42   ` [tip: perf/urgent] " tip-bot2 for Jin Yao
2020-02-27  4:39 ` [PATCH v5 3/3] perf report: Support hotkey 'a' on address for annotation Jin Yao
2020-04-04  8:42   ` [tip: perf/urgent] perf report/top TUI: Support hotkey 'a' for annotation of unresolved addresses tip-bot2 for Jin Yao
2020-03-02  4:08 ` [PATCH v5 0/3] perf report: Support annotation of code without symbols Ravi Bangoria
2020-03-02 12:51 ` Jiri Olsa

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