Re: [PATCH 1/3] perf build: Fix cross-arch build failures by isolating auxtrace objects

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Linker errors on non-x86 architectures due to missing `tpebs` variables.
- [High] Loss of cross-architecture hardware trace decoding.
- [High] Linker errors on x86_64 when building with `NO_AUXTRACE=1`.
- [High] CoreSight (CS_ETM) trace decoding broken on 32-bit `arm`.
- [Low] Use of undefined macros for preprocessor guards.
- [Low] Signature mismatch and undefined behavior for `insn_decode` weak stub.
--

commit beae63268ca91077cb9cb1a3341f8277e45cb290
Author: Li Guan <guanli.oerv@isrc.iscas.ac.cn>

perf build: Fix cross-arch build failures by isolating auxtrace objects

This commit isolates architecture-specific hardware tracing drivers from the
generic perf build process. It conditionally links objects based on exported
feature flags and introduces weak stub functions for gracefully handling
omitted drivers.

> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 333ddd0e4bd81..c4c0d79242d5d 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config

[ ... ]

> @@ -107,6 +107,27 @@ ifeq ($(SRCARCH),loongarch)
>    endif
>  endif
>  
> +ifndef NO_AUXTRACE
> +  ifeq ($(SRCARCH),x86)
> +    $(call detected,CONFIG_PERF_INTEL_PT)
> +    $(call detected,CONFIG_PERF_AMD_PT)
> +  endif
> +
> +  ifeq ($(SRCARCH),arm64)
> +    $(call detected,CONFIG_PERF_ARM_SPE)
> +    $(call detected,CONFIG_PERF_HISI_PTT)
> +    $(call detected,CONFIG_PERF_CS_ETM)
> +  endif

Does this omit CoreSight support on 32-bit arm builds? CoreSight trace
decoding is natively supported on 32-bit arm via tools/perf/arch/arm/, but
this explicitly restricts the feature flag solely to arm64.

[ ... ]

> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 70cc91d00804d..79ade6066df9d 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build

[ ... ]

> @@ -132,21 +132,21 @@ perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
>  perf-util-y += kvm-stat-arch/
>  perf-util-y += lock-contention.o
>  perf-util-y += auxtrace.o
> -perf-util-y += intel-pt-decoder/
> -perf-util-y += intel-pt.o
> -perf-util-y += intel-bts.o
> -perf-util-y += arm-spe.o
> -perf-util-y += arm-spe-decoder/
> -perf-util-y += hisi-ptt.o
> -perf-util-y += hisi-ptt-decoder/
> -perf-util-y += s390-cpumsf.o
> +perf-util-$(CONFIG_PERF_INTEL_PT) += intel-pt-decoder/
> +perf-util-$(CONFIG_PERF_INTEL_PT) += intel-pt.o
> +perf-util-$(CONFIG_PERF_INTEL_PT) += intel-bts.o
> +perf-util-$(CONFIG_PERF_ARM_SPE) += arm-spe.o
> +perf-util-$(CONFIG_PERF_ARM_SPE) += arm-spe-decoder/
> +perf-util-$(CONFIG_PERF_HISI_PTT) += hisi-ptt.o
> +perf-util-$(CONFIG_PERF_HISI_PTT) += hisi-ptt-decoder/
> +perf-util-$(CONFIG_PERF_S390_CPUMSF) += s390-cpumsf.o

Will this break the ability to decode hardware traces across different
architectures?

A core feature of perf is analyzing traces like ARM SPE on non-native
hosts like x86. Omitting these objects entirely on non-native
architectures would cause cross-architecture decoding to silently fall back
to the weak stubs and fail.

[ ... ]

> @@ -168,7 +168,7 @@ perf-util-y += clockid.o
>  perf-util-y += list_sort.o
>  perf-util-y += mutex.o
>  perf-util-y += sharded_mutex.o
> -perf-util-y += intel-tpebs.o
> +perf-util-$(CONFIG_PERF_INTEL_PT) += intel-tpebs.o

Could this cause linker errors on non-x86 architectures? 

The builtin-stat.c file unconditionally references the tpebs_recording and
tpebs_mode global variables defined in intel-tpebs.o. If this file is
conditionally compiled only for x86, non-x86 builds will likely fail with
undefined references during the final linking stage.

> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index a224687ffbc1b..f6e426c3c56e1 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c

[ ... ]

> @@ -2960,3 +2961,72 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
>  
>  	return session->auxtrace->evsel_is_auxtrace(session, evsel);
>  }
> +
> +/*
> + * Stub functions for architecture-specific features to support cross-platform building.
> + * These weak symbols are overridden by strong symbols when the corresponding
> + * architecture-specific code is compiled in.
> + */
> +#ifndef HAVE_ARCH_X86_64_SUPPORT
> +int __weak intel_pt_process_auxtrace_info(union perf_event *event __maybe_unused,

Will this lead to unresolved symbols on 64-bit x86 if NO_AUXTRACE=1 is set?

The HAVE_ARCH_X86_64_SUPPORT macro is defined unconditionally for x86_64
builds. If NO_AUXTRACE=1 skips compiling the strong definitions, this
preprocessor guard will also skip the weak stubs, leaving unconditional
callers with missing symbols.

[ ... ]

> +int __weak insn_decode(void *insn __maybe_unused, const void *kaddr __maybe_unused,
> +		       int len __maybe_unused, int mode __maybe_unused)
> +{
> +	return -EOPNOTSUPP;
> +}

Is the signature for insn_decode correct here?

The callers in tools/perf/util/sample.c expect the first parameter to be
struct insn *insn rather than void *insn. Calling a function via a
declaration with mismatched pointer types is undefined behavior in C. A
forward declaration for struct insn might be needed instead.

> +#endif
> +
> +#ifndef HAVE_ARCH_ARM64_SUPPORT
> +int __weak arm_spe_process_auxtrace_info(union perf_event *event __maybe_unused,
> +					 struct perf_session *session __maybe_unused)

Are the HAVE_ARCH_ARM64_SUPPORT and HAVE_ARCH_S390_SUPPORT macros defined
anywhere in the tree?

These preprocessor guards do not appear to exist, which would mean these
weak stubs are compiled unconditionally on all architectures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513154851.1085334-1-guanli.oerv@isrc.iscas.ac.cn?part=1