Re: [PATCH v6 36/59] perf export-to-sqlite: Port export-to-sqlite to use python module

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

Sashiko AI review found 4 potential issue(s):
- [Critical] Runtime Crash / Integer Overflow on kernel addresses.
- [High] Silent Data Loss / Incorrect Attribute Names for Sample Metrics.
- [High] Silent Data Loss / Missing API Surface.
- [High] Feature Regression / Hardcoded Data for Branch Stack.
--

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

perf export-to-sqlite: Port export-to-sqlite to use python module

This commit ports the export-to-sqlite.py script to use the modern perf
Python module and the standard library sqlite3 module. It drops the
dependency on PySide2.QtSql.

> diff --git a/tools/perf/python/export-to-sqlite.py b/tools/perf/python/export-to-sqlite.py
> new file mode 100755
> index 000000000000..736b56ff8d59
> --- /dev/null
> +++ b/tools/perf/python/export-to-sqlite.py

[ ... ]

> + # Insert sample
> + self.con.execute("""
> + INSERT INTO samples VALUES (
> + NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
> + )
> + """, (
> + self.get_event_id(getattr(sample.evsel, 'name', str(sample.evsel))),
> + 0, thread_id, comm_id,
> + dso_id, symbol_id, getattr(sample, 'sym_offset', 0),

Are sym_offset, transaction_, in_tx, and flags actually exposed by the perf
python module? It looks like they might be missing from the
pyrf_sample_event__members definition, which would cause getattr() to
silently return 0.

> + sample.sample_ip, sample.sample_time, sample.sample_cpu,

SQLite enforces a 64-bit signed integer limit for its INTEGER type. Will this
cause an OverflowError if sample.sample_ip contains a kernel address (which
often has the high bit set, making it too large for a signed 64-bit integer)?

> + 0, 0, 0, 0, # to_dso, to_symbol, to_sym_offset, to_ip

It looks like branch stack data is now completely hardcoded to 0. Does this
code skip processing the sample.brstack property, dropping branch analysis
data from the database?

> + getattr(sample, 'sample_period', 0) or 0,
> + getattr(sample, 'sample_weight', 0) or 0,
> + getattr(sample, 'transaction_', 0),
> + getattr(sample, 'data_src', 0),
> + 0, # branch_type
> + getattr(sample, 'in_tx', 0),
> + call_path_id,
> + getattr(sample, 'insn_count', 0),
> + getattr(sample, 'cyc_count', 0),

Are the property names correct for data_src, insn_count, and cyc_count?

Looking at tools/perf/util/python.c, it seems these should be prefixed with
sample_ (e.g., sample_data_src, sample_insn_count, sample_cyc_count).

Without the prefix, won't getattr() silently fall back to 0?

> + getattr(sample, 'flags', 0)
> + ))

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