Re: [PATCH v4 08/10] 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: fully stripped executables falsely rejected for data access
--

commit 6b9e46855d4e0d8df1dcaaed600da2f693471adb
Author: Namhyung Kim <namhyung@kernel.org>

perf symbol: Do not set binary type from symtab type

This commit separates tracking of the DSO binary type from the symtab type.
It removes the explicit setting of the binary type during symbol loading,
deferring it until the DSO is actually opened.

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 4a8c298039447..116870595f75c 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1943,19 +1943,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 removing this explicit assignment cause fully stripped executables to be
falsely rejected during data access operations like perf annotate?

By leaving the DSO binary type as NOT_FOUND here, disassembly paths are forced
to fall back to dso__find_binary_type_locked() to identify the file type.
That logic attempts to verify the file using filename__has_section() for the
.text section:

tools/perf/util/dso.c:dso__find_binary_type_locked() {
    ...
    found = filename__has_section(path, ".text");
    ...
}

For stripped executables (which have had their section headers removed but
still have valid PT_LOAD segments), this check returns false. Previously,
because dso__load() set the binary type directly when locating the system
path DSO, it bypassed the strict .text check.

Consequently, tools like objdump that can parse PT_LOAD segments are never
invoked, and operations fail with COULDNT_DETERMINE_FILE_TYPE.

Additionally, could this cause debuginfo files to be incorrectly selected for
data reads, since their SHT_NOBITS .text sections might falsely satisfy the
check?

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

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