[1/3] perf scripts python: arm-cs-trace-disasm.py: print dso base address

  arm-cs-trace-disasm just print offset for library dso:

	0000000000002200 <memcpy>:
            2200: d503201f      nop
            2204: 8b020024      add     x4, x1, x2
            2208: 8b020005      add     x5, x0, x2

This print DSO base address to get complete virtual address for
userspace application:

	0000000000002200 <memcpy>: (base address is 0x0000ffffb4c21000)
            2200: d503201f      nop
            2204: 8b020024      add     x4, x1, x2
            2208: 8b020005      add     x5, x0, x2

Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
---
 tools/perf/scripts/python/arm-cs-trace-disasm.py | 2 ++
 1 file changed, 2 insertions(+)
  

On 14/12/2023 12:33, Ruidong Tian wrote:
> arm-cs-trace-disasm just print offset for library dso:
> 
> 	0000000000002200 <memcpy>:
>             2200: d503201f      nop
>             2204: 8b020024      add     x4, x1, x2
>             2208: 8b020005      add     x5, x0, x2
> 
> This print DSO base address to get complete virtual address for
> userspace application:
> 
> 	0000000000002200 <memcpy>: (base address is 0x0000ffffb4c21000)
>             2200: d503201f      nop
>             2204: 8b020024      add     x4, x1, x2
>             2208: 8b020005      add     x5, x0, x2
> 

I believe the output format without the base address is consistent with
objdump. For compatibility I would say that it's better to keep it that way.

We could add this as an option, but have it disabled by default. I
suppose it depends how likely that someone is using this output in a
tool and processing it further whether an option is needed or not.

Although it's also not that clear what this is useful for, given that
all the other output is relative too? Maybe you could add an example to
the commit message, even if it's just for debugging. Would an option
that turned _all_ the output into virtual addresses not be more useful?

Thanks
James

> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
> ---
>  tools/perf/scripts/python/arm-cs-trace-disasm.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> index d59ff53f1d94..46bf6b02eea1 100755
> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> @@ -108,6 +108,8 @@ def print_disam(dso_fname, dso_start, start_addr, stop_addr):
>  			m = disasm_re.search(line)
>  			if m is None:
>  				continue
> +		else:
> +			line += " (base address is 0x%016x)" % dso_start
>  		print("\t" + line)
>  
>  def print_sample(sample):
  

Hi James

在 2023/12/20 18:44, James Clark 写道:
>
> On 14/12/2023 12:33, Ruidong Tian wrote:
>> arm-cs-trace-disasm just print offset for library dso:
>>
>> 	0000000000002200 <memcpy>:
>>              2200: d503201f      nop
>>              2204: 8b020024      add     x4, x1, x2
>>              2208: 8b020005      add     x5, x0, x2
>>
>> This print DSO base address to get complete virtual address for
>> userspace application:
>>
>> 	0000000000002200 <memcpy>: (base address is 0x0000ffffb4c21000)
>>              2200: d503201f      nop
>>              2204: 8b020024      add     x4, x1, x2
>>              2208: 8b020005      add     x5, x0, x2
>>
> I believe the output format without the base address is consistent with
> objdump. For compatibility I would say that it's better to keep it that way.
Sure, the output is totally corrected, i just print the base address 
rather than modify the output address, i don't think this change is 
incompatible.
>
> We could add this as an option, but have it disabled by default. I
> suppose it depends how likely that someone is using this output in a
> tool and processing it further whether an option is needed or not.
I want to get the runtime trace flow with virtual address, i can get the 
trace information with virtual address in `perf report` and `perf 
script` by default, so i think it is more reasonable to print virtual 
address in arm-cs-trace-disasm script by default.
>
> Although it's also not that clear what this is useful for, given that
> all the other output is relative too? Maybe you could add an example to
> the commit message, even if it's just for debugging. Would an option
> that turned _all_ the output into virtual addresses not be more useful?
I want to use arm-cs-trace-disasm.py output associate with PMU and SPE 
data to explore more CPU performance info, all the PMU/SPE/Coresight 
informations generated by `perf report` and `perf script` include 
virtual address, so i want this script do the same thing.
>
> Thanks
> James
>
>> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
>> ---
>>   tools/perf/scripts/python/arm-cs-trace-disasm.py | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> index d59ff53f1d94..46bf6b02eea1 100755
>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> @@ -108,6 +108,8 @@ def print_disam(dso_fname, dso_start, start_addr, stop_addr):
>>   			m = disasm_re.search(line)
>>   			if m is None:
>>   				continue
>> +		else:
>> +			line += " (base address is 0x%016x)" % dso_start
>>   		print("\t" + line)
>>   
>>   def print_sample(sample):
  

Hi Ruidong,

On Fri, Dec 22, 2023 at 03:29:18PM +0800, Ruidong Tian wrote:

[...]

> > Although it's also not that clear what this is useful for, given that
> > all the other output is relative too? Maybe you could add an example to
> > the commit message, even if it's just for debugging. Would an option
> > that turned _all_ the output into virtual addresses not be more useful?
>
> I want to use arm-cs-trace-disasm.py output associate with PMU and SPE data
> to explore more CPU performance info, all the PMU/SPE/Coresight informations
> generated by `perf report` and `perf script` include virtual address, so i
> want this script do the same thing.

I think James' suggestion is valid for replacing all offsets with
virtual addresses, in addition to introducing a new option.

Below change works well at my side. Hope this is helpful.

  diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
  index de58991c78bb..6c94ff2287cd 100755
  --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
  +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
  @@ -36,7 +36,10 @@ option_list = [
                      help="Set path to objdump executable file"),
          make_option("-v", "--verbose", dest="verbose",
                      action="store_true", default=False,
  -                   help="Enable debugging log")
  +                   help="Enable debugging log"),
  +       make_option("-a", "--vaddr", dest="vaddr",
  +                   action="store_true", default=False,
  +                   help="Enable virtual address")
   ]
  
   parser = OptionParser(option_list=option_list)
  @@ -108,6 +111,14 @@ def print_disam(dso_fname, dso_start, start_addr, stop_addr):
                          m = disasm_re.search(line)
                          if m is None:
                                  continue
  +
  +               # Replace offset with virtual address
  +               if (options.vaddr == True):
  +                       offset = re.search(r"^\s*([0-9a-fA-F]+)", line).group()
  +                       if offset:
  +                               virt_addr = dso_start + int(offset, 16)
  +                               line = line.replace(offset.lstrip(), "0x%016x" % virt_addr)
  +
                  print("\t" + line)
  
   def print_sample(sample):

Thanks,
Leo