[PATCH 0/3] perf scripts python: arm-cs-trace-disasm.py:

[PATCH 0/3] perf scripts python: arm-cs-trace-disasm.py:

[Ruidong Tian]

Now the instruction flow disasmed by arm-cs-trace-disasm.py is
ambiguous and uncorrect, fix them in one patch set. Please refer to
each patch for details.

Ruidong Tian (3):
  perf scripts python: arm-cs-trace-disasm.py: print dso base address
  perf scripts python: arm-cs-trace-disasm.py: set start vm addr of
    exectable file to 0
  perf scripts python: arm-cs-trace-disasm.py: do not ignore disam first
    sample

 .../scripts/python/arm-cs-trace-disasm.py     | 28 +++++++++++--------
 1 file changed, 16 insertions(+), 12 deletions(-)

-- 
2.33.1

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

[Ruidong Tian]

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(+)

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):
-- 
2.33.1

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

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

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

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

[Ruidong Tian]

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

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

[Leo Yan]

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

[PATCH 2/3] perf scripts python: arm-cs-trace-disasm.py: set start vm addr of exectable file to 0

[Ruidong Tian]

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

Re: [PATCH 2/3] perf scripts python: arm-cs-trace-disasm.py: set start vm addr of exectable file to 0

[James Clark]

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>

[PATCH 3/3] perf scripts python: arm-cs-trace-disasm.py: do not ignore disam first sample

[Ruidong Tian]

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

  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

Re: [PATCH 3/3] perf scripts python: arm-cs-trace-disasm.py: do not ignore disam first sample

[Arnaldo Carvalho de Melo]

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

-- 

- Arnaldo

Re: [PATCH 3/3] perf scripts python: arm-cs-trace-disasm.py: do not ignore disam first sample

[James Clark]

On 14/12/2023 12:33, Ruidong Tian wrote:
> arm-cs-trace-disasm ignore disam the first branch sample, For example as
> follow, the instructions beteween 0x0000ffffae878750 and
> 0x0000ffffae878754 is lose:
> 
>   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>

I noticed that this is for the missing second branch sample. Technically
the first one is also still missing, but it doesn't have the origin set,
only the destination, so I'm not sure if we need to do anything with it,
but the first one always looks like this:

  0 [unknown] ([unknown]) =>     ffff8a3b9100 _start+0x0

Followed by this one which you now generate the disassembly for:

  ffff8a3b9104 _start+0x4 (ld-2.31.so) => ffff8a3b9b80 _dl_start+0x0

Either way:

Reviewed-by: James Clark <james.clark@arm.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:
>  	#
>  	#		  +------------+------------+------------+

Re: [PATCH 0/3] perf scripts python: arm-cs-trace-disasm.py:

[Arnaldo Carvalho de Melo]

Em Thu, Dec 14, 2023 at 08:33:01PM +0800, Ruidong Tian escreveu:
> Now the instruction flow disasmed by arm-cs-trace-disasm.py is
> ambiguous and uncorrect, fix them in one patch set. Please refer to
> each patch for details.
> 
> Ruidong Tian (3):
>   perf scripts python: arm-cs-trace-disasm.py: print dso base address
>   perf scripts python: arm-cs-trace-disasm.py: set start vm addr of
>     exectable file to 0
>   perf scripts python: arm-cs-trace-disasm.py: do not ignore disam first
>     sample

Picked patches 2 and 3, waiting for further discussion about the other
one.

- Arnaldo

[PATCH v2 0/1] perf scripts python: arm-cs-trace-disasm.py: print correct disasm info

[Ruidong Tian]

Now the instruction flow disasmed by arm-cs-trace-disasm.py is
ambiguous and uncorrect, fix them in one patch set. Please refer to
each patch for details.

Changes since v1:

Applied all the suggestions from James and Leo of v1.

James: https://lore.kernel.org/lkml/912a39f4-025e-26a1-7786-091fa211f293@arm.com/
Leo: https://lore.kernel.org/lkml/20231224083332.GB13521@leoy-huanghe.lan/

Ruidong Tian (1):
  perf scripts python: arm-cs-trace-disasm.py: add option to print
    virtual address

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

-- 
2.33.1

[PATCH v2 1/1] perf scripts python: arm-cs-trace-disasm.py: add option to print virtual address

[Ruidong Tian]

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):
-- 
2.33.1

Re: [PATCH v2 1/1] perf scripts python: arm-cs-trace-disasm.py: add option to print virtual address

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

Re: [PATCH v2 1/1] perf scripts python: arm-cs-trace-disasm.py: add option to print virtual address

[Ruidong Tian]

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

[PATCH v3 0/1] perf scripts python: arm-cs-trace-disasm.py: print correct disasm info

[Ruidong Tian]

Now the instruction flow disasmed by arm-cs-trace-disasm.py is
ambiguous and uncorrect, fix them in one patch set. Please refer to
each patch for details.

Changes since v2:

Leo: Replace Leo's SoB with Reviewed-by, and resend 02 and 03 patches.
https://lore.kernel.org/lkml/20240110125544.GG44@debian-dev/

Changes since v1:

Applied all the suggestions from James and Leo of v1.

James: https://lore.kernel.org/lkml/912a39f4-025e-26a1-7786-091fa211f293@arm.com/
Leo: https://lore.kernel.org/lkml/20231224083332.GB13521@leoy-huanghe.lan/

Ruidong Tian (1):
  perf scripts python: arm-cs-trace-disasm.py: add option to print
    virtual address

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

-- 
2.33.1

[PATCH v3 1/1] perf scripts python: arm-cs-trace-disasm.py: add option to print virtual address

[Ruidong Tian]

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):
-- 
2.33.1

Re: [PATCH v3 1/1] perf scripts python: arm-cs-trace-disasm.py: add option to print virtual address

[James Clark]

On 16/01/2024 02:08, 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>
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

I think your s-o-b should always come last, so these should be the other
way around.

Also patch 3 is missing on v3 for some reason so you might want to
resend. No need to send it as a reply to the thread, you can just send
it as a complete new one.

Thanks
James

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

[PATCH v3 2/3] perf scripts python: arm-cs-trace-disasm.py: set start vm addr of exectable file to 0

[Ruidong Tian]

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

Re: [PATCH v3 2/3] perf scripts python: arm-cs-trace-disasm.py: set start vm addr of exectable file to 0

[James Clark]

On 16/01/2024 02:08, 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)

Hi Ruidong,

Please pick up my review tag from V1 for this patch if you resend. You
can apply the review tags automatically with the b4 tool.

Thanks
James

[PATCH v3 2/3] perf scripts python: arm-cs-trace-disasm.py: set start vm addr of exectable file to 0

[Ruidong Tian]

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