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

sashiko-bot
2026-04-24T17:23:26Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Unhandled exception in stackcollapse.py when resolving missing threads crashes the script
--

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

perf stackcollapse: Port stackcollapse to use python module

This commit modernizes the legacy stackcollapse.py trace script by refactoring
it into a class-based StackCollapseAnalyzer architecture. It uses perf.session
for event processing and aggregates call stacks to produce flame graph output.

> diff --git a/tools/perf/python/stackcollapse.py b/tools/perf/python/stackcollapse.py
> new file mode 100755
> index 00000000000000..fae0f0f503a354
> --- /dev/null
> +++ b/tools/perf/python/stackcollapse.py
[ ... ]
> @@ -0,0 +1,126 @@
> +#!/usr/bin/env python3
[ ... ]
> + def process_event(self, sample: perf.sample_event) -> None:
> + """Collect call stack for each sample."""
> + stack = []
> + callchain = getattr(sample, "callchain", None)
> + if callchain is not None:
> + for node in callchain:
> + stack.append(self.tidy_function_name(node.symbol, node.dso))
> + else:
> + # Fallback if no callchain
> + sym = getattr(sample, "symbol", "[unknown]")
> + dso = getattr(sample, "dso", "[unknown]")
> + stack.append(self.tidy_function_name(sym, dso))
> +
> + if self.args.include_comm:
> + if hasattr(self, 'session') and self.session:
> + comm = self.session.process(sample.sample_pid).comm()

Could this crash the script if a thread cannot be found or if its comm
string is missing?

If the underlying C API (pyrf_session__process) fails to find the thread
for the given PID, it explicitly raises a TypeError. Similarly, if the
thread has no valid comm string, pyrf_thread__comm() can raise a SystemError.

Since there is no try...except block around this call, will an exception
from a lost event or short-lived process propagate to the C wrapper and
abruptly abort perf_session__process_events()?

> + else:
> + comm = "Unknown"
> + comm = comm.replace(" ", "_")
> + sep = "-"
> + if self.args.include_pid:
> + comm = f"{comm}{sep}{getattr(sample, 'sample_pid', 0)}"
[ ... ]

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