2023-12-14 12:33:25

by Ruidong Tian

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

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


2023-12-14 12:33:36

by Ruidong Tian

[permalink] [raw]
Subject: [PATCH 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 <[email protected]>
---
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

2023-12-14 12:33:37

by Ruidong Tian

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

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 <[email protected]>
---
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

2023-12-14 12:33:47

by Ruidong Tian

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

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 <[email protected]>
---
.../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

2023-12-18 20:28:55

by Arnaldo Carvalho de Melo

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

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 <[email protected]>
> ---
> .../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

2023-12-20 10:45:03

by James Clark

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



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 <[email protected]>
> ---
> 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):

2023-12-20 11:16:29

by James Clark

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



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 <[email protected]>

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 <[email protected]>

> ---
> .../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:
> #
> # +------------+------------+------------+

2023-12-20 12:07:11

by James Clark

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



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 <[email protected]>
> ---
> 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 <[email protected]>

2023-12-20 17:31:20

by Arnaldo Carvalho de Melo

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

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

2023-12-22 07:29:50

by Ruidong Tian

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

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 <[email protected]>
>> ---
>> 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):

2023-12-24 08:33:49

by Leo Yan

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

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

2024-01-10 02:56:49

by Ruidong Tian

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

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/[email protected]/
Leo: https://lore.kernel.org/lkml/[email protected]/

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


2024-01-10 02:57:09

by Ruidong Tian

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

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 <[email protected]>
Signed-off-by: Leo Yan <[email protected]>
---
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


2024-01-10 12:59:33

by Leo Yan

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

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 <[email protected]>
> Signed-off-by: Leo Yan <[email protected]>

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 <[email protected]>

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
>

2024-01-10 13:51:06

by Ruidong Tian

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

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 <[email protected]>
>> Signed-off-by: Leo Yan <[email protected]>
> 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 <[email protected]>
>
> 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
>>

2024-01-16 02:09:30

by Ruidong Tian

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

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/[email protected]/
Leo: https://lore.kernel.org/lkml/[email protected]/

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


2024-01-16 02:09:56

by Ruidong Tian

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

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 <[email protected]>
---
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


2024-01-16 02:10:17

by Ruidong Tian

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

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 <[email protected]>
---
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


2024-01-17 10:12:37

by James Clark

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



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 <[email protected]>
> ---
> 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