Re: [PATCH v9 06/18] perf symbol: Avoid use of machine__is

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch completely fails to read the ELF machine architecture from the DSO for kernel objects, introducing a cross-architecture analysis regression.
--

commit 01fba2f5fba1a6782bc236fbef6a297cb4c04daf
Author: Ian Rogers <irogers@google.com>

perf symbol: Avoid use of machine__is

This commit switches to using the ELF machine from the dso or running
machine rather than the machine perf_env arch to avoid falling back on
EM_HOST and imprecise string comparisons.

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index fcaeeddbbb6b..a4b1f837a5a5 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c

[ ... ]

> @@ -1428,7 +1444,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
>  		free(new_node);
>  	}
>  
> -	if (machine__is(machine, "x86_64")) {
> +	if (machine_or_dso_e_machine(machine, dso) == EM_X86_64) {

Does this actually avoid falling back to EM_HOST for kernel objects?

Looking at dso__e_machine_endian(), kernel DSOs (like DSO_BINARY_TYPE__VMLINUX,
KALLSYMS, and KCORE) skip reading the ELF header entirely and immediately
return perf_env__e_machine(machine->env).

Additionally, at this point in dso__load_kcore(), the DSO binary type has not
been set yet. It is set later in the function via dso__set_binary_type().
Because of this, it returns EM_NONE, causing machine_or_dso_e_machine() to
also fall back to perf_env__e_machine().

Since perf_env__e_machine_nocache() defaults to EM_HOST if the architecture is
missing, analyzing a non-x86 perf.data file missing an architecture string on
an x86_64 host will evaluate to EM_X86_64.

Previously, machine__is() used perf_env__raw_arch(), which safely returned
"unknown" if the architecture was missing, preventing incorrect matches.

Could this change incorrectly trigger x86_64-specific trampoline mappings for
non-x86 kernel objects?

>  		u64 addr;
>  
>  		/*
>  		 * If one of the corresponding symbols is there, assume the

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