Re: [PATCH 3/4] perf python: Add perfetto script to open profiles in the Perfetto UI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] perf python: NO_ID sentinel mismatch in process_sample()
  causes ValueError
--

commit cffb2c3ec0bc0ef022e32f9177d74617f3f52e47
Author: Lalit Maganti <lalitmaganti@gmail.com>

perf python: Add perfetto script to open profiles in the Perfetto UI

This commit adds a Python script that converts samples in a perf.data file
to a Perfetto trace and serves it to the Perfetto UI, along with a shell
test for the conversion.

> diff --git a/tools/perf/python/perfetto.py b/tools/perf/python/perfetto.py
> new file mode 100755
> index 0000000000000..e168b146e5385
> --- /dev/null
> +++ b/tools/perf/python/perfetto.py
> @@ -0,0 +1,625 @@

[ ... ]

> +# perf reports these when the sample type lacks PERF_SAMPLE_CPU/TID/TIME.
> +NO_CPU = 0xffffffff
> +NO_ID = 0xffffffff
> +NO_TIME = 0xffffffffffffffff

[ ... ]

> +    def process_sample(self, sample: perf.sample_event) -> None:
> +        """Writes a perf sample as a StackSample packet."""
> +        if sample.sample_time == NO_TIME:
> +            self.nr_skipped += 1
> +            return
> +        self.nr_samples += 1
> +        event = str(sample.evsel)
> +        if event.startswith('evsel(') and event.endswith(')'):
> +            event = event[len('evsel('):-1]
> +        seq = self._sequence(event, sample.sample_time)
> +
> +        stack_sample = b''
> +        pid, tid = sample.sample_pid, sample.sample_tid
> +        if NO_ID not in (pid, tid):
> +            self._update_names(pid, tid)
> +            stack_sample += varint_field(StackSampleField.TASK_CONTEXT_IID, seq.task(pid, tid))

[Severity: Medium]
Does checking NO_ID (0xffffffff) against sample_pid and sample_tid miss
samples without a valid PID or TID and raise a ValueError in varint()?

Unlike sample_cpu (T_UINT), the C binding exposes sample_pid and
sample_tid as signed T_INT:

tools/perf/util/python.c:
	sample_member_def(sample_pid, pid, T_INT, "event pid"),			 \
	sample_member_def(sample_tid, tid, T_INT, "event tid"),			 \

When a sample is recorded without PERF_SAMPLE_TID or for a task with no
PID in the event namespace, sample_pid and sample_tid are -1 rather than
0xffffffff.  That passes the NO_ID check and reaches seq.task() ->
varint_field() -> varint(-1), where out.append(-1) raises ValueError and
aborts the trace conversion.

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