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

["Jin, Yao" <yao.jin@linux.intel.com>]

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