arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

Add map_pgoff parameter to python dictionary so it can be seen by the python script.

Signed-off-by Steve Clevenger <[email protected]>

---
  tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +6 -4

index d7183134b669..367132b3a51a 100644
@@ -793,7 +793,8 @@ static int set_regs_in_dict(PyObject *dict,
  static void set_sym_in_dict(PyObject *dict, struct addr_location *al,
                             const char *dso_field, const char *dso_bid_field,
                             const char *dso_map_start, const char *dso_map_end,
-                           const char *sym_field, const char *symoff_field)
+                           const char *sym_field, const char *symoff_field,
+                           const char *map_pgoff)
  {
         char sbuild_id[SBUILD_ID_SIZE];
 

@@ -809,6 +810,8 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al,
                         PyLong_FromUnsignedLong(map__start(al->map)));
                 pydict_set_item_string_decref(dict, dso_map_end,
                         PyLong_FromUnsignedLong(map__end(al->map)));
+               pydict_set_item_string_decref(dict, map_pgoff,
+                       PyLong_FromUnsignedLongLong(al->map->pgoff));
         }
         if (al->sym) {
                 pydict_set_item_string_decref(dict, sym_field,

@@ -895,7 +898,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,
         pydict_set_item_string_decref(dict, "comm",
                         _PyUnicode_FromString(thread__comm_str(al->thread)));
         set_sym_in_dict(dict, al, "dso", "dso_bid", "dso_map_start", "dso_map_end",
-                       "symbol", "symoff");
+                       "symbol", "symoff", "map_pgoff");
 
         pydict_set_item_string_decref(dict, "callchain", callchain);
 

@@ -920,7 +923,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,
                         PyBool_FromLong(1));
                 set_sym_in_dict(dict_sample, addr_al, "addr_dso", "addr_dso_bid",
                                 "addr_dso_map_start", "addr_dso_map_end",
-                               "addr_symbol", "addr_symoff");
+                               "addr_symbol", "addr_symoff", "addr_map_pgoff");
         }
 
         if (sample->flags)
--

2.44.0

Warning: EXTERNAL SENDER, use caution when opening links or attachments.

On 10/11/24 18:17, Steve Clevenger wrote:>

Quoted 4 lines

Add map_pgoff parameter to python dictionary so it can be seen by the python script.

Signed-off-by Steve Clevenger <[email protected]>

Sorry for late replying. LGTM:

Reviewed-by Leo Yan <[email protected]>

Extract map_pgoff parameter from the dictionary, and adjust start/end range passed to objdump based on the value.

A zero start_addr is filtered to prevent output of dso address range check failures. This script repeatedly sees a zero value passed in for
      start_addr = cpu_data[str(cpu) + 'addr']

These zero values are not a new problem. The start_addr/stop_addr warning clutters the instruction trace output, hence this change.

Signed-off-by Steve Clevenger <[email protected]>

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

tools/perf/scripts/python/arm-cs-trace-disasm.py +0 -0

index 7aff02d84ffb..e29a4035723c 100755

tools/perf/scripts/python/arm-cs-trace-disasm.py +7 -6

         cpu = sample["cpu"]
         ip = sample["ip"]

@@ -243,9 +247,10 @@ def process_event(param_dict):
         # Record for previous sample packet
         cpu_data[str(cpu) + 'addr'] = addr
 
-       # Handle CS_ETM_TRACE_ON packet if start_addr=0 and stop_addr=4
-       if (start_addr == 0 and stop_addr == 4):
-               print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
+       # Filter out zero start_address. Optionally identify CS_ETM_TRACE_ON packet
+       if (start_addr == 0):
+               if ((stop_addr == 4) and (options.verbose == True)):
+                       print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
                 return
 
         if (start_addr < int(dso_start) or start_addr > int(dso_end)):

@@ -262,13 +267,14 @@ def process_event(param_dict):
                 # vm_start to zero.
                 if (dso == "[kernel.kallsyms]" or dso_start == 0x400000):
                         dso_vm_start = 0
+                       map_pgoff = 0
                 else:
                         dso_vm_start = int(dso_start)
 
                 dso_fname = get_dso_file_path(dso, dso_bid)
                 if path.exists(dso_fname):
-                       print_disam(dso_fname, dso_vm_start, start_addr, stop_addr)
+                       print_disam(dso_fname, dso_vm_start, start_addr + map_pgoff, stop_addr + map_pgoff)
                 else:
-                       print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr))
+                       print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr + map_pgoff, stop_addr + map_pgoff))
 
         print_srccode(comm, param_dict, sample, symbol, dso)
--

2.44.0

On 10/11/24 18:17, Steve Clevenger wrote:>

Quoted 21 lines

Extract map_pgoff parameter from the dictionary, and adjust start/end range passed to objdump based on the value.

A zero start_addr is filtered to prevent output of dso address range check failures. This script repeatedly sees a zero value passed in for
      start_addr = cpu_data[str(cpu) + 'addr']

These zero values are not a new problem. The start_addr/stop_addr warning clutters the instruction trace output, hence this change.

Signed-off-by Steve Clevenger <[email protected]>

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

tools/perf/scripts/python/arm-cs-trace-disasm.py +0 -0

index 7aff02d84ffb..e29a4035723c 100755

tools/perf/scripts/python/arm-cs-trace-disasm.py def process_event(param_dict): · line 193

Quoted 7 lines

tools/perf/scripts/python/arm-cs-trace-disasm.py +4 -0

@@ -187,6 +187,10 @@ def process_event(param_dict):
         dso_start = get_optional(param_dict, "dso_map_start")
         dso_end = get_optional(param_dict, "dso_map_end")
         symbol = get_optional(param_dict, "symbol")
+       map_pgoff = get_optional(param_dict, "map_pgoff")
+       # check for valid map offset
+       if (str(map_pgoff) == '[unknown]'):
+               map_pgoff = 0

I tried to apply this patch, it reports warning:

Applying: Adjust objdump start/end range per map pgoff parameter .git/rebase-apply/patch:16: space before tab in indent.
map_pgoff = 0
warning: 1 line applied after fixing whitespace errors.

This means it has unexpected spaces in above line.

When applying patches, I found the prefix is missed in the subject:

perf scripts python cs-etm: Adjust objdump start/end range per map pgoff parameter

With above fixing:

Reviewed-by Leo Yan <[email protected]>

Quoted 37 lines

tools/perf/scripts/python/arm-cs-trace-disasm.py +7 -6

         cpu = sample["cpu"]
         ip = sample["ip"]

@@ -243,9 +247,10 @@ def process_event(param_dict):
         # Record for previous sample packet
         cpu_data[str(cpu) + 'addr'] = addr
 
-       # Handle CS_ETM_TRACE_ON packet if start_addr=0 and stop_addr=4
-       if (start_addr == 0 and stop_addr == 4):
-               print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
+       # Filter out zero start_address. Optionally identify CS_ETM_TRACE_ON packet
+       if (start_addr == 0):
+               if ((stop_addr == 4) and (options.verbose == True)):
+                       print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
                 return
 
         if (start_addr < int(dso_start) or start_addr > int(dso_end)):

@@ -262,13 +267,14 @@ def process_event(param_dict):
                 # vm_start to zero.
                 if (dso == "[kernel.kallsyms]" or dso_start == 0x400000):
                         dso_vm_start = 0
+                       map_pgoff = 0
                 else:
                         dso_vm_start = int(dso_start)
 
                 dso_fname = get_dso_file_path(dso, dso_bid)
                 if path.exists(dso_fname):
-                       print_disam(dso_fname, dso_vm_start, start_addr, stop_addr)
+                       print_disam(dso_fname, dso_vm_start, start_addr + map_pgoff, stop_addr + map_pgoff)
                 else:
-                       print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr))
+                       print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr + map_pgoff, stop_addr + map_pgoff))
 
         print_srccode(comm, param_dict, sample, symbol, dso)
--

2.44.0

Quoted 3 lines

When applying patches, I found the prefix is missed in the subject:

perf scripts python cs-etm: Adjust objdump start/end range per map pgoff parameter

It could simply be "perf script cs-etm:" but it's up to you. :)

Changes in V9:
- Removed V8 patch files 1/4 and 2/4.
- Modified set_sym_in_dict (trace-event-python.c) to add map_pgoff in dictionary as-is without regard to MAPPING_IDENTITY. This patch file is now patch 2/2.

Changes in V8:
- in arm-cs-trace-disasm.py, ensure map_pgoff is not converted to string.
- Remove map_pgoff integer conversion in dso not found print message.

Changes in V7:
- In arm-cs-trace-disasm.py, fix print message core dump resulting from mixed type arithmetic.
- Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The CS_ETM_TRACE_ON message is changed to print only in verbose mode.
- Removed verbose mode only notification for start_addr/stop_addr outside of dso address range.

Changes in V6:
- In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add map_pgoff to start/end address for dso not found message.
- Added "Reviewed-by" trailer for patches 1-3 previously reviewed by Leo Yan in V4 and V5.

Changes in V5:
- In symbol-elf.c, branch to exit_close label if open file.
- In trace_event_python.c, correct indentation. set_sym_in_dict call parameter "map_pgoff" renamed as "addr_map_pgoff" to match local naming.

Changes in V4:
- In trace-event-python.c, fixed perf-tools-next merge problem.

Changes in V3:
- Rebased to linux-perf-tools branch.
- Squash symbol-elf.c and symbol.h into same commit.
- In map.c, merge dso__is_pie() call into existing if statement.
- In arm-cs-trace-disasm.py, remove debug artifacts.

Changes in V2:
- In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer checks per Leo Yan review.
- Updated mailing list distribution.

Steve Clevenger (2):
Add map_pgoff to python dictionary
Adjust objdump start/end address per map_pgoff parameter

 tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++-----
 .../util/scripting-engines/trace-event-python.c  |  9 ++++++---
 2 files changed, 17 insertions(+), 8 deletions(-)

Signature · 1 line

2.44.0

Hello,

Quoted 5 lines · Steve Clevenger

Changes in V9:
- Removed V8 patch files 1/4 and 2/4.
- Modified set_sym_in_dict (trace-event-python.c) to add map_pgoff in dictionary as-is without regard to MAPPING_IDENTITY. This patch file is now patch 2/2.

I think the previous version had Leo's Reviewed-by tag.

Leo, can you confirm if it still holds?

Thanks,
Namhyung

Quoted 53 lines

Changes in V8:
- in arm-cs-trace-disasm.py, ensure map_pgoff is not converted to string.
- Remove map_pgoff integer conversion in dso not found print message.

Changes in V7:
- In arm-cs-trace-disasm.py, fix print message core dump resulting from mixed type arithmetic.
- Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The CS_ETM_TRACE_ON message is changed to print only in verbose mode.
- Removed verbose mode only notification for start_addr/stop_addr outside of dso address range.

Changes in V6:
- In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add map_pgoff to start/end address for dso not found message.
- Added "Reviewed-by" trailer for patches 1-3 previously reviewed by Leo Yan in V4 and V5.

Changes in V5:
- In symbol-elf.c, branch to exit_close label if open file.
- In trace_event_python.c, correct indentation. set_sym_in_dict call parameter "map_pgoff" renamed as "addr_map_pgoff" to match local naming.

Changes in V4:
- In trace-event-python.c, fixed perf-tools-next merge problem.

Changes in V3:
- Rebased to linux-perf-tools branch.
- Squash symbol-elf.c and symbol.h into same commit.
- In map.c, merge dso__is_pie() call into existing if statement.
- In arm-cs-trace-disasm.py, remove debug artifacts.

Changes in V2:
- In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer checks per Leo Yan review.
- Updated mailing list distribution.

Steve Clevenger (2):
Add map_pgoff to python dictionary
Adjust objdump start/end address per map_pgoff parameter

 tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++-----
 .../util/scripting-engines/trace-event-python.c  |  9 ++++++---
 2 files changed, 17 insertions(+), 8 deletions(-)

Signature · 1 line

2.44.0

Changes in V10:
- Removed errant space in patch file 0002. Passed 'git apply --check'

    at perf-tools-next, 6.11.0-rc6.

- Added back missing prefixes.

Add map_pgoff parameter to python dictionary so it can be seen by the python script.

Signed-off-by Steve Clevenger <[email protected]>

Reviewed-by Leo Yan <[email protected]>

---
 tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +2 -1

index d7183134b669..367132b3a51a 100644
@@ -793,7 +793,8 @@ static int set_regs_in_dict(PyObject *dict,
 static void set_sym_in_dict(PyObject *dict, struct addr_location *al,
 			    const char *dso_field, const char *dso_bid_field,
 			    const char *dso_map_start, const char *dso_map_end,
-			    const char *sym_field, const char *symoff_field)
+			    const char *sym_field, const char *symoff_field,
+			    const char *map_pgoff)
 {
 	char sbuild_id[SBUILD_ID_SIZE];
 

tools/perf/util/scripting-engines/trace-event-python.c +2 -2

 	}
 	if (al->sym) {
 		pydict_set_item_string_decref(dict, sym_field,

@@ -895,7 +898,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,
 	pydict_set_item_string_decref(dict, "comm",
 			_PyUnicode_FromString(thread__comm_str(al->thread)));
 	set_sym_in_dict(dict, al, "dso", "dso_bid", "dso_map_start", "dso_map_end",
-			"symbol", "symoff");
+			"symbol", "symoff", "map_pgoff");
 
 	pydict_set_item_string_decref(dict, "callchain", callchain);
 

@@ -920,7 +923,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,
 			PyBool_FromLong(1));
 		set_sym_in_dict(dict_sample, addr_al, "addr_dso", "addr_dso_bid",
 				"addr_dso_map_start", "addr_dso_map_end",
-				"addr_symbol", "addr_symoff");
+				"addr_symbol", "addr_symoff", "addr_map_pgoff");
 	}
 
 	if (sample->flags)

Signature · 1 line

2.44.0

Quoted 24 lines · Steve Clevenger

Add map_pgoff parameter to python dictionary so it can be seen by the python script.

Signed-off-by Steve Clevenger <[email protected]>

Reviewed-by Leo Yan <[email protected]>

---
 tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

tools/perf/util/scripting-engines/trace-event-python.c +2 -1

index d7183134b669..367132b3a51a 100644
@@ -793,7 +793,8 @@ static int set_regs_in_dict(PyObject *dict,
 static void set_sym_in_dict(PyObject *dict, struct addr_location *al,
 			    const char *dso_field, const char *dso_bid_field,
 			    const char *dso_map_start, const char *dso_map_end,
-			    const char *sym_field, const char *symoff_field)
+			    const char *sym_field, const char *symoff_field,
+			    const char *map_pgoff)
 {
 	char sbuild_id[SBUILD_ID_SIZE];
 

tools/perf/util/scripting-engines/trace-event-python.c static void set_sym_in_dict(PyObject *dict, struct addr_location *al, · line 814

Quoted 5 lines

tools/perf/util/scripting-engines/trace-event-python.c +2 -0

@@ -809,6 +810,8 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al,
 			PyLong_FromUnsignedLong(map__start(al->map)));
 		pydict_set_item_string_decref(dict, dso_map_end,
 			PyLong_FromUnsignedLong(map__end(al->map)));
+		pydict_set_item_string_decref(dict, map_pgoff,
+			PyLong_FromUnsignedLongLong(al->map->pgoff));

Please use map__pgoff(al->map) instead. Otherwise you'll get this in the debug build:

util/scripting-engines/trace-event-python.c: In function 'set_sym_in_dict':
util/scripting-engines/trace-event-python.c:814:60: error: 'struct map' has no member named 'pgoff'

    814 |                         PyLong_FromUnsignedLongLong(al->map->pgoff));   
        |                                                            ^~

Thanks,
Namhyung

Quoted 25 lines

tools/perf/util/scripting-engines/trace-event-python.c +2 -2

 	}
 	if (al->sym) {
 		pydict_set_item_string_decref(dict, sym_field,

@@ -895,7 +898,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,
 	pydict_set_item_string_decref(dict, "comm",
 			_PyUnicode_FromString(thread__comm_str(al->thread)));
 	set_sym_in_dict(dict, al, "dso", "dso_bid", "dso_map_start", "dso_map_end",
-			"symbol", "symoff");
+			"symbol", "symoff", "map_pgoff");
 
 	pydict_set_item_string_decref(dict, "callchain", callchain);
 

@@ -920,7 +923,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample,
 			PyBool_FromLong(1));
 		set_sym_in_dict(dict_sample, addr_al, "addr_dso", "addr_dso_bid",
 				"addr_dso_map_start", "addr_dso_map_end",
-				"addr_symbol", "addr_symoff");
+				"addr_symbol", "addr_symoff", "addr_map_pgoff");
 	}
 
 	if (sample->flags)

Signature · 1 line

2.44.0

Quoted 11 lines · Namhyung Kim

Please use map__pgoff(al->map) instead. Otherwise you'll get this in the debug build:

util/scripting-engines/trace-event-python.c: In function 'set_sym_in_dict':
util/scripting-engines/trace-event-python.c:814:60: error: 'struct map' has no member named 'pgoff'

    814 |                         PyLong_FromUnsignedLongLong(al->map->pgoff));
        |                                                            ^~

Thanks,
Namhyung

Hi Namhyung,

I do not see this compile error, and I typically build perf with debug. This make command works through 6.11.0-rc6:

$ make -C tools/perf DEBUG=1 VF=1 CORESIGHT=1 PYTHON=python3
CONFIG_LIBPYTHON=y CSLIBS=/usr/lib CSINCLUDES=/usr/include install

I can substitute in the map__pgoff() macro in any case.