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

[Steve Clevenger]

On 9/6/2024 10:27 AM, Steve Clevenger wrote:
> 
> 
> 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.

Python thinks the map_pgoff object is already an integer. For example,
	map_pgoff = get_optional(param_dict, "map_pgoff")
	print("%d" % map_pgoff.isdigit())

AttributeError: 'int' object has no attribute 'isdigit'
Fatal Python error: handler_call die: problem in Python trace event handler

Converting map_pgoff to a string works in the print statement.

print("%d" % str(map_pgoff).isdigit())
1

I'm not sure, but it's possible get_optional() during some call had
converted it to '[unknown]' which would cause a problem.

I can explicitly force to integer.

Steve

> 
>> 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
>>>
> 
> _______________________________________________
> CoreSight mailing list -- coresight at lists.linaro.org
> To unsubscribe send an email to coresight-leave at lists.linaro.org