Re: [PATCH v6 18/21] tools/perf: Support event code for arch standard events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `if not eventcode:` check incorrectly evaluates to True for explicit encodings where the event code is `0` or when alternative encodings (like `ConfigCode`) are used, causing them to be overwritten.
--

commit c082fe77b89457807536772c21ae88a13885568d
Author: Atish Patra <atishp@rivosinc.com>

tools/perf: Support event code for arch standard events

This commit updates the perf tool to support custom event encodings for
architecture standard events. It modifies jevents.py to avoid overwriting
the event encoding with the architecture standard event if an event code is
already present in the JSON file.

[ ... ]

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 3a1bcdcdc685f..457fce7a59820 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -413,7 +413,11 @@ class JsonEvent:
>          self.long_desc = None
>      if arch_std:
>        if arch_std.lower() in _arch_std_events:
> -        event = _arch_std_events[arch_std.lower()].event
> +        # If the JSON event already specified an event code, the encoding has
> +        # been set above; don't overwrite it with the arch standard event or
> +        # the event encoding would be lost.
> +        if not eventcode:
[Severity: Medium]
Does this condition incorrectly evaluate to True for valid 0 encodings?

If a JSON event explicitly specifies an event code of 0 (e.g.,
"EventCode": "0x0"), the parsed eventcode will be 0. Because Python evaluates
0 as false, the not eventcode condition evaluates to True.

This incorrectly overwrites the explicit encoding and discards previously
parsed event fields.

Can the same regression occur if alternative encodings like ConfigCode or
EventidCode are used, leaving the eventcode variable at 0?

Would it be safer to explicitly check if the eventcode is None to distinguish
between an absent event code and a literal zero?

> +          event = _arch_std_events[arch_std.lower()].event
>          # Copy from the architecture standard event to self for undefined fields.
>          for attr, value in _arch_std_events[arch_std.lower()].__dict__.items():
>            if hasattr(self, attr) and not getattr(self, attr):

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-counter_delegation-v6-0-285b72ed65a9@meta.com?part=18