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

Ruidong Tian posted 3 patches 2 years ago
.../scripts/python/arm-cs-trace-disasm.py     | 28 +++++++++++--------
1 file changed, 16 insertions(+), 12 deletions(-)
[PATCH 0/3] perf scripts python: arm-cs-trace-disasm.py:
Posted by Ruidong Tian 2 years ago
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 v3 0/1] perf scripts python: arm-cs-trace-disasm.py: print correct disasm info
Posted by Ruidong Tian 1 year, 11 months ago
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
Posted by Ruidong Tian 1 year, 11 months ago
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
Posted by James Clark 1 year, 11 months ago

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
Posted by Ruidong Tian 1 year, 11 months ago
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
Posted by James Clark 1 year, 11 months ago

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
Posted by Ruidong Tian 1 year, 11 months ago
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
[PATCH v2 0/1] perf scripts python: arm-cs-trace-disasm.py: print correct disasm info
Posted by Ruidong Tian 1 year, 11 months ago
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
Posted by Ruidong Tian 1 year, 11 months ago
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
Posted by Leo Yan 1 year, 11 months ago
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
Posted by Ruidong Tian 1 year, 11 months ago
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
>>
Re: [PATCH 0/3] perf scripts python: arm-cs-trace-disasm.py:
Posted by Arnaldo Carvalho de Melo 2 years ago
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