Re: [PATCH v6 30/59] perf flamegraph: Port flamegraph to use python module

sashiko-bot
2026-04-25T18:12:27Z

Sashiko AI review found 4 potential issue(s):
- [Medium] Python 3.9+ typing syntax will cause a TypeError on older Python versions.
- [Medium] Template injection vulnerability due to sequential string replacements.
- [Medium] Fallback symbol name resolution evaluates to None, breaking the JavaScript UI.
- [Low] Missing space in minimal HTML fallback warning message.
--

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

perf flamegraph: Port flamegraph to use python module

This commit adds a Python port of the flamegraph script that directly uses
the perf module. It avoids intermediate dictionaries to improve performance.

> diff --git a/tools/perf/python/flamegraph.py b/tools/perf/python/flamegraph.py
> new file mode 100755
> index 000000000000..b0eb5844b772
> --- /dev/null
> +++ b/tools/perf/python/flamegraph.py
[ ... ]
> +class Node:
> + """A node in the flame graph tree."""
> + def __init__(self, name: str, libtype: str):
> + self.name = name
> + self.libtype = libtype
> + self.value: int = 0
> + self.children: dict[str, Node] = {}

Could this typing syntax introduce a regression on older Python versions?

In Python versions prior to 3.9 (like Python 3.8), built-in types like dict
are not subscriptable. Because there is no
"from __future__ import annotations" directive, these type hints are
evaluated at runtime during class definition, which might prevent the
script from running entirely.

> +
> + def to_json(self) -> Dict[str, Union[str, int, list[Dict]]]:

Similarly, does using list[Dict] here cause a TypeError regression on older
Python versions?

[ ... ]
> + else:
> + # Fallback if no callchain
> + name = getattr(sample, "symbol", "[unknown]")

Is there a regression here where getattr returns None instead of "[unknown]"
when the symbol is unresolved?

The symbol property on perf.sample_event (defined in
pyrf_sample_event__getset) returns Py_RETURN_NONE when a symbol cannot be
resolved. Since the property exists on the object, getattr() would
successfully return None.

This None value would be assigned to the Node and serialized by json.dumps
as "n": null. The d3-flame-graph JavaScript library strictly expects the node
name to be a string and throws a TypeError when rendering, which would break
the flamegraph UI for unresolved symbols.

[ ... ]
> + if use_minimal:
> + print("Using internal minimal HTML that refers to d3's web site. JavaScript " +
> + "loaded this way from a local file may be blocked unless your " +
> + "browser has relaxed permissions. Run with '--allow-download' to fetch" +
> + "the full D3 HTML template.", file=sys.stderr)

This isn't a bug, but there's a missing space between "fetch" and "the" in
the warning message above, resulting in "fetchthe".

[ ... ]
> + assert output_str is not None
> + output_str = output_str.replace("/** @options_json **/", options_json)
> + output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)

Could this sequential replacement introduce a template injection regression?

The options_json string includes user-controlled fields, such as the --event
argument or the perf report header. If a user inputs the exact string
"/** @flamegraph_json **/" into these fields, the first replacement injects
this placeholder into the JavaScript options array literal.

The second replacement would then substitute stacks_json (which contains
unescaped double quotes) into this string literal, breaking the JavaScript
syntax and preventing the flamegraph UI from rendering.

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