Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH] perf test: Make leafloop workload immune to compiler options
@ 2026-05-08 10:33 James Clark
  2026-05-08 20:20 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: James Clark @ 2026-05-08 10:33 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter
  Cc: linux-perf-users, linux-kernel, James Clark

Since the leafloop test program was moved into the main Perf binary as a
workload, it inherited the same compiler options as Perf. In this case
the -fstack-protector option broke the assumption that simple leaf
frames don't have a stack frame on Arm. This causes
test_arm_callgraph_fp.sh to pass even if the stack isn't augmented with
the link register, making the test useless.

Fix it by rewriting the leaf function in assembly seeing as it's so
simple. Adding -fno-stack-protector would also work, but wouldn't be
robust against other future compiler option additions.

The local variables and 'a' variable were never needed so remove them to
simplify.

Assisted-by: GitHub-Copilot:GPT-5.5
Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/tests/workloads/leafloop.c | 36 +++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
index f7561767e32c..58367fd1daec 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;
 
 static void sighandler(int sig __maybe_unused)
@@ -17,15 +16,36 @@ static void sighandler(int sig __maybe_unused)
 	done = 1;
 }
 
-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"
+	".global leaf\n"
+	".type leaf, %function\n"
+	"leaf:\n"
+	"	adrp	x1, done\n"
+	"	ldr	w2, [x1, #:lo12:done]\n"
+	"	cbz	w2, leaf\n"
+	"	ret\n"
+);
+
+#else
+
+noinline void leaf(void)
 {
 	while (!done)
-		a += b;
+		;
 }
 
-noinline void parent(volatile int b)
+#endif
+
+noinline void parent(void)
 {
-	leaf(b);
+	leaf();
 }
 
 static int leafloop(int argc, const char **argv)
@@ -39,7 +59,7 @@ static int leafloop(int argc, const char **argv)
 	signal(SIGALRM, sighandler);
 	alarm(sec);
 
-	parent(sec);
+	parent();
 	return 0;
 }
 

---
base-commit: 8c8f2093614373ea8179b562320212a25cf937c0
change-id: 20260508-james-perf-leafloop-stack-c221600eddf2

Best regards,
-- 
James Clark <james.clark@linaro.org>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] perf test: Make leafloop workload immune to compiler options
  2026-05-08 10:33 [PATCH] perf test: Make leafloop workload immune to compiler options James Clark
@ 2026-05-08 20:20 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-05-08 20:20 UTC (permalink / raw)
  To: James Clark; +Cc: linux-perf-users

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-08 20:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 10:33 [PATCH] perf test: Make leafloop workload immune to compiler options James Clark
2026-05-08 20:20 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox