lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 18 Mar 2020 12:42:06 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Jin Yao <yao.jin@...ux.intel.com>
Cc:     jolsa@...nel.org, peterz@...radead.org, mingo@...hat.com,
        alexander.shishkin@...ux.intel.com, Linux-kernel@...r.kernel.org,
        ak@...ux.intel.com, kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v5 2/3] perf report: Support interactive annotation of
 code without symbols

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@...ux.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ