Re: [PATCH] perf test: Make leafloop workload immune to compiler options

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] LTO build failure due to hardcoded static variable name in inline assembly.
- [Low] Missing ELF `.size` directive for inline assembly function.
- [Low] Fragile section management in top-level assembly.
--

commit 57b4e20838dd06862703b6049b087e60606a2dd4
Author: James Clark <james.clark@linaro.org>

perf test: Make leafloop workload immune to compiler options

This commit replaces the C implementation of the leaf function with an
inline assembly version for AArch64. This ensures compiler options like
-fstack-protector don't inadvertently add a stack frame, which would make
the test_arm_callgraph_fp test pass incorrectly.

> diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> index f7561767e32cd..58367fd1daece 100644
> --- a/tools/perf/tests/workloads/leafloop.c
> +++ b/tools/perf/tests/workloads/leafloop.c
> @@ -6,10 +6,9 @@
>  #include "../tests.h"
>  
>  /* We want to check these symbols in perf script */
> -noinline void leaf(volatile int b);
> -noinline void parent(volatile int b);
> +noinline void leaf(void);
> +noinline void parent(void);
>  
> -static volatile int a;
>  static volatile sig_atomic_t done;

[ ... ]

> -noinline void leaf(volatile int b)
> +#if defined(__aarch64__)
> +/*
> + * Write leaf() in assembly so it stays as a minimal leaf function with no
> + * stack frame and won't get silently broken in the future by any Perf wide
> + * compilation options like -fstack-protector-all.
> + */
> +asm(
> +	".text\n"

Could using .text here instead of .pushsection .text and .popsection
interfere with the compiler's section tracking?

If the compiler assumes the previous section remains active after the asm
block, it might place subsequent data or code in the wrong section under
certain compilation flags.

> +	".global leaf\n"
> +	".type leaf, %function\n"
> +	"leaf:\n"
> +	"	adrp	x1, done\n"

Since done is declared as a static variable at the top of the file, if perf
is compiled with link-time optimization (LTO=1), the compiler might
partition and rename the C variable to avoid collisions across translation
units (e.g., done.lto_priv.0).

Will the hardcoded string "done" in this inline assembly cause an undefined
reference linker error under LTO builds?

> +	"	ldr	w2, [x1, #:lo12:done]\n"
> +	"	cbz	w2, leaf\n"
> +	"	ret\n"

This isn't a bug, but there doesn't appear to be a size directive
(.size leaf, . - leaf) at the end of the function.

While perf might handle 0-sized symbols internally, could omitting the size
cause issues with external ELF tools, debuggers, or profilers trying to
resolve the symbol bounds?

> +);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-james-perf-leafloop-stack-v1-1-637c260b2da8@linaro.org?part=1