[PATCH V9 0/2] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

[PATCH V9 0/2] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

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

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

-- 
2.44.0

[PATCH V9 2/2] Adjust objdump start/end range per map pgoff parameter

[Steve Clevenger]

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 <scclevenger@os.amperecomputing.com>
---
 tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
index 7aff02d84ffb..e29a4035723c 100755
--- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
+++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
@@ -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
 
 	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

Re: [PATCH V9 2/2] Adjust objdump start/end range per map pgoff parameter

[Leo Yan]

On 10/11/24 18:17, Steve Clevenger wrote:> 
> 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 <scclevenger@os.amperecomputing.com>
> ---
>   tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> index 7aff02d84ffb..e29a4035723c 100755
> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> @@ -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 <leo.yan@arm.com>

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

Re: [PATCH V9 2/2] Adjust objdump start/end range per map pgoff parameter

[Namhyung Kim]

Hi Leo,

On Thu, Oct 17, 2024 at 12:01:28PM +0100, Leo Yan wrote:
> On 10/11/24 18:17, Steve Clevenger wrote:>
> > 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 <scclevenger@os.amperecomputing.com>
> > ---
> >   tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++-----
> >   1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > index 7aff02d84ffb..e29a4035723c 100755
> > --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > @@ -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

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

> 
> With above fixing:
> 
> Reviewed-by: Leo Yan <leo.yan@arm.com>

Thanks for your review!

Steve, can you please send v10 with the fixes?

Thanks,
Namhyung

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

Re: [PATCH V9 2/2] Adjust objdump start/end range per map pgoff parameter

[Leo Yan]

On Thu, Oct 17, 2024 at 11:28:06AM -0700, Namhyung Kim wrote:

[...]

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

Yeah, this is fine for me.

Thanks,
Leo

[PATCH V10 RESEND 2/2] perf script python: Adjust objdump start/end range per map pgoff parameter

[Steve Clevenger]

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 <scclevenger@os.amperecomputing.com>
Reviewed-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
index 7aff02d84ffb..e29a4035723c 100755
--- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
+++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
@@ -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
 
 	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

[PATCH V9 1/2] Add map_pgoff to python dictionary

[Steve Clevenger]

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

Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
 tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index d7183134b669..367132b3a51a 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -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

Re: [PATCH V9 1/2] Add map_pgoff to python dictionary

[Leo Yan]

On 10/11/24 18:17, Steve Clevenger wrote:> 
> Add map_pgoff parameter to python dictionary so it can be seen by the
> python script.
> 
> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>

Sorry for late replying. LGTM:

Reviewed-by: Leo Yan <leo.yan@arm.com>

> ---
>   tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index d7183134b669..367132b3a51a 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -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
> 

Re: [PATCH V9 1/2] Add map_pgoff to python dictionary

[Leo Yan]

On 10/17/24 11:43, Leo Yan wrote:
> Warning: EXTERNAL SENDER, use caution when opening links or attachments.
> 
> 
> On 10/11/24 18:17, Steve Clevenger wrote:>
>> Add map_pgoff parameter to python dictionary so it can be seen by the
>> python script.
>>
>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> 
> Sorry for late replying. LGTM:

For this patch, please consider to add prefix in subject:

   perf script python: Add map_pgoff to python dictionary

With this, please feel free to add my reviewed tag.
  
> Reviewed-by: Leo Yan <leo.yan@arm.com>

[PATCH V10 RESEND 1/2] perf util scripting-engines cs-etm: Add map_pgoff to python dictionary

[Steve Clevenger]

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

Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
Reviewed-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index d7183134b669..367132b3a51a 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -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

Re: [PATCH V10 RESEND 1/2] perf util scripting-engines cs-etm: Add map_pgoff to python dictionary

[Namhyung Kim]

On Wed, Nov 06, 2024 at 04:51:10PM -0700, Steve Clevenger wrote:
> Add map_pgoff parameter to python dictionary so it can be seen by the
> python script.
> 
> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> ---
>  tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index d7183134b669..367132b3a51a 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -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));

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


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

Re: [PATCH V10 RESEND 1/2] perf util scripting-engines cs-etm: Add map_pgoff to python dictionary

[Steve Clevenger]

On 11/7/2024 10:51 AM, Namhyung Kim wrote:
> On Wed, Nov 06, 2024 at 04:51:10PM -0700, Steve Clevenger wrote:
>> Add map_pgoff parameter to python dictionary so it can be seen by the
>> python script.
>>
>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
>> Reviewed-by: Leo Yan <leo.yan@arm.com>
>> ---
>>  tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
>> index d7183134b669..367132b3a51a 100644
>> --- a/tools/perf/util/scripting-engines/trace-event-python.c
>> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
>> @@ -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));
> 
> 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.

Thanks,
Steve

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

Re: [PATCH V10 RESEND 1/2] perf util scripting-engines cs-etm: Add map_pgoff to python dictionary

[Ian Rogers]

On Fri, Nov 8, 2024 at 9:58 AM Steve Clevenger
<scclevenger@os.amperecomputing.com> wrote:
> On 11/7/2024 10:51 AM, Namhyung Kim wrote:
> > 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.

The map_pgoff was added for reference count checking which changes
struct layout when enabled. More context here:
https://perfwiki.github.io/main/reference-count-checking/

Thanks,
Ian

Re: [PATCH V9 0/2] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

[Namhyung Kim]

Hello,

On Fri, Oct 11, 2024 at 11:17:10AM -0600, Steve Clevenger wrote:
> 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

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

Re: [PATCH V9 0/2] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

[Steve Clevenger]

On 10/16/2024 10:51 AM, Namhyung Kim wrote:
> Hello,
> 
> On Fri, Oct 11, 2024 at 11:17:10AM -0600, Steve Clevenger wrote:
>> 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

It did. You can confirm there's no arm-cs-trace-python.py script change
from V8. Note the patch file numbering is different (since 2 patches
dropped). The trace-event-python.c patch file changed so I had to drop
out his "Reviewed-by" tag for that file. Due to the patch numbering
change, I didn't want to add confusion so I left it out.

Steve
> 
>>
>> 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(-)
>>
>> -- 
>> 2.44.0
>>

[PATCH V10 RESEND 0/2] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

[Steve Clevenger]

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.

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

-- 
2.44.0

Re: [PATCH V10 RESEND 0/2] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

[Leo Yan]

Hi Steve,

On Wed, Nov 06, 2024 at 04:51:11PM -0700, Steve Clevenger wrote:

[...]

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

The subject prefixes in two patches are not quite right.

The subjects would be:

  perf script python: Add map_pgoff to python dictionary
  perf script cs-etm: Adjust objdump start/end range per map pgoff parameter

With above change, you could keep my review tags.

Please use git commands to generate patches ('git commit --amend' and
'git format-patch').  If directly modify the plain patch files and resend
them, I suspect this is the reason that the archieve website messes up
rendering the patch series v9 and v10 [1].

Thanks,
Leo

[1] https://lore.kernel.org/linux-perf-users/cover.1728599785.git.scclevenger@os.amperecomputing.com/T/#t