Re: [PATCH v2 07/19] perf test attr: Ignore the core PMU type perf puts in 'config'

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: KeyError crash in Event.equal() on missing event terms
--

commit 2d6e742f00c1817f447e64e8fed117f4dbcf95c2
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf test attr: Ignore the core PMU type perf puts in 'config'

Since commit 55bcf6ef314a in v5.13, and arm_pmu opted in with commit
5c816728651a in v6.6, the kernel can be told which core PMU a legacy type
event is meant for, with its type in the top 32 bits of 'config'. This patch
takes those bits out when comparing the 'config' of legacy type events so
expectations written for the single core PMU case continue matching on
heterogeneous machines.

> diff --git a/tools/perf/tests/shell/lib/attr.py b/tools/perf/tests/shell/lib/attr.py
> index f254d03c1b253..70ff73585cb3f 100644
> --- a/tools/perf/tests/shell/lib/attr.py
> +++ b/tools/perf/tests/shell/lib/attr.py

[ ... ]

> @@ -130,12 +156,33 @@ class Event(dict):
>          self.add(base)
>          self.add(data)
>  

[ ... ]

>      def equal(self, other):
>          for t in Event.terms:
>              log.debug("      [%s] %s %s", t, self[t], other[t])
>              if t not in self or t not in other:

[Severity: Medium]
In Event.equal(), log.debug() accesses self[t] and other[t] before the
boundary check executes:

    log.debug("      [%s] %s %s", t, self[t], other[t])
    if t not in self or t not in other:

Because the Event class inherits from the standard Python dict and does not
provide default values for missing keys, will accessing a non-existent key
here immediately raise an unhandled KeyError instead of gracefully returning
False?

>                  return False
> -            if not data_equal(self[t], other[t]):
> +            exp = self.hide_core_pmu_type(t, self[t], other)
> +            got = self.hide_core_pmu_type(t, other[t], other)
> +            if not data_equal(exp, got):
>                  return False
>          return True
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013201415.36428-1-acme@kernel.org?part=7