Re: [PATCH 3/3] perf jevents: Check the JSON keys that jevents.py never reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf jevents: JsonEvent.__init__() suppresses mistyped keys
  matching a sibling hint
--

commit 618a7a7e812ef95bd110f0becbecdb78f73a1397
Author: TAKANO Mitsuhiro <takano32@gmail.com>

perf jevents: Check the JSON keys that jevents.py never reads

This commit adds a --check mode to jevents.py, a jevents-check make target,
and a shell test to report unread JSON keys that are near misses for keys
read by jevents.py, as well as metrics missing a MetricExpr.

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 844fa82887bbf..3063282599a00 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
[ ... ]
> @@ -416,6 +484,42 @@ class JsonEvent:
>          raise argparse.ArgumentTypeError('Cannot find arch std event:', arch_std)
>  
>      self.event = real_event(self.name, event)
> +    # Keys of the JSON object that no code above looked at, set last so that
> +    # the copy of an architecture standard event above does not overwrite it.
> +    # A name that is a near miss for one the code reads, and whose name the
> +    # object does not carry with the same text, is almost certainly a
> +    # misspelling of it: the object lost the field the reader was looking for.
> +    # A name that is not a near miss is left alone, because a table is free to
> +    # carry metadata jevents.py has no use for.
> +    self.mistyped: Optional[Dict[str, Tuple[str, ...]]] = None
> +    if not _track:
> +      return
> +    assert isinstance(jd, _JsonObject)
> +    unread = jd.unread
> +    if unread:
> +      found = {}
> +      for key in unread:
> +        if key not in _hints:
> +          lowered = {name.lower(): name for name in _read_keys}
> +          # Two names can be equally close - RetirementLatencyMax and its Min -
> +          # and naming one of them would be a coin toss presented as an answer.
> +          match = difflib.get_close_matches(key.lower(), sorted(lowered), n=2, cutoff=0.8)
> +          _hints[key] = tuple(lowered[m] for m in match)
> +        hints = _hints[key]
> +        if not hints:
> +          continue
> +        # Nothing was lost if the object carries one of those names with the
> +        # same text: that is a table with the field twice, not a misspelling.
> +        # dict.__contains__ so that the look is not recorded as a read.
> +        lost = True
> +        for name in hints:
> +          if dict.__contains__(jd, name) and dict.get(jd, key) == dict.get(jd, name):
> +            lost = False
> +            break

[Severity: Low]
Does setting lost = False when any name in hints has the same value in jd
hide mistyped sibling keys?

Because get_close_matches() collects up to two matches (n=2), hints for a
typo in a closely-named sibling field (such as RetirementLatencyMax alongside
RetirementLatencyMin) contains both sibling names.  If both fields have the
same value on an event object and one key is mistyped, the other valid
sibling in hints matches in jd, setting lost = False so --check silently
misses the typo even though that field was dropped from the generated event.

In addition, json.load() already collapses identically-spelled duplicate JSON
keys into a single dictionary entry that is removed from jd.unread when read,
so any key remaining in jd.unread is spelled differently from every key
jevents.py reads and suppresses a misspelled duplicate key whenever the valid
key has the same value.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110222250.1864777-1-takano32@gmail.com?part=3