[PATCH v3 0/2] perf report: Support annotation of code without symbols

[PATCH v3 0/2] perf report: Support annotation of code without symbols

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

 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 (2):
  perf util: Print al_addr when symbol is not found
  Support interactive annotation of code without symbols

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

-- 
2.17.1

[PATCH v3 1/2] perf util: Print al_addr when symbol is not found

[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

[PATCH v3 2/2] Support interactive annotation of code without symbols

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

 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

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jiri Olsa]

On Mon, Feb 24, 2020 at 10:22:25AM +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.
> 
> 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.
> 
>  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;
> +		}

hi,
I like the patchset:

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

could you please also check if we can do this earlier,
so the dummy symbol is actualy collecting all the hits?

like within the symbol__inc_addr_samples function,
but I mght be missing something..

thanks,
jirka

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Ravi Bangoria]

Hi Jin,

On 2/24/20 7:52 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.
> 
> 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.

If I press 'a' on address, it's not annotating. But if I annotate
by pressing enter, like you explained, it works. Is it intentional?

Ravi

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jiri Olsa]

On Mon, Feb 24, 2020 at 06:55:12PM +0530, Ravi Bangoria wrote:
> Hi Jin,
> 
> On 2/24/20 7:52 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.
> > 
> > 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.
> 
> If I press 'a' on address, it's not annotating. But if I annotate
> by pressing enter, like you explained, it works. Is it intentional?

I saw that too, but I thought it's unrelated issue,
because we played with that just recently

if you go through the 'enter' way and back, then the 
next time 'a' works ;-)

jirka

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Ravi Bangoria]

On 2/24/20 7:52 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.
> 
> 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)
This might be an add on...

Even though there are samples on a particular instruction, 'Percent' column
is empty, I guess, because we don't have symbol length?

Should we show 'global percent'? Or nr samples? Will it be useful?

Ravi

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Ravi Bangoria]

On 2/24/20 7:21 PM, Jiri Olsa wrote:
> On Mon, Feb 24, 2020 at 06:55:12PM +0530, Ravi Bangoria wrote:
>> Hi Jin,
>>
>> On 2/24/20 7:52 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.
>>>
>>> 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.
>>
>> If I press 'a' on address, it's not annotating. But if I annotate
>> by pressing enter, like you explained, it works. Is it intentional?
> 
> I saw that too, but I thought it's unrelated issue,
> because we played with that just recently
> 
> if you go through the 'enter' way and back, then the
> next time 'a' works ;-)

Yes.

I liked the series so I was trying out the patches. Just reporting issues I found...

jump/call arrows are also screwed up.. Like jump instruction is showing call arrows.
But these all are minor issues and can be fixed later on. I don't mind ;)

Ravi

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jin, Yao]

On 2/24/2020 8:35 PM, Jiri Olsa wrote:
> On Mon, Feb 24, 2020 at 10:22:25AM +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.
>>
>> 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.
>>
>>   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;
>> +		}
> 
> hi,
> I like the patchset:
> 
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> 
> could you please also check if we can do this earlier,
> so the dummy symbol is actualy collecting all the hits?
> 
> like within the symbol__inc_addr_samples function,
> but I mght be missing something..
> 
> thanks,
> jirka
> 

Thanks so much for like and ack this patchset!

For your suggestion, I had thought about the similar idea before. Maybe 
we can, but we need to process some cases.

Say the first address is 0x1000 and the dummy symbol size is 256. We 
create a new dummy symbol for this address (start address is 0x1000 and 
end address is 0x1100).

If the second address is 0x1010, we can't create a new dummy symbol for 
this address directly. On the contrary, we need to search the dummy 
symbol list by the address first. If the dummy symbol is found then 
reuse this symbol.

This idea is a bit more complicated than current patchset in 
implementation but it can collect the hits for the dummy symbols, that's 
the advantage. The advantage of current patchset is it's very simple. :)

Accept current patchset or rewrite for the new idea, both OK for me. :)

Thanks
Jin Yao

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jin, Yao]

On 2/24/2020 9:25 PM, Ravi Bangoria wrote:
> Hi Jin,
> 
> On 2/24/20 7:52 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.
>>
>> 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.
> 
> If I press 'a' on address, it's not annotating. But if I annotate
> by pressing enter, like you explained, it works. Is it intentional?
> 
> Ravi
> 

Oh, I often use the ENTER, I forget the hotkey 'a'. :(

Sorry about that...

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jin, Yao]

On 2/24/2020 9:56 PM, Ravi Bangoria wrote:
> 
> 
> On 2/24/20 7:52 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.
>>
>> 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)
> This might be an add on...
> 
> Even though there are samples on a particular instruction, 'Percent' column
> is empty, I guess, because we don't have symbol length?
> 
> Should we show 'global percent'? Or nr samples? Will it be useful?
> 
> Ravi
> 

As the mail I reply to Jiri, if we use the Jiri's idea, we can collect 
the hits for dummy symbols then we can show the percent of dummy symbol 
here.

Thanks
Jin Yao

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jin, Yao]

On 2/24/2020 10:04 PM, Ravi Bangoria wrote:
> 
> 
> On 2/24/20 7:21 PM, Jiri Olsa wrote:
>> On Mon, Feb 24, 2020 at 06:55:12PM +0530, Ravi Bangoria wrote:
>>> Hi Jin,
>>>
>>> On 2/24/20 7:52 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.
>>>>
>>>> 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.
>>>
>>> If I press 'a' on address, it's not annotating. But if I annotate
>>> by pressing enter, like you explained, it works. Is it intentional?
>>
>> I saw that too, but I thought it's unrelated issue,
>> because we played with that just recently
>>
>> if you go through the 'enter' way and back, then the
>> next time 'a' works ;-)
> 
> Yes.
> 
> I liked the series so I was trying out the patches. Just reporting 
> issues I found...
> 
> jump/call arrows are also screwed up.. Like jump instruction is showing 
> call arrows.
> But these all are minor issues and can be fixed later on. I don't mind ;)
> 
> Ravi
> 

Some jumps will show arrow '->". For example,

→ jne   120

Yes, we can check and fix that later.

Thanks
Jin Yao

Re: [PATCH v3 2/2] Support interactive annotation of code without symbols

[Jiri Olsa]

On Mon, Feb 24, 2020 at 11:39:43PM +0800, Jin, Yao wrote:

SNIP

> > > +	sym = symbol__new(addr, ANNOTATION_DUMMY_LEN, 0, 0, name);
> > > +	if (sym) {
> > > +		src = symbol__hists(sym, 1);
> > > +		if (!src) {
> > > +			symbol__delete(sym);
> > > +			return NULL;
> > > +		}
> > 
> > hi,
> > I like the patchset:
> > 
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
> > 
> > could you please also check if we can do this earlier,
> > so the dummy symbol is actualy collecting all the hits?
> > 
> > like within the symbol__inc_addr_samples function,
> > but I mght be missing something..
> > 
> > thanks,
> > jirka
> > 
> 
> Thanks so much for like and ack this patchset!
> 
> For your suggestion, I had thought about the similar idea before. Maybe we
> can, but we need to process some cases.
> 
> Say the first address is 0x1000 and the dummy symbol size is 256. We create
> a new dummy symbol for this address (start address is 0x1000 and end address
> is 0x1100).
> 
> If the second address is 0x1010, we can't create a new dummy symbol for this
> address directly. On the contrary, we need to search the dummy symbol list
> by the address first. If the dummy symbol is found then reuse this symbol.
> 
> This idea is a bit more complicated than current patchset in implementation
> but it can collect the hits for the dummy symbols, that's the advantage. The
> advantage of current patchset is it's very simple. :)

ah right, we dont know the symbol's size.. perhaps we could hold
histogram of unresolved addresses and use it later on when displaying
the annotation for the particular unresolved address.. but then
how to bucket the counts for accounting.. yep, it seems tricky ;-)

> 
> Accept current patchset or rewrite for the new idea, both OK for me. :)

I think you it can be build on top of this

thanks,
jirka

Re: [PATCH v3 0/2] perf report: Support annotation of code without symbols

[Arnaldo Carvalho de Melo]

Em Mon, Feb 24, 2020 at 10:22:23AM +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.
> 
> 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.

I'm considering this a new feature, so I'll leave this for perf/core,
i.e. for the next release, ok?

I'm now going thru the pure fixes that should go to perf/urgent, please
holler if something you think should go in now is not in my perf/urgent
branch.

- Arnaldo
 
>  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 (2):
>   perf util: Print al_addr when symbol is not found
>   Support interactive annotation of code without symbols
> 
>  tools/perf/ui/browsers/hists.c | 43 +++++++++++++++++++++++++++++-----
>  tools/perf/util/annotate.h     |  1 +
>  tools/perf/util/sort.c         |  6 +++--
>  3 files changed, 42 insertions(+), 8 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 

- Arnaldo

Re: [PATCH v3 0/2] perf report: Support annotation of code without symbols

[Jin, Yao]

On 2/27/2020 9:50 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 24, 2020 at 10:22:23AM +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.
>>
>> 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.
> 
> I'm considering this a new feature, so I'll leave this for perf/core,
> i.e. for the next release, ok?
> 

Yes, it's a new feature, so perf/core branch is OK. Thanks! :)

BTW, v3 is old, I just post v5 according to Jiri's comments.

> I'm now going thru the pure fixes that should go to perf/urgent, please
> holler if something you think should go in now is not in my perf/urgent
> branch.
>  > - Arnaldo
>

In summary, I have some patch series waiting but I think they can be in 
perf/core.

https://lkml.org/lkml/2020/2/2/83
https://lkml.org/lkml/2020/2/14/136
https://lkml.org/lkml/2020/2/19/1274
https://lkml.org/lkml/2020/2/26/1348

Following is a fix, maybe it can be in perf/urgent.

"perf stat: Align the output for interval aggregation mode"
https://lkml.org/lkml/2020/2/20/321

Thanks
Jin Yao

>>   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 (2):
>>    perf util: Print al_addr when symbol is not found
>>    Support interactive annotation of code without symbols
>>
>>   tools/perf/ui/browsers/hists.c | 43 +++++++++++++++++++++++++++++-----
>>   tools/perf/util/annotate.h     |  1 +
>>   tools/perf/util/sort.c         |  6 +++--
>>   3 files changed, 42 insertions(+), 8 deletions(-)
>>
>> -- 
>> 2.17.1
>>
>