Re: [PATCH v2 10/58] perf evlist: Add reference count

sashiko-bot
2026-04-23T04:19:44Z

Sashiko AI review found 2 potential issue(s):
- [High] Invalid free() on Python-embedded evsel objects causes heap corruption during evlist teardown
- [High] Python evlist item access returns a disconnected clone, discarding modifications and breaking object identity
--

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

perf evlist: Add reference count

This commit introduces a reference count to the evlist structure. The
reference count is initialized to 1 at allocation and decremented in the new
evlist__put() function, which performs the deletion when the count reaches 0.
This change prepares for future modifications to the Python bindings.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 1e6c99efff90e..3f0758d5bd904 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -1302,7 +1309,7 @@
>
> static void pyrf_evlist__delete(struct pyrf_evlist *pevlist)
> {
> - evlist__exit(&pevlist->evlist);
> + evlist__put(pevlist->evlist);
> Py_TYPE(pevlist)->tp_free((PyObject*)pevlist);
> }

Does calling evlist__put() here introduce a regression where evlist__purge()
will eventually attempt to call free() on an evsel that is embedded inside a
pyrf_evsel object allocated by Python's memory allocator?

The commit message mentions changes being broken apart, but could this
intermediate state break git bisectability by causing heap corruption during
teardown?

[ ... ]
> @@ -1794,17 +1817,16 @@ static PyObject *pyrf_evlist__item(PyObject *obj, Py_ssize_t i)
> struct pyrf_evlist *pevlist = (void *)obj;
> struct evsel *pos;
>
> - if (i >= pevlist->evlist.core.nr_entries) {
> + if (i >= pevlist->evlist->core.nr_entries) {
> PyErr_SetString(PyExc_IndexError, "Index out of range");
> return NULL;
> }
>
> - evlist__for_each_entry(&pevlist->evlist, pos) {
> + evlist__for_each_entry(pevlist->evlist, pos) {
> if (i-- == 0)
> break;
> }
> -
> - return Py_BuildValue("O", container_of(pos, struct pyrf_evsel, evsel));
> + return pyrf_evsel__from_evsel(pos);
> }

Does returning pyrf_evsel__from_evsel() here result in a disconnected clone
rather than the original wrapper?

If so, could this cause any modifications made to the returned object from
Python scripts to only affect the clone, leaving the actual evsel in the
list unchanged?

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