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

[Jin, Yao]

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