| Message ID | 7bbd5ca482b052d1c52efc52742f5c2713d5c26c.1724698250.git.scclevenger@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset | expand |
On 8/26/2024 10:35 PM, Steve Clevenger wrote: > > Changes in V2: > - Updated mailing list distribution > > 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> > --- > .../perf/scripts/python/arm-cs-trace-disasm.py | 17 +++++++++++++++-- > 1 file changed, 15 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 d973c2baed1c..6bf806078f9a 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"] > @@ -250,13 +251,25 @@ 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 map_pgoff != None and map_pgoff != '[unknown]': > + if (dso == "[kernel.kallsyms]"): > + dso_vm_start = 0 > + map_pgoff = '[unknown]' > + else: > + dso_vm_start = int(dso_start) > + start_addr += map_pgoff > + stop_addr += map_pgoff > + map_pgoff = '[unknown]' Every sample has its own field `map_pgoff`. So I am confused why we need the sentence: map_pgoff = '[unknown]'. And you can see with above change, we will set dso_vm_start delicately with below code section. How about a simple change like below? @@ -267,7 +268,7 @@ def process_event(param_dict): 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) If this change does not work, I am curious if anything we missed to handle in the perf's C code (frontend). Thanks, Leo > if (options.objdump_name != None): > # It doesn't need to decrease virtual memory offset for disassembly > # for kernel dso and executable file dso, so in this case we set > -- > 2.25.1 >
On 8/27/2024 1:23 PM, Leo Yan wrote: > On 8/26/2024 10:35 PM, Steve Clevenger wrote: >> >> Changes in V2: >> - Updated mailing list distribution >> >> 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> >> --- >> .../perf/scripts/python/arm-cs-trace-disasm.py | 17 +++++++++++++++-- >> 1 file changed, 15 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 d973c2baed1c..6bf806078f9a 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"] >> @@ -250,13 +251,25 @@ 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 map_pgoff != None and map_pgoff != '[unknown]': >> + if (dso == "[kernel.kallsyms]"): >> + dso_vm_start = 0 >> + map_pgoff = '[unknown]' >> + else: >> + dso_vm_start = int(dso_start) >> + start_addr += map_pgoff >> + stop_addr += map_pgoff >> + map_pgoff = '[unknown]' > > Every sample has its own field `map_pgoff`. So I am confused why we need the > sentence: map_pgoff = '[unknown]'. And you can see with above change, we will > set dso_vm_start delicately with below code section. > > How about a simple change like below? > > @@ -267,7 +268,7 @@ def process_event(param_dict): > > 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) > > If this change does not work, I am curious if anything we missed to handle in > the perf's C code (frontend). > > Thanks, > Leo Leo, Sorry, the map_pgoff = '[unknown]' instances are actually leftover debug artifacts used for print type debug. I resorted to print debug after I didn't see success attempting mixed-mode debug with Visual Studio Code on AARCH64. If you have a known working launch configuration, please share it. See V3 of the patch series. I've rebased to the perf-tools-next branch. Steve > >> if (options.objdump_name != None): >> # It doesn't need to decrease virtual memory offset for disassembly >> # for kernel dso and executable file dso, so in this case we set >> -- >> 2.25.1 >>
diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py index d973c2baed1c..6bf806078f9a 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"] @@ -250,13 +251,25 @@ 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 map_pgoff != None and map_pgoff != '[unknown]': + if (dso == "[kernel.kallsyms]"): + dso_vm_start = 0 + map_pgoff = '[unknown]' + else: + dso_vm_start = int(dso_start) + start_addr += map_pgoff + stop_addr += map_pgoff + map_pgoff = '[unknown]' + if (options.objdump_name != None): # It doesn't need to decrease virtual memory offset for disassembly # for kernel dso and executable file dso, so in this case we set
Changes in V2: - Updated mailing list distribution 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> --- .../perf/scripts/python/arm-cs-trace-disasm.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)