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

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Oct 12, 2026 at 11:58:48PM +0000, sashiko-bot@kernel.org wrote:
> 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()?

Ok, I'll add that too.

Thanks,
Namhyung

> 
> > +			continue;
> > +		}
> > +
> > +		errno = 0;
> > +		found = filename__has_section(path, ".text");
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261012234654.60666-1-namhyung@kernel.org?part=6