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

On 14/12/2023 12:33, Ruidong Tian wrote:

For exectable ELF file, which e_type is ET_EXEC, dso start address is a absolute address other than offset. Just set vm_start to zero when dso start is 0x400000, which means it is a exectable file.

Signed-off-by: Ruidong Tian tianruidong@linux.alibaba.com

tools/perf/scripts/python/arm-cs-trace-disasm.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index 46bf6b02eea1..c9e14af5b58c 100755 --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py @@ -260,8 +260,9 @@ def process_event(param_dict): if (options.objdump_name != None): # It doesn't need to decrease virtual memory offset for disassembly

• # for kernel dso, so in this case we set vm_start to zero.
  

• if (dso == "[kernel.kallsyms]"):
  

• # for kernel dso and executable file dso, so in this case we set
  

• # vm_start to zero.
  

• if (dso == "[kernel.kallsyms]" or dso_start == 0x400000):
  dso_vm_start = 0
  

  else: dso_vm_start = int(dso_start)

I confirmed that this fixes the disassembly for static binaries. It would have been nice to check the type of the file rather than using a magic number, but it's not that easy and I don't really see a chance of the number having a false positive.

I wonder if it's worth putting a fixes tag on this one? For the other ones I'd say no tag as they have a chance of breaking things.

Reviewed-by: James Clark james.clark@arm.com

Em Thu, Dec 14, 2023 at 08:33:04PM +0800, Ruidong Tian escreveu:

arm-cs-trace-disasm ignore disam the first branch sample, For example as follow, the instructions beteween 0x0000ffffae878750 and 0x0000ffffae878754 is lose:

Leo, Mathieu, Tor, Al, can you guys take a look and provide an Acked or Reviewed-by tag?

Thanks,

- Arnaldo

ARM CoreSight Trace Data Assembler Dump Event type: branches:uH Sample = { cpu: 0000 addr: 0x0000ffffae878750 phys_addr: 0x0000000000000000 ip: 0x0000000000000000 pid: 4003489 tid: 4003489 period: 1 time: 26765151766034 } Event type: branches:uH Sample = { cpu: 0000 addr: 0x0000000000000000 phys_addr: 0x0000000000000000 ip: 0x0000ffffae878754 pid: 4003489 tid: 4003489 period: 1 time: 26765151766034 }

Initialize cpu_data earlier to fix it:

ARM CoreSight Trace Data Assembler Dump Event type: branches:uH Sample = { cpu: 0000 addr: 0x0000000000000000 phys_addr: 0x0000000000000000 ip: 0x0000ffffae878754 pid: 4003489 tid: 4003489 period: 1 time: 26765151766034 } 0000000000028740 <ioctl>: (base address is 0x0000ffffae850000) 28750: b13ffc1f cmn x0, #4095 28754: 54000042 b.hs 0x2875c <ioctl+0x1c> test 4003489/4003489 [0000] 26765.151766034 __GI___ioctl+0x14 /usr/lib64/libc-2.32.so Event type: branches:uH Sample = { cpu: 0000 addr: 0x0000ffffa67535ac phys_addr: 0x0000000000000000 ip: 0x0000000000000000 pid: 4003489 tid: 4003489 period: 1 time: 26765151766034 }

Signed-off-by: Ruidong Tian tianruidong@linux.alibaba.com

.../scripts/python/arm-cs-trace-disasm.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index c9e14af5b58c..b1eb4293cbef 100755 --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py @@ -190,6 +190,17 @@ def process_event(param_dict): dso_end = get_optional(param_dict, "dso_map_end") symbol = get_optional(param_dict, "symbol")

• cpu = sample["cpu"]
• ip = sample["ip"]
• addr = sample["addr"]
• # Initialize CPU data if it's empty, and directly return back
• # if this is the first tracing event for this CPU.
• if (cpu_data.get(str(cpu) + 'addr') == None):

• cpu_data[str(cpu) + 'addr'] = addr
  

• return
  

• if (options.verbose == True): print("Event type: %s" % name) print_sample(sample)

@@ -211,16 +222,6 @@ def process_event(param_dict): if (name[0:8] != "branches"): return

• cpu = sample["cpu"]
• ip = sample["ip"]
• addr = sample["addr"]
• # Initialize CPU data if it's empty, and directly return back
• # if this is the first tracing event for this CPU.
• if (cpu_data.get(str(cpu) + 'addr') == None):

• cpu_data[str(cpu) + 'addr'] = addr
  

• return
  

• # The format for packet is: # # +------------+------------+------------+

-- 2.33.1

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

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

Add a option `-a` to print virtual offset other than offset:

# perf script -s scripts/python/arm-cs-trace-disasm.py -- -d llvm-objdump -a ... ffffb4c23200 <memcpy>: ffffb4c23200: d503201f nop ffffb4c23204: 8b020024 add x4, x1, x2 ffffb4c23208: 8b020005 add x5, x0, x2 ...

Signed-off-by: Ruidong Tian tianruidong@linux.alibaba.com Signed-off-by: Leo Yan leo.yan@linaro.org --- tools/perf/scripts/python/arm-cs-trace-disasm.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index d973c2baed1c..78419498237e 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(), "%x" % virt_addr) + print("\t" + line)

def print_sample(sample):

Hi Leo:

Thank you very much for your advice. I will remove your SoB and add 02 and 03 patch in V3.

在 2024/1/10 20:55, Leo Yan 写道:

Hi Ruidong,

On Wed, Jan 10, 2024 at 10:56:17AM +0800, Ruidong Tian wrote:

...

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

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

Add a option `-a` to print virtual offset other than offset:

 # perf script -s scripts/python/arm-cs-trace-disasm.py -- -d llvm-objdump -a
 ...
 ffffb4c23200 <memcpy>:
     ffffb4c23200: d503201f      nop
     ffffb4c23204: 8b020024      add     x4, x1, x2
     ffffb4c23208: 8b020005      add     x5, x0, x2
 ...

Signed-off-by: Ruidong Tian tianruidong@linux.alibaba.com Signed-off-by: Leo Yan leo.yan@linaro.org

I only gave suggestion, it's no need to add my SoB and this might break the SoB chain and rejected by maintainers.

So with removing my SoB, the patch is fine for me:

Reviewed-by: Leo Yan leo.yan@linaro.org

I would like to suggest you to resend patch set v2 with all patches

• though patches 02 and 03 have no any change, but it would be easier

for maintainers to pick up the whole patches (especially this can save time with b4 tool).

Thanks, Leo

...

---

tools/perf/scripts/python/arm-cs-trace-disasm.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index d973c2baed1c..78419498237e 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(), "%x" % virt_addr)
  

• print("\t" + line)

def print_sample(sample): -- 2.33.1

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

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

Add a option `-a` to print virtual offset other than offset:

# perf script -s scripts/python/arm-cs-trace-disasm.py -- -d llvm-objdump -a ... ffffb4c23200 <memcpy>: ffffb4c23200: d503201f nop ffffb4c23204: 8b020024 add x4, x1, x2 ffffb4c23208: 8b020005 add x5, x0, x2 ...

Signed-off-by: Ruidong Tian tianruidong@linux.alibaba.com Reviewed-by: Leo Yan leo.yan@linaro.org --- tools/perf/scripts/python/arm-cs-trace-disasm.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index d973c2baed1c..78419498237e 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(), "%x" % virt_addr) + print("\t" + line)

def print_sample(sample):

For exectable ELF file, which e_type is ET_EXEC, dso start address is a absolute address other than offset. Just set vm_start to zero when dso start is 0x400000, which means it is a exectable file.

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

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index 46bf6b02eea1..c9e14af5b58c 100755 --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py @@ -260,8 +260,9 @@ def process_event(param_dict):

if (options.objdump_name != None): # It doesn't need to decrease virtual memory offset for disassembly - # for kernel dso, so in this case we set vm_start to zero. - if (dso == "[kernel.kallsyms]"): + # for kernel dso and executable file dso, so in this case we set + # vm_start to zero. + if (dso == "[kernel.kallsyms]" or dso_start == 0x400000): dso_vm_start = 0 else: dso_vm_start = int(dso_start)

For exectable ELF file, which e_type is ET_EXEC, dso start address is a absolute address other than offset. Just set vm_start to zero when dso start is 0x400000, which means it is a exectable file.

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

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index 46bf6b02eea1..c9e14af5b58c 100755 --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py @@ -260,8 +260,9 @@ def process_event(param_dict):

if (options.objdump_name != None): # It doesn't need to decrease virtual memory offset for disassembly - # for kernel dso, so in this case we set vm_start to zero. - if (dso == "[kernel.kallsyms]"): + # for kernel dso and executable file dso, so in this case we set + # vm_start to zero. + if (dso == "[kernel.kallsyms]" or dso_start == 0x400000): dso_vm_start = 0 else: dso_vm_start = int(dso_start)