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

[sashiko-bot@kernel.org]

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?

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,
    };

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?

>  			if (syms_ss && runtime_ss)
>  				break;
>  		} else {

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