Re: [PATCH v8 08/17] 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:
- [High] machine_or_dso_e_machine() incorrectly prioritizes the globally cached environment architecture over the DSO, and erroneously returns the host architecture (EM_HOST) for kernel traces.
--

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

perf symbol: Avoid use of machine__is

This commit switches from using machine__is to checking the ELF machine
type from either the DSO or the running machine's environment. This aims
to use a more accurate architecture representation and avoid imprecise
string comparisons.

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index fcaeeddbbb6b..8aaaab0ad4b7 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
[ ... ]
> @@ -851,6 +851,24 @@ static int maps__split_kallsyms_for_kcore(struct maps *kmaps, struct dso *dso)
>  	return count;
>  }
>  
> +static uint16_t machine_or_dso_e_machine(struct machine *machine, struct dso *dso)
> +{
> +	uint16_t e_machine = EM_NONE;
> +
> +	/* Check for a cached value first. */
> +	if (machine && machine->env && machine->env->e_machine != EM_NONE)
> +		return machine->env->e_machine;
> +
> +	/* DSO should be most accurate */

Does checking the globally cached environment architecture before the DSO
contradict the comment that the DSO should be most accurate?

If the environment architecture is already cached, won't this bypass the DSO
entirely and potentially break analysis of mixed-architecture user binaries?

> +	if (dso)
> +		e_machine = dso__e_machine(dso, machine, /*e_flags=*/NULL);
> +
> +	if (e_machine != EM_NONE)
> +		return e_machine;

When dso__e_machine() is called for kernel DSOs, does it return the host
architecture (EM_HOST) of the machine running the perf tool?

If so, would returning this host architecture skip the fallback to
perf_env__e_machine(), causing cross-architecture analysis (like reading
an x86_64 trace on an ARM64 host) to fail checks like e_machine == EM_X86_64?

> +
> +	return perf_env__e_machine(machine ? machine->env : NULL, /*e_flags=*/NULL);
> +}

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