Re: [PATCH v2 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 3 potential issue(s) to consider:
- [High] perf: data-only DSOs fail to open due to strict .text section requirement
- [High] perf: compat offline DWARF unwinding broken due to missing binary_type
- [Medium] perf: redundant double-decompression of compressed kernel modules
--

commit 0528580a893b2b2066f28586347977aa4bf1e268
Author: Namhyung Kim <namhyung@kernel.org>

perf symbol: Do not set binary type from symtab type

The DSO binary type is to access the binary (e.g. instruction) data
while symtab type is for the symbol table.  They may or may not be in
the same file.  We should track them separately and correctly.

> 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: High]
Does removing this fallback prevent opening data-only DSOs?

Without this early assignment, DSOs remain as DSO_BINARY_TYPE__NOT_FOUND.
Any profiling workload attempting to access symbols in a DSO without
executable code might hit this path:

try_to_open_dso()
  __dso__find_binary_type()
    filename__has_section(path, ".text")

Because the ".text" check fails for data-only DSOs, the binary type remains
DSO_BINARY_TYPE__NOT_FOUND. This bypasses the correct opening logic in
open_dso() and permanently marks the DSO with DSO_DATA_STATUS_ERROR. Can we
still open data-only DSOs (like eBPF maps or data-only kernel modules)
after this change?

[Severity: Medium]
Without the early binary type assignment, does this introduce redundant
decompression for compressed kernel modules?

When try_to_open_dso() is called for a compressed kernel module, it first
calls __dso__find_binary_type(), which uses dso__get_filename() to fully
decompress the module to a temporary file just to check for a ".text"
section, and then unlinks it.

Immediately following this, try_to_open_dso() calls open_dso() which again
calls dso__get_filename(), triggering a second complete decompression of the
exact same module. Can this cause severe performance degradation when
processing profiles containing compressed kernel modules?

[Severity: High]
Could this removal also break offline DWARF stack unwinding for compat tasks?

Because user-space maps now have a binary type of DSO_BINARY_TYPE__NOT_FOUND
during early processing, when thread__e_machine_endian() calls
dso__e_machine_endian() to determine the architecture, it hits the fallback:

tools/perf/util/dso.c:dso__e_machine_endian() {
    ...
    switch (dso__binary_type(dso)) {
    ...
    case DSO_BINARY_TYPE__NOT_FOUND:
    default:
        if (e_flags)
            *e_flags = 0;
        return EM_NONE;
    }
}

It immediately fails and returns EM_NONE without attempting to open the file.
Consequently, thread__e_machine_endian() fails to extract the architecture
from any map and falls back to perf_env__e_machine(), which incorrectly
applies the host architecture to the compat thread.

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

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