Re: [PATCH v4 06/10] perf tools: Factor out dso__find_binary_type()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: spurious FD-eviction and DSO failure due to uninitialized errno
--

commit 00af7deeab2857b031fe6b82dcee452b2fe4afb8
Author: Namhyung Kim <namhyung@kernel.org>

perf tools: Factor out dso__find_binary_type()

This commit separates out the logic to detect the binary type of a DSO by
checking if the candidate file contains a .text section. It ignores
data-only DSOs and adds handling to retry with the DSO data open lock if the
search fails due to hitting the open file limit.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 913877b84b2d4..c42ad0c5c66a7 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
[ ... ]
> +	do {
> +		dso__set_binary_type(dso, binary_type_data[i++]);
>  
> -		dso_data->fd = open_dso(dso, machine);
> -		if (dso_data->fd >= 0)
> -			goto out;
> +		path = dso__get_filename(dso, "", &decomp, dso__binary_type(dso));
> +		if (path == NULL) {
> +			if (errno == EMFILE)
> +				goto open_err;

[Severity: Medium]
Could this lead to spurious FD evictions and failure to find the binary type
if errno is already EMFILE from a prior, unrelated operation?

If dso__get_filename() returns NULL for a normal reason (such as a missing
build-id cache) without touching errno, it appears a stale EMFILE would cause
an early abort to the open_err label.

Since the caller __dso__find_binary_type() evicts a DSO and retries up to 3
times, and errno would remain EMFILE across those retries, it seems this might
permanently mark the DSO binary type as NOT_FOUND.

Would it make sense to clear errno before calling dso__get_filename()?

> +			continue;
> +		}
> +
> +		errno = 0;
> +		found = filename__has_section(path, ".text");
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012234654.60666-1-namhyung@kernel.org?part=6