Re: [PATCH v1 1/2] perf libdw: Pass dso_name explicitly to properly anchor split-debug builds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] State caching bug in dso__libdw_dwfl ignores dso_name and breaks split-debug resolution.
- [Low] Missing __maybe_unused attribute on the dso_name parameter in the libdw__addr2line() fallback stub.
--

--- Patch [1]: [PATCH v1 1/2] perf libdw: Pass dso_name explicitly to properly anchor split-debug builds ---
commit d6c05f5d9fb09508b13daaab6b752a240e96edf3
Author: Ian Rogers <irogers@google.com>

perf libdw: Pass dso_name explicitly to properly anchor split-debug builds

This commit modifies libdw__addr2line and dso__libdw_dwfl to accept an explicit
dso_name parameter. It aims to accurately anchor libdw to the dynamically
discovered split-debug file instead of the stripped executable.

> diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c
> index 4ca7e7e4fbe93..f53caee980b0b 100644
> --- a/tools/perf/util/libdw.c
> +++ b/tools/perf/util/libdw.c
> @@ -25,17 +25,17 @@ void dso__free_libdw(struct dso *dso)
>  	}
>  }
>  
> -struct Dwfl *dso__libdw_dwfl(struct dso *dso)
> +struct Dwfl *dso__libdw_dwfl(struct dso *dso, const char *dso_name)
>  {
>  	Dwfl *dwfl = dso__libdw(dso);
> -	const char *dso_name;
>  	Dwfl_Module *mod;
>  	int fd;
>  
>  	if (dwfl)
>  		return dwfl;

[Severity: Medium]
Does this early return prevent the newly passed dso_name from being used if a
Dwfl object is already cached?

For example, in tools/perf/arch/powerpc/util/skip-callchain-idx.c,
check_return_addr() calls:

    dwfl = dso__libdw_dwfl(dso, NULL);

This early call passes NULL for dso_name, which caches the stripped executable's
Dwfl. When get_srcline() later calls libdw__addr2line() with the newly
discovered split-debug file, won't this early return just provide the already
cached stripped executable's Dwfl and ignore the passed split-debug dso_name?

[ ... ]
> diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h
> index b120947374159..4bdd28d44e25e 100644
> --- a/tools/perf/util/libdw.h
> +++ b/tools/perf/util/libdw.h
[ ... ]
> @@ -40,7 +40,8 @@ void dso__free_libdw(struct dso *dso);
>  
>  #else /* HAVE_LIBDW_SUPPORT */
>  
> -static inline int libdw__addr2line(u64 addr __maybe_unused, char **file __maybe_unused,
> +static inline int libdw__addr2line(const char *dso_name, u64 addr __maybe_unused,
> +				   char **file __maybe_unused,

[Severity: Low]
Will this cause a build failure when compiling with HAVE_LIBDW_SUPPORT disabled
(e.g., NO_LIBDW=1)? The fallback stub introduces the dso_name parameter but
doesn't use it, and lacks the __maybe_unused attribute present on the other
unused parameters in the same signature.

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