Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols

From: Arnaldo Carvalho de Melo
Date: Wed Mar 18 2020 - 11:42:14 EST

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@xxxxxxxxxxxxxxx>
> ---
> 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