From 269ba26163484ce523e95d8605b4123f1bdc0029 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Thu, 24 Sep 2026 17:39:59 +0000 Subject: [PATCH 1/2] fix(test): stop -O3 tail-calling away the return-address trampolines Test3_UnwindRowSelectedAtReturnAddress fails in the release configuration on x86_64. prof_ra_cfi_trampoline exists so that the caller of the boundary frame is a specific, symbolizable function rather than the gtest body, but at -O3 the compiler turns its call into a tail jump: prof_ra_cfi_trampoline: movl $0x1,-0x4(%rsp) mov -0x4(%rsp),%eax jmp prof_ra_cfi_caller With the jmp the trampoline never establishes a frame, so at runtime the real caller of prof_ra_cfi_caller is TestBody. The walker reports that correctly and the fixture's expectation is what is wrong -- chain[kBoundaryIndex] still resolves to prof_ra_cfi_caller, so the boundary traversal the test was written to cover is unaffected. The existing guard did not prevent this. noinline stops the trampoline being inlined into its caller, and the volatile store completed before the call, so neither blocked the tail jump. Moving the volatile store after the call makes the call non-tail by construction, which is compiler-agnostic -- unlike __attribute__((disable_tail_calls)), which is clang-only while ConfigurationPresets still carries a gcc path. prof_ra_plt_trampoline had the same defect; its test is currently gated off on x86_64 by the alignment check, so it never surfaced. Both are fixed. Release codegen after the change, for both trampolines: push %rbp / mov %rsp,%rbp / ... / call / movl $0x2 / ret Verified on linux-x64 in all four configurations -- release, debug, asan and tsan -- each 15 passed, 3 arch-gated skips, 0 failures. The failure dates to #786, which added the fixture; no later commit is involved. It stayed invisible because PR CI builds its matrix from labels (ci.yml, compute-configurations): debug always, release only behind a test:release label. #786 carried only sphinx:critical, so every cell in its run was debug, where the tail call does not occur. The asan and tsan presets both pass -fno-optimize-sibling-calls, so they cannot exhibit it either. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/test/cpp/returnAddressAttribution_ut.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp b/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp index b7f4e6c0de..9e18ee5c09 100644 --- a/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp +++ b/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp @@ -856,10 +856,14 @@ extern "C" void prof_ra_cfi_collect(void) { // Named trampoline so Test 3's "caller of the boundary frame" is a specific, // symbolizable function rather than the test body itself. extern "C" __attribute__((noinline)) void prof_ra_cfi_trampoline(void) { - // volatile to defeat tail-call/inlining folding this frame away. + // noinline alone is not enough: it stops this function being inlined + // into its caller, but at -O3 the call below still becomes a tail jump, + // which erases this frame at runtime. The volatile store AFTER the call + // is what makes the call non-tail by construction. volatile int guard = 1; (void)guard; prof_ra_cfi_caller(); + guard = 2; } #endif // __x86_64__ || __aarch64__ @@ -1027,9 +1031,14 @@ extern "C" void prof_ra_plt_collect(void) { } extern "C" __attribute__((noinline)) void prof_ra_plt_trampoline(void) { + // noinline alone is not enough: it stops this function being inlined + // into its caller, but at -O3 the call below still becomes a tail jump, + // which erases this frame at runtime. The volatile store AFTER the call + // is what makes the call non-tail by construction. volatile int guard = 1; (void)guard; prof_ra_plt_caller(); + guard = 2; } #endif // __x86_64__ || __aarch64__ From 8b500d4ffc6f1c90a0d51fdf3799a4a99bc74584 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Thu, 24 Sep 2026 22:00:22 +0200 Subject: [PATCH 2/2] test: drop the pre-call half of the tail-call guard The volatile store before the call never contributed anything. Both compilers emit it and then tear the frame down anyway -- that is the original defect, not a weaker version of the fix. Only the store after the call leaves work to do on return, which is what stops the call being turned into a tail jump. Keeping both halves left the ineffective one sitting directly under a comment claiming it worked, which is how the first version came to be believed. Codegen checked at -O3 with clang and gcc on x86_64 and with clang on aarch64: a store only before the call tail-jumps, a store only after does not, and the two together are no better than after alone. Co-Authored-By: Claude Opus 5 (1M context) --- .../test/cpp/returnAddressAttribution_ut.cpp | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp b/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp index 9e18ee5c09..564b81faac 100644 --- a/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp +++ b/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp @@ -856,14 +856,16 @@ extern "C" void prof_ra_cfi_collect(void) { // Named trampoline so Test 3's "caller of the boundary frame" is a specific, // symbolizable function rather than the test body itself. extern "C" __attribute__((noinline)) void prof_ra_cfi_trampoline(void) { - // noinline alone is not enough: it stops this function being inlined - // into its caller, but at -O3 the call below still becomes a tail jump, - // which erases this frame at runtime. The volatile store AFTER the call - // is what makes the call non-tail by construction. - volatile int guard = 1; - (void)guard; prof_ra_cfi_caller(); - guard = 2; + // noinline only stops this function being inlined into its caller; at -O3 + // the call above would still become a tail jump, erasing this frame at + // runtime. A volatile store after the call leaves the compiler something + // to do on return, so the call cannot be a tail call and the frame + // survives for the walker to attribute against. A volatile store *before* + // the call does not work -- the compiler emits it and then tears the frame + // down anyway. + volatile int sink = 0; + (void)sink; } #endif // __x86_64__ || __aarch64__ @@ -1031,14 +1033,10 @@ extern "C" void prof_ra_plt_collect(void) { } extern "C" __attribute__((noinline)) void prof_ra_plt_trampoline(void) { - // noinline alone is not enough: it stops this function being inlined - // into its caller, but at -O3 the call below still becomes a tail jump, - // which erases this frame at runtime. The volatile store AFTER the call - // is what makes the call non-tail by construction. - volatile int guard = 1; - (void)guard; prof_ra_plt_caller(); - guard = 2; + // Same tail-call guard as prof_ra_cfi_trampoline above. + volatile int sink = 0; + (void)sink; } #endif // __x86_64__ || __aarch64__