On 9/9/2024 1:33 PM, Leo Yan wrote:
On 9/7/2024 12:20 AM, Steve Clevenger wrote:
[...]
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.
I agreed with you. After reading code, I have same conclusion that we don't need to type conversion to int type.
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.
For backward compatibility, we can use below code:
map_pgoff = get_optional(param_dict, "map_pgoff") if (map_pgoff == '[unknown]'): map_pgoff = 0
Then in the later flow, we should can always use "map_pgoff" as an int type.
I struggled for James reported issue.
The variables “map_pgoff,” “dso_start,” and “dso_end” are set together in the Python engine. All of them should be of type int if the DSO is found, or all should be ‘[unknown]’ if the DSO is missing. We have checked the types for “dso_start” and “dso_end”, and if either is ‘[unknown]’ the flow will directly bail out. Thus, in theory, “map_pgoff” will not cause trouble if it is an ‘[unknown]’ string.
One possibility is that James has applied your patches but has not built perf. So, the field “map_pgoff” is not passed from the Python engine, which will cause the reported error. I think the proposed above change can effectively avoid the error.
Thanks, Leo
I added that exact'[unknown]' check in last week. There's no need to explicitly convert to an integer although Python doesn't complain when about integer to integer conversions. It's probably just a no op. I'll commit the change later today.
Steve
-
Steve Clevenger