Re: [PATCH v9 10/18] perf arch common: Use perf_env e_machine rather than arch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Native objdump fallback breaks for cross-bitness profiling (e.g., analyzing a 32-bit profile on a 64-bit host).
- [Medium] Unintended address space handling change for 32-bit SPARC.
--

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

perf arch common: Use perf_env e_machine rather than arch

This commit updates the perf architecture matching logic to use e_machine
values instead of architecture strings.

> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 21836f70f231..1d8aff9b32d6 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
[ ... ]
> @@ -145,7 +150,8 @@ static int perf_env__lookup_binutils_path(struct perf_env *env,
[ ... ]
>  	/*
>  	 * We don't need to try to find objdump path for native system.
>  	 * Just use default binutils path (e.g.: "objdump").
>  	 */
> -	if (!strcmp(perf_env__arch(NULL), arch))
> +	if (e_machine == EM_HOST)
>  		goto out;

Does this change break cross-bitness profiling on the same architecture
family?

Previously, perf_env__arch() normalized both 32-bit (EM_386) and 64-bit
(EM_X86_64) ELF machine types into a single family string like x86. This
allowed a 64-bit host to fall back to the native objdump when analyzing a
32-bit profile.

With the new check, EM_386 is not equal to EM_X86_64 for a 32-bit profile
on a 64-bit host, causing the check to fail. The code then attempts a triplet
lookup, such as i686-pc-linux-gnu-objdump. 

If these specific prefixed symlinks are not installed, will this trigger a
fatal ui__error and abort perf annotate, even though the native objdump
inherently supports multilib and 32-bit binaries?

[ ... ]
> @@ -237,5 +255,5 @@ int perf_env__lookup_objdump(struct perf_env *env, char **path)
>   */
>  bool perf_env__single_address_space(struct perf_env *env)
>  {
> -	return strcmp(perf_env__arch(env), "sparc");
> +	return perf_env__e_machine(env, /*e_flags=*/NULL) != EM_SPARCV9;
>  }

Is this an intended logic change for 32-bit SPARC?

Before this patch, perf_env__arch() mapped both 32-bit (EM_SPARC) and
64-bit (EM_SPARCV9) to sparc, so the strcmp returned 0 (false) for both
variants. 

Now, the function evaluates to false for 64-bit SPARC (EM_SPARCV9) but
evaluates to true for 32-bit SPARC (EM_SPARC).

Could this unexpectedly alter address space separation for 32-bit SPARC
profiles?

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