Re: [PATCH v7 19/22] 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:
- [High] Logic error in preserving event encodings for architecture standard events: the script silently discards valid `EventCode: 0`, alternate encodings (e.g., `ConfigCode`), and all associated modifiers (e.g., `UMask`, `CounterIDMask`).
--

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

tools/perf: Support event code for arch standard events

This commit modifies the perf tool's json event parsing to preserve the event
encoding of architecture standard events. It skips overwriting the event code
if one is already provided 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:
> +          event = _arch_std_events[arch_std.lower()].event

[Severity: High]
Does this check properly handle an event code of 0 or alternate encodings?

If EventCode is explicitly 0, or if an alternate encoding like ConfigCode
is used (leaving eventcode as 0), the truthiness check will evaluate to true.

This would cause JsonEvent.__init__ to overwrite the newly constructed event
string with the base architecture standard event string, silently discarding
the explicit 0 encoding or alternate encodings, as well as any modifiers
appended beforehand like UMask.

Would it be safer to check dictionary membership directly, or initialize
eventcode to None to distinguish between an explicit 0 and an unset value?

>          # 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/20260622-counter_delegation-v7-0-0ba2fd34614e@meta.com?part=19