Re: [PATCH v1 7/7] perf symbol: Do not set binary type from symtab type

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Oct 05, 2026 at 06:45:30AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] perf: `perf annotate` and guest module data read broken by missing `binary_type`
> --
> 
> commit 7196d300589913af007226f7522345f0d46a1af3
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf symbol: Do not set binary type from symtab type
> 
> The commit separates DSO binary type from symtab type and removes the
> logic that sets the binary type from the symtab type in dso__load.
> 
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 389dbd7e5192f..1175cb60f8206 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1956,19 +1956,6 @@ int dso__load(struct dso *dso, struct map *map)
> >  
> >  		if (next_slot) {
> >  			ss_pos++;
> > -
> > -			/*
> > -			 * The binary type is used to find the file containing
> > -			 * the executed instructions, so prefer the types that
> > -			 * refer to the actual object over debug-only files such
> > -			 * as DSO_BINARY_TYPE__DEBUGLINK.
> > -			 */
> > -			if (dso__binary_type(dso) == DSO_BINARY_TYPE__NOT_FOUND ||
> > -			    symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE ||
> > -			    (symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_DSO &&
> > -			     dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE))
> > -				dso__set_binary_type(dso, symtab_type);
> > -
> 
> [Severity: Medium]
> Will this removal cause perf annotate and guest module data reads to fail?

I'm not sure if perf annotate would work well for guest modules.

> 
> Without this logic, binary_type defaults to DSO_BINARY_TYPE__NOT_FOUND
> for generic DSOs, guest kernel modules, or binaries using debuginfo/system
> paths. 
> 
> When tools/perf/util/dso.c:try_to_open_dso() is later called to read data,
> it only iterates over BUILD_ID_CACHE and SYSTEM_PATH_DSO when the binary
> type is NOT_FOUND, completely ignoring guest-specific paths that require
> machine->root_dir:
> 
>     enum dso_binary_type binary_type_data[] = {
>         DSO_BINARY_TYPE__BUILD_ID_CACHE,
>         DSO_BINARY_TYPE__SYSTEM_PATH_DSO,
>         DSO_BINARY_TYPE__NOT_FOUND,
>     };

I believe guest modules always need machine->root_dir to access the
file.  It won't get the correct file contents without it so the current
code didn't work well.

> 
> Additionally, tools/perf/util/disasm.c:symbol__disassemble() explicitly
> returns an error if binary_type remains unset:
> 
>     if (dso__binary_type(dso) == DSO_BINARY_TYPE__NOT_FOUND) {
>         return SYMBOL_ANNOTATE_ERRNO__COULDNT_DETERMINE_FILE_TYPE;
>     }
> 
> Does this unintentionally break disassembly for binaries not in the
> build-id cache, or binary data reads for guest kernel modules?

As I said in other reply, it should be updated to use binary type of the
dso.  I'll update the code in v2.

Thanks,
Namhyung