[PATCH V7 4/4] Adjust objdump start/end range per map pgoff parameter

[Steve Clevenger]

On 9/6/2024 4:27 AM, Leo Yan wrote:
> 
> 
> On 9/5/24 23:28, 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 at os.amperecomputing.com>
>> ---
>>   tools/perf/scripts/python/arm-cs-trace-disasm.py | 14 +++++++++-----
>>   1 file changed, 9 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..a867e0db02b8 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")
> 
> 
> I am concerned the two sentences below are inconsistence: one uses
> 'start_addr + map_pgoff' and the other uses 'start_addr + int(map_pgoff)'.
> 

Valid point. It's working fine as is, but how is it even working? I look
at print_disam/read_disasm, and no type conversion is done in either
call. The dso_start parameter for these calls is actually dso_vm_start
which is the dso_start integer conversion.

> Here about below code?
> 
>            map_pgoff_str = get_optional(param_dict, "map_pgoff")
>        if map_pgoff_str == "":
>         map_pgoff = 0
>            else:
>         map_pgoff = int(map_pgoff_str)
> 
> With above change, 'map_pgoff' is an int type. As a result, the changes
> below can simply add 'map_pgoff'.

I propose:
	map_pgoff_str = get_optional(param_dict, "map_pgoff"
	if (map_pgoff_str.isdigit()):
	    map_pgoff = int(map_pgoff)
	else:
	    map_pgoff = 0

The int() conversions in the print() statement can then be removed.

Steve

> With these changes, LGTM:
> 
> Reviewed-by: Leo Yan <leo.yan at arm.com>
> 
> 
>>          cpu = sample["cpu"]
>>          ip = sample["ip"]
>> @@ -243,9 +244,11 @@ 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 and stop_addr=4.
>> +       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 +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 + int(map_pgoff), stop_addr +
>> int(map_pgoff)))
> 
>>
>>          print_srccode(comm, param_dict, sample, symbol, dso)
>> -- 
>> 2.44.0
>>