| Message ID | 110501d82b12ea7909eb7d02899ef60ea42c7e19.1725493961.git.scclevenger@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset | expand |
On 05/09/2024 1:11 am, Steve Clevenger wrote: > Extract map_pgoff parameter from the dictionary, and adjust start/end > range passed to objdump based on the value. > > The start_addr/stop_addr address checks are changed to print a warning > only if verbose == True. 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 | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 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..35a2ab60ca12 100755 > --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py > +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py > @@ -187,6 +187,7 @@ 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") > > cpu = sample["cpu"] > ip = sample["ip"] > @@ -249,11 +250,13 @@ def process_event(param_dict): > return > > if (start_addr < int(dso_start) or start_addr > int(dso_end)): > - print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) > + if (options.verbose == True): > + print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) > return > > if (stop_addr < int(dso_start) or stop_addr > int(dso_end)): > - print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) > + if (options.verbose == True): > + print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) What about skipping printing if the address is 0 rather than if it's not verbose? Seems like a zero is expected because that's for discontinuities, but other non zero addresses that don't match are a genuine issue. And there is already an early exit for zero addresses above, maybe that just needs to be fixed to make it more general. But this one probably does need the verbosity change: # 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) return Also minor nit, but I think this change in verbosity is a separate change to the change to add the map offset. > return > > if (options.objdump_name != None): > @@ -262,13 +265,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)) I get this error here for this line: $ perf script -k vmlinux -s python:tools/perf/scripts/python/arm-cs\ -trace-disasm.py -- -d -k vmlinux Traceback (most recent call last): File "tools/perf/scripts/python/arm-cs-trace-disasm.py", line 331, in process_event print_disam(dso_fname, dso_vm_start, start_addr + map_pgoff, stop_addr + map_pgoff) TypeError: unsupported operand type(s) for +: 'int' and 'str' Fatal Python error: handler_call_die: problem in Python trace event handler Python runtime state: initialized Current thread 0x000075db4c249400 (most recent call first): <no Python frame> Extension modules: perf_trace_context, apt_pkg (total: 2) Aborted (core dumped)
On 9/5/2024 6:45 AM, James Clark wrote: > > > On 05/09/2024 1:11 am, Steve Clevenger wrote: >> Extract map_pgoff parameter from the dictionary, and adjust start/end >> range passed to objdump based on the value. >> >> The start_addr/stop_addr address checks are changed to print a warning >> only if verbose == True. 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 | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 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..35a2ab60ca12 100755 >> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py >> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py >> @@ -187,6 +187,7 @@ 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") >> cpu = sample["cpu"] >> ip = sample["ip"] >> @@ -249,11 +250,13 @@ def process_event(param_dict): >> return >> if (start_addr < int(dso_start) or start_addr > int(dso_end)): >> - print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] >> for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) >> + if (options.verbose == True): >> + print("Start address 0x%x is out of range [ 0x%x .. >> 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) >> return >> if (stop_addr < int(dso_start) or stop_addr > int(dso_end)): >> - print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for >> dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) >> + if (options.verbose == True): >> + print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] >> for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) > > What about skipping printing if the address is 0 rather than if it's not > verbose? Seems like a zero is expected because that's for > discontinuities, but other non zero addresses that don't match are a > genuine issue. > > And there is already an early exit for zero addresses above, maybe that > just needs to be fixed to make it more general. But this one probably > does need the verbosity change: > > # 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) > return > > Also minor nit, but I think this change in verbosity is a separate > change to the change to add the map offset. In my experience, non-zero start addresses outside the dso address range are exceedingly rare. I don't recall the last time I saw one. To avoid having to pick through thousands of generated messages with zero start addresses, I'll filter these out. I also don't recall ever seeing the CS_ETM_TRACE_ON notification message embedded in the instruction trace output. And a typical trace capture has many thousands of TRACE_ON packets. I only have Ampere HW to use for CoreSight trace, but how valid is the test with start_addr=0 and stop_addr=4? I'll change the script to optionally print the message in verbose mode for these conditions. > >> return >> if (options.objdump_name != None): >> @@ -262,13 +265,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)) > > I get this error here for this line: > > $ perf script -k vmlinux -s python:tools/perf/scripts/python/arm-cs\ > -trace-disasm.py -- -d -k vmlinux > > Traceback (most recent call last): > File "tools/perf/scripts/python/arm-cs-trace-disasm.py", line 331, in > process_event > print_disam(dso_fname, dso_vm_start, start_addr + map_pgoff, stop_addr > + map_pgoff) > TypeError: unsupported operand type(s) for +: 'int' and 'str' > Fatal Python error: handler_call_die: problem in Python trace event > handler > Python runtime state: initialized > > Current thread 0x000075db4c249400 (most recent call first): > <no Python frame> > > Extension modules: perf_trace_context, apt_pkg (total: 2) > Aborted (core dumped) My mistake. I intermixed the types, and didn't catch it during test. I Look for Version 7 to address your comments. Steve C.
diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index 7aff02d84ffb..35a2ab60ca12 100755 --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py @@ -187,6 +187,7 @@ 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") cpu = sample["cpu"] ip = sample["ip"] @@ -249,11 +250,13 @@ def process_event(param_dict): return if (start_addr < int(dso_start) or start_addr > int(dso_end)): - print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) + if (options.verbose == True): + print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso)) return if (stop_addr < int(dso_start) or stop_addr > int(dso_end)): - print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) + if (options.verbose == True): + print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) return if (options.objdump_name != None): @@ -262,13 +265,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)
Extract map_pgoff parameter from the dictionary, and adjust start/end range passed to objdump based on the value. The start_addr/stop_addr address checks are changed to print a warning only if verbose == True. 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 | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)