Re: [PATCH v2 02/19] perf test attr: Allow tests to require a core PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf python: memory leak of parse_events_error in pyrf__parse_events()
--

commit f47ff7c9fc4b5bf17c150eab6410f6c98f01cb7c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf test attr: Allow tests to require a core PMU

On machines without a usable core PMU, like an aarch64 box with no DT
node for the ARM PMU, the expectations that pin the legacy hardware
encoding can't be checked: without a core PMU at all 'cycles' doesn't
even resolve, and with one that can't be opened 'perf record' takes the
software fallback and records attributes they can never match.

> diff --git a/tools/perf/tests/shell/lib/attr.py b/tools/perf/tests/shell/lib/attr.py
> index 68f06df9565a0..b55c9b881e598 100644
> --- a/tools/perf/tests/shell/lib/attr.py
> +++ b/tools/perf/tests/shell/lib/attr.py

[ ... ]

> @@ -28,6 +29,34 @@ def data_equal(a, b):
>  
>      return False
>  
> +def core_pmu_available():
> +    """
> +    Is there a core PMU on this machine? Without one hardware events aren't
> +    supported and get replaced by the software fallback, so there's no way to
> +    check the expectations that pin their legacy encodings.
> +    """
> +    try:
> +        import perf
> +        parse_events = perf.parse_events
> +    except (ImportError, AttributeError):
> +        # Nothing to ask about it, let the expectations speak.
> +        return True
> +
> +    try:
> +        evlist = parse_events('cycles')
> +    except OSError:
> +        # No PMU that can count it, the same as the ENOENT below.
> +        return False
> +

[Severity: Medium]
Does this expected failure trigger a memory leak of the parse_events_error
structure in the underlying C extension?

When parse_events() fails or generates warnings, it dynamically allocates
memory for error strings. However, looking at the python bindings in
tools/perf/util/python.c:

tools/perf/util/python.c:pyrf__parse_events() {
	...
	parse_events_error__init(&err);
	perf_evlist__set_maps(evlist__core(evlist), cpus, threads);
	if (parse_events(evlist, input, &err)) {
		parse_events_error__print(&err, input);
		PyErr_SetFromErrno(PyExc_OSError);
		evlist__put(evlist);
		return NULL;
	}
	result = pyrf_evlist__from_evlist(evlist);
	evlist__put(evlist);
	return result;
}

The err structure is dynamically populated but parse_events_error__exit(&err)
is never called on either the success or error paths, leaking memory whenever
errors or warnings are generated.

Could pyrf__parse_events() be updated to clean up the error structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013201415.36428-1-acme@kernel.org?part=2