Re: [PATCH v2 45/58] perf stackcollapse: Port stackcollapse to use python module

sashiko-bot
2026-04-23T04:17:48Z

Sashiko AI review found 2 potential issue(s):
- [High] Unhandled TypeError crashes trace processing when a thread cannot be found.
- [High] Multi-threaded applications lose thread-specific comm breakdown due to passing `sample_pid` to `session.process()`.
--

commit 8033337e08bc60a928a1c42fc1269626cb2e0069
Author: Ian Rogers <irogers@google.com>

perf stackcollapse: Port stackcollapse to use python module

Modernize the legacy stackcollapse.py trace script by refactoring it
into a class-based architecture. The script uses perf.session for event
processing and aggregates call stacks to produce output suitable for flame
graphs.

> diff --git a/tools/perf/python/stackcollapse.py b/tools/perf/python/stackcollapse.py
> new file mode 100755
> index 0000000000000..fae0f0f503a35
> --- /dev/null
> +++ b/tools/perf/python/stackcollapse.py

[ ... ]

> + def process_event(self, sample: perf.sample_event) -> None:
> + """Collect call stack for each sample."""

[ ... ]

> + if self.args.include_comm:
> + if hasattr(self, 'session') and self.session:
> + comm = self.session.process(sample.sample_pid).comm()

When a thread cannot be found in the perf session (due to dropped events
or late starting), the C implementation pyrf_session__process() raises a
TypeError.

Since there is no try...except block around this call, will this unhandled
exception abort the session.process_events() loop and crash the script?

Other ported scripts like syscall-counts.py wrap this in a check for
TypeError and AttributeError to handle missing threads safely.

Additionally, does passing sample.sample_pid to session.process() cause
multi-threaded applications to lose their thread-specific comm breakdown?

In Linux perf, sample_pid is the process ID (TGID). Passing it forces the
lookup to return the process's main thread.

Sub-threads will have their call stacks collapsed under the main process
comm instead of their actual thread names.

> + else:
> + comm = "Unknown"
> + comm = comm.replace(" ", "_")

[ ... ]

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260423035526.1537178-1-irogers@google.com?part=45