From a58121d5e7d1e4ee78cae3b46b6145080bd7a275 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Tue, 20 Oct 2026 19:14:25 +0200 Subject: [PATCH 1/7] test: add reproduction test --- .../FrozenVirtualThreadStackTest.java | 249 +++++++++++++++++- 1 file changed, 236 insertions(+), 13 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/FrozenVirtualThreadStackTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/FrozenVirtualThreadStackTest.java index e4337e2b46..c42b6b25c8 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/FrozenVirtualThreadStackTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/FrozenVirtualThreadStackTest.java @@ -9,28 +9,52 @@ import com.datadoghq.profiler.JfrFrame; import com.datadoghq.profiler.JfrStackTrace; import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.RemoteSymHelper; import com.datadoghq.profiler.junit.CStack; +import java.lang.management.ManagementFactory; import java.util.concurrent.FutureTask; import java.util.concurrent.locks.LockSupport; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.params.provider.ValueSource; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; -/** Checks stack completeness against the JVM's logical stack after virtual-thread remounts. */ +/** Checks recovered callers against the JVM's logical stack after virtual-thread remounts. */ public class FrozenVirtualThreadStackTest extends CStackAwareAbstractProfilerTest { private static final int RECURSION_DEPTH = 128; private volatile long sink; private StackTraceElement[] logicalStack; + private boolean growChunks; + private boolean nativeLeaf; + private boolean cpuProfiling; + private volatile boolean collecting; + private volatile byte[] garbage; + private int maxStackDepth = 256; /** Selects the VM walker with or without carrier frames. */ public FrozenVirtualThreadStackTest(@CStack String cstack) { super(cstack); } + /** Selects a deliberately small capture limit only for the depth-limit regression. */ + @Override + @BeforeEach + public void setupProfiler(TestInfo testInfo) throws Exception { + maxStackDepth = testInfo.getTestMethod() + .map(method -> method.getName().equals("respectsConfiguredDepth") ? 64 : 256) + .orElse(256); + cpuProfiling = testInfo.getTestMethod() + .map(method -> method.getName().equals("restoresFrozenCallersForCpu")) + .orElse(false); + super.setupProfiler(testInfo); + } + @Override protected boolean isPlatformSupported() { // CStackInjector falls back to the first mode when every mode is excluded. @@ -41,26 +65,198 @@ protected boolean isPlatformSupported() { @Override protected String getProfilerCommand() { - return "wall=1ms,walltpt=64,filter=,jstackdepth=256"; + return (cpuProfiling ? "cpu=1ms,event=itimer" : "wall=1ms,walltpt=64") + + ",filter=,jstackdepth=" + maxStackDepth; } - /** Missing logical callers must be marked truncated in both VM walking modes. */ + /** Both VM walking modes must recover every application caller after a partial thaw. */ @TestTemplate @ValueSource(strings = {"vm", "vmx"}) - public void marksSkippedFrozenCallersTruncated(@CStack String cstack) throws Exception { + public void restoresFrozenCallers(@CStack String cstack) throws Exception { + assumeTrue(!usesConcurrentRelocation()); runWorkload(true, true); - assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_cont_barrier_hit", 0L) > 0, - "The workload must encounter a continuation return barrier"); + assertRecoveredCallers(cstack, false); + } + + /** CPU samples must reconstruct the same callers as wall-clock samples. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void restoresFrozenCallersForCpu(@CStack String cstack) throws Exception { + assumeTrue(!usesConcurrentRelocation()); + runWorkload(true, true); + assertRecoveredCallers(cstack, false); + } + + /** Repeated collection and remounting must not invalidate chunk or carrier oop discovery. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void restoresWhileCollecting(@CStack String cstack) throws Exception { + assumeTrue(!usesConcurrentRelocation()); + long before = collectionCount(); + collecting = true; + FutureTask task = new FutureTask<>(() -> { + while (collecting) { + for (int i = 0; i < 32; i++) garbage = new byte[8192]; + System.gc(); + LockSupport.parkNanos(20_000_000L); + } + return null; + }); + Thread collector = new Thread(task, "frozen-stack-gc-control"); + collector.start(); + try { + runWorkload(true, true); + } finally { + collecting = false; + collector.join(10_000); + } + assertFalse(collector.isAlive(), "The GC workload did not finish"); + task.get(); + assertTrue(collectionCount() > before, "The workload must encounter garbage collection"); + assertRecoveredCallers(cstack, false); + } + /** A JNI call may be sampled while older callers are frozen, but raw heap reads are unsafe. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void preservesTruncationDuringNativeCalls(@CStack String cstack) throws Exception { + nativeLeaf = true; + runWorkload(true, true); int[] counts = reduceEvents("datadog.MethodSample", () -> new int[2], (count, sample) -> { JfrStackTrace trace = sample.getStackTrace(); - if (containsMethod(trace, "recursiveWorkload") && !containsMethod(trace, "outerWorkload")) { + if ("NATIVE".equals(sample.getEnumName("mode")) && containsMethod(trace, "sampledLeaf") + && !containsMethod(trace, "outerWorkload")) { count[0]++; if (!trace.isTruncated()) count[1]++; } }); - assertTrue(counts[0] > 0, "The workload must leave application callers frozen"); - assertEquals(0, counts[1], "Samples omitted a JVM-confirmed caller but reported a complete stack"); + assertTrue(counts[0] > 0, "No native-mode sample exercised the incomplete-stack fallback"); + assertEquals(0, counts[1], "Native-mode samples must acknowledge missing callers"); + assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_frozen_failed", 0L) > 0, + "Native execution must not attempt raw heap recovery"); + } + + /** A second park below still-frozen callers must follow the chunk's parent link. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void restoresAcrossParentChunks(@CStack String cstack) throws Exception { + assumeTrue(!usesConcurrentRelocation()); + growChunks = true; + runWorkload(true, true); + assertRecoveredCallers(cstack, true); + assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_frozen_parent", 0L) > 0, + "The workload must recover callers across multiple chunks"); + } + + /** Heap walking must obey the same capture limit and truncation contract as native walking. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void respectsConfiguredDepth(@CStack String cstack) throws Exception { + runWorkload(true, true); + int[] counts = reduceEvents("datadog.MethodSample", () -> new int[3], (count, sample) -> { + JfrStackTrace trace = sample.getStackTrace(); + if (containsMethod(trace, "sampledLeaf")) { + count[0]++; + if (trace.frames().size() > maxStackDepth) count[1]++; + if (!trace.isTruncated()) count[2]++; + } + }); + assertTrue(counts[0] > 0, "No sample captured the leaf workload"); + assertEquals(0, counts[1], "Heap walking exceeded the configured depth"); + assertEquals(0, counts[2], "Depth-limited application stacks must be truncated"); + } + + /** Concurrently relocating collectors retain the safe incomplete-stack fallback. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void preservesTruncationWhenRecoveryUnavailable(@CStack String cstack) throws Exception { + assumeTrue(usesConcurrentRelocation()); + runWorkload(true, true); + assertEquals(0L, profiler.getDebugCounters().getOrDefault("walkvm_frozen_recovered", 0L)); + assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_frozen_failed", 0L) > 0); + int[] counts = reduceEvents("datadog.MethodSample", () -> new int[2], (count, sample) -> { + JfrStackTrace trace = sample.getStackTrace(); + if (containsMethod(trace, "sampledLeaf") && !containsMethod(trace, "outerWorkload")) { + count[0]++; + if (!trace.isTruncated()) count[1]++; + } + }); + assertTrue(counts[0] > 0, "No sample exercised the unavailable recovery path"); + assertEquals(0, counts[1], "Unavailable heap walking must preserve truncation"); + } + + /** Compares the captured application chain with the independently confirmed JVM chain. */ + private void assertRecoveredCallers(String cstack, boolean nested) { + assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_cont_barrier_hit", 0L) > 0, + "The workload must encounter a continuation return barrier"); + + String event = cpuProfiling ? "datadog.ExecutionSample" : "datadog.MethodSample"; + int[] counts = reduceEvents(event, () -> new int[8], (count, sample) -> { + JfrStackTrace trace = sample.getStackTrace(); + if (containsMethod(trace, "sampledLeaf")) { + int recursiveCallers = 0; + int nestedCallers = 0; + boolean leafSeen = false; + boolean outerSeen = false; + for (JfrFrame frame : trace.frames()) { + if (!getClass().getName().equals(frame.className())) continue; + if ("sampledLeaf".equals(frame.methodName())) { + if (leafSeen) count[7]++; + leafSeen = true; + } + if ("nestedWorkload".equals(frame.methodName())) { + if (!leafSeen || recursiveCallers != 0 || outerSeen) count[4]++; + nestedCallers++; + } + if ("recursiveWorkload".equals(frame.methodName())) { + if (!leafSeen || outerSeen) count[4]++; + recursiveCallers++; + } + if ("outerWorkload".equals(frame.methodName())) outerSeen = true; + } + int expectedNested = nested ? 65 : 0; + if (recursiveCallers > RECURSION_DEPTH + 1 || nestedCallers > expectedNested) count[7]++; + // doYield's generated native wrapper can still have mode=JAVA. + // Require full recovery at application execution; every other + // sample must preserve order, avoid duplicates and flag omissions. + JfrFrame top = trace.frames().get(0); + boolean applicationJava = "JAVA".equals(sample.getEnumName("mode")) + && getClass().getName().equals(top.className()) + && "sampledLeaf".equals(top.methodName()); + if (!applicationJava) { + if ((!outerSeen || recursiveCallers != RECURSION_DEPTH + 1 + || nestedCallers != expectedNested) && !trace.isTruncated()) count[6]++; + return; + } + count[0]++; + if (!outerSeen) count[1]++; + if (recursiveCallers != RECURSION_DEPTH + 1) count[2]++; + if (nestedCallers != expectedNested) count[3]++; + if ("vm".equals(cstack) && trace.isTruncated()) count[5]++; + } + }); + assertTrue(counts[0] > 0, "No sample captured the leaf workload"); + assertEquals(0, counts[1], "Samples omitted the JVM-confirmed outer caller; recovered=" + + profiler.getDebugCounters().getOrDefault("walkvm_frozen_recovered", 0L) + + ", failed=" + profiler.getDebugCounters().getOrDefault("walkvm_frozen_failed", 0L)); + assertEquals(0, counts[2], "Samples omitted or duplicated recursive application callers"); + assertEquals(0, counts[3], "Samples omitted or duplicated nested application callers"); + assertEquals(0, counts[4], "Application callers were recovered in the wrong order"); + assertEquals(0, counts[5], "Recovered VM stacks must be complete"); + assertEquals(0, counts[6], "Unsafe-state samples must acknowledge missing callers"); + assertEquals(0, counts[7], "Freezing/thawing samples must not duplicate heap and carrier frames"); + assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_frozen_recovered", 0L) > 0, + "No continuation boundary recovered its frozen callers"); + } + + private boolean usesConcurrentRelocation() { + return ManagementFactory.getGarbageCollectorMXBeans().stream() + .anyMatch(gc -> gc.getName().contains("ZGC") || gc.getName().contains("Shenandoah")); + } + + private long collectionCount() { + return ManagementFactory.getGarbageCollectorMXBeans().stream() + .mapToLong(gc -> Math.max(0, gc.getCollectionCount())).sum(); } /** The same parked workload on a platform thread keeps its application callers on the stack. */ @@ -101,13 +297,16 @@ private void runWorkload(boolean virtual, boolean park) throws Exception { stopProfiler(); int recursion = 0; + int nested = 0; boolean outer = false; for (StackTraceElement frame : logicalStack) { if (!getClass().getName().equals(frame.getClassName())) continue; if (frame.getMethodName().equals("recursiveWorkload")) recursion++; + if (frame.getMethodName().equals("nestedWorkload")) nested++; if (frame.getMethodName().equals("outerWorkload")) outer = true; } assertEquals(RECURSION_DEPTH + 1, recursion, "The JVM must preserve all recursive callers"); + assertEquals(growChunks ? 65 : 0, nested, "The JVM must preserve all nested callers"); assertTrue(outer, "The outer caller must be present in the JVM logical stack"); } @@ -142,12 +341,36 @@ private void outerWorkload(boolean park) { private void recursiveWorkload(int depth, boolean park) { if (depth > 0) { recursiveWorkload(depth - 1, park); - } else { + } else if (growChunks) { if (park) LockSupport.parkNanos(500_000L); - if (logicalStack == null) logicalStack = Thread.currentThread().getStackTrace(); - long deadline = System.nanoTime() + 2_000_000L; - while (System.nanoTime() < deadline) sink ^= System.nanoTime(); + nestedWorkload(64, park); + } else { + sampledLeaf(park); } sink += depth; } + + /** Grows another stack segment while the earlier recursive callers remain frozen. */ + private void nestedWorkload(int depth, boolean park) { + if (depth > 0) nestedWorkload(depth - 1, park); + else sampledLeaf(park); + sink += depth; + } + + /** Identifies samples taken while all recursive callers are still live. */ + private void sampledLeaf(boolean park) { + if (park) LockSupport.parkNanos(500_000L); + if (logicalStack == null) logicalStack = Thread.currentThread().getStackTrace(); + long deadline = System.nanoTime() + 2_000_000L; + long value = sink; + while (System.nanoTime() < deadline) { + if (nativeLeaf) { + value ^= RemoteSymHelper.burnCpu(100_000, 2); + } else { + // Keep the interrupted PC in Java rather than nanoTime's native implementation. + for (int i = 0; i < 16_384; i++) value = (value ^ i) * 0x5deece66dL + 11; + } + sink = value; + } + } } From 4681b2db030540a16038f03e4f8b45e54d8bdedd Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Tue, 20 Oct 2026 19:14:56 +0200 Subject: [PATCH 2/7] fix: add frozenStackWalker logic --- ddprof-lib/src/main/cpp/counters.h | 5 + .../main/cpp/hotspot/frozenStackWalker.cpp | 238 ++++++++++++++++++ .../src/main/cpp/hotspot/frozenStackWalker.h | 21 ++ .../src/main/cpp/hotspot/hotspotSupport.cpp | 20 +- ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp | 2 + ddprof-lib/src/main/cpp/hotspot/vmStructs.h | 15 ++ 6 files changed, 296 insertions(+), 5 deletions(-) create mode 100644 ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp create mode 100644 ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.h diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index 1f0846b2d0..ab49a4a8ef 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -137,6 +137,11 @@ X(WALKVM_FP_CHAIN_REACHED_CODEHEAP, "walkvm_fp_chain_reached_codeheap") \ X(WALKVM_ANCHOR_NOT_IN_JAVA, "walkvm_anchor_not_in_java") \ X(WALKVM_CONT_BARRIER_HIT, "walkvm_cont_barrier_hit") \ + X(WALKVM_FROZEN_RECOVERED, "walkvm_frozen_recovered") \ + X(WALKVM_FROZEN_FAILED, "walkvm_frozen_failed") \ + X(WALKVM_FROZEN_INTERPRETED, "walkvm_frozen_interpreted") \ + X(WALKVM_FROZEN_COMPILED, "walkvm_frozen_compiled") \ + X(WALKVM_FROZEN_PARENT, "walkvm_frozen_parent") \ X(WALKVM_ENTER_SPECIAL_HIT, "walkvm_enter_special_hit") \ X(WALKVM_CONT_SPECULATIVE_HIT,"walkvm_cont_speculative_hit") \ X(WALKVM_CONT_ENTRY_NULL, "walkvm_cont_entry_null") \ diff --git a/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp new file mode 100644 index 0000000000..5c816cd1ba --- /dev/null +++ b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp @@ -0,0 +1,238 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "hotspot/frozenStackWalker.h" +#include "hotspot/hotspotSupport.h" +#include "hotspot/vmStructs.inline.h" +#include "stackWalker.inline.h" + +namespace { + +struct ChunkLayout { + int carrier_cont; + int cont_parent; + int cont_tail; + int parent; + int size; + int sp; + int bottom; + int argsize; + int stack; + uintptr_t oop_base; + int oop_shift; + bool supported; +}; + +ChunkLayout layout = {}; + +int readOffset(CodeCache* libjvm, const char* name) { + const int* address = (const int*)libjvm->findSymbol(name); + if (address == nullptr || *address <= 0 || *address > 4096) return -1; + return *address; +} + +uintptr_t readOop(uintptr_t object, int offset) { + if (layout.oop_shift >= 0) { + unsigned int narrow = __atomic_load_n((unsigned int*)(object + offset), __ATOMIC_ACQUIRE); + return narrow == 0 ? 0 : layout.oop_base + ((uintptr_t)narrow << layout.oop_shift); + } + return __atomic_load_n((uintptr_t*)(object + offset), __ATOMIC_ACQUIRE); +} + +// Interpreted frames in a chunk store FP, last_sp, sender_sp and locals as +// word offsets, rather than the native-stack pointers used by walkVM. +// See OpenJDK's StackChunkFrameStream::{fp,next_for_interpreter_frame}. +bool relativePointer(uintptr_t base, intptr_t words, uintptr_t low, uintptr_t high, uintptr_t& result) { + if (words < -(intptr_t)((base - low) / sizeof(uintptr_t)) || + words > (intptr_t)((high - base) / sizeof(uintptr_t))) return false; + result = base + words * sizeof(uintptr_t); + return true; +} + +bool appendCompiled(VMNMethod* nm, const void* pc, ASGCT_CallFrame* frames, + volatile int& depth, int max_depth, bool details) { + int level = nm->level(); + FrameTypeId type = details && level >= 1 && level <= 3 ? FRAME_C1_COMPILED : FRAME_JIT_COMPILED; + int scope_offset = nm->findScopeOffset(pc); + if (scope_offset > 0) { + ScopeDesc scope(nm); + do { + scope_offset = scope.decode(scope_offset); + VMMethod* method = scope.method(); + if (method == nullptr) return false; + jmethodID id = method->validatedId(); + if (id == JMETHODID_NOT_WALKABLE) return false; + FrameTypeId frame_type = details && scope_offset > 0 ? FRAME_INLINED : type; + HotspotSupport::fillJavaFrame(frames[depth++], frame_type, scope.bci(), id, method); + } while (scope_offset > 0 && depth < max_depth); + return scope_offset <= 0; + } + VMMethod* method = nm->method(); + if (method == nullptr) return false; + jmethodID id = method->validatedId(); + if (id == JMETHODID_NOT_WALKABLE) return false; + HotspotSupport::fillJavaFrame(frames[depth++], type, 0, id, method); + return true; +} + +} // namespace + +void FrozenStackWalker::init() { + layout.supported = false; + if (_libjvm == nullptr || VM::java_version() < 21 || VM::java_version() > 27) return; +#if !defined(__x86_64__) && !defined(__aarch64__) + return; +#endif + // G1/Serial/Parallel relocate objects at safepoints. A sampled Java thread + // cannot reach a safepoint while its signal handler is reading these oops. + // Concurrently relocating collectors require load barriers not provided here. + bool collector_supported = false; + for (const char* name : {"UseG1GC", "UseSerialGC", "UseParallelGC"}) { + VMFlag* flag = VMFlag::find(name, {VMFlag::Type::Bool}); + if (flag != nullptr && flag->get()) collector_supported = true; + } + TEST_LOG("Frozen stack prerequisites: collector=%d thread_oop=%d handle=%d", collector_supported, + _thread_oop_offset, _oop_handle_offset); + if (!collector_supported || _thread_oop_offset < 0 || _oop_handle_offset < 0) return; + + layout.carrier_cont = readOffset(_libjvm, "_ZN16java_lang_Thread20_continuation_offsetE"); + layout.cont_parent = readOffset(_libjvm, "_ZN28jdk_internal_vm_Continuation14_parent_offsetE"); + layout.cont_tail = readOffset(_libjvm, "_ZN28jdk_internal_vm_Continuation12_tail_offsetE"); + layout.parent = readOffset(_libjvm, "_ZN26jdk_internal_vm_StackChunk14_parent_offsetE"); + layout.size = readOffset(_libjvm, "_ZN26jdk_internal_vm_StackChunk12_size_offsetE"); + layout.sp = readOffset(_libjvm, "_ZN26jdk_internal_vm_StackChunk10_sp_offsetE"); + layout.bottom = readOffset(_libjvm, "_ZN26jdk_internal_vm_StackChunk14_bottom_offsetE"); + layout.argsize = readOffset(_libjvm, "_ZN26jdk_internal_vm_StackChunk15_argsize_offsetE"); + layout.stack = readOffset(_libjvm, "_ZN23InstanceStackChunkKlass16_offset_of_stackE"); + layout.oop_shift = -1; + VMFlag* compressed = VMFlag::find("UseCompressedOops", {VMFlag::Type::Bool}); + if (compressed == nullptr) return; + if (compressed->get()) { + if (_narrow_oop_base_addr == nullptr || _narrow_oop_shift_addr == nullptr) return; + layout.oop_base = *(uintptr_t*)_narrow_oop_base_addr; + layout.oop_shift = *(int*)_narrow_oop_shift_addr; + if (layout.oop_shift < 0 || layout.oop_shift > 16) return; + } + layout.supported = layout.carrier_cont > 0 && layout.cont_parent > 0 && layout.cont_tail > 0 + && layout.parent > 0 && layout.size > 0 && layout.sp > 0 && layout.stack > 0 + && layout.stack % sizeof(uintptr_t) == 0 && (layout.bottom > 0 || layout.argsize > 0); + TEST_LOG("Frozen stack layout: supported=%d carrier=%d tail=%d parent=%d size=%d sp=%d bottom=%d argsize=%d stack=%d oop_shift=%d", + layout.supported, layout.carrier_cont, layout.cont_tail, layout.parent, layout.size, layout.sp, + layout.bottom, layout.argsize, layout.stack, layout.oop_shift); +} + +bool FrozenStackWalker::append(VMThread* thread, const void* sampled_pc, unsigned int continuation_depth, + ASGCT_CallFrame* frames, volatile int& depth, int max_depth, + bool details) { + if (!layout.supported || thread == nullptr || thread->state() != _thread_in_Java + || continuation_depth >= 64 || depth >= max_depth) return false; + + // Freeze/thaw runs without a Java-to-VM transition. During doYield or its + // runtime calls, copied frames can still be on the carrier as well as in a + // chunk. Only an interrupted Java body has finished those stack updates. + VMNMethod* origin = CodeHeap::findNMethod(sampled_pc); + if (origin == nullptr || (!origin->isInterpreter() && + (!origin->isNMethod() || origin->level() <= 0))) return false; + + uintptr_t carrier = thread->carrierThreadOop(); + if (carrier == 0) return false; + // Continuation.run publishes the innermost mounted continuation in the + // carrier Thread.cont, including when the current Java thread is virtual. + uintptr_t continuation = readOop(carrier, layout.carrier_cont); + for (unsigned int i = 0; i < continuation_depth && continuation != 0; i++) { + continuation = readOop(continuation, layout.cont_parent); + } + if (continuation == 0) return false; + uintptr_t chunk = readOop(continuation, layout.cont_tail); + if (chunk == 0) return false; + int interpreted_frames = 0; + int compiled_frames = 0; + int parent_chunks = 0; + + // Empty chunks do not consume output depth. Bound their traversal as well + // so corrupt parent links cannot loop indefinitely in a signal handler. + for (int chunks = 0; chunk != 0 && chunks < 256; chunks++) { + int size = *(int*)(chunk + layout.size); + int sp_offset = *(int*)(chunk + layout.sp); + int bottom_offset = layout.bottom > 0 ? *(int*)(chunk + layout.bottom) + : size - *(int*)(chunk + layout.argsize); + if (size <= 0 || size > (1 << 24) || sp_offset < 0 || sp_offset > size + || bottom_offset < 0 || bottom_offset > size) return false; + + uintptr_t low = chunk + layout.stack; + uintptr_t high = low + (uintptr_t)size * sizeof(uintptr_t); + uintptr_t end = low + (uintptr_t)bottom_offset * sizeof(uintptr_t); + uintptr_t sp = low + (uintptr_t)sp_offset * sizeof(uintptr_t); + uintptr_t unextended_sp = sp; + + while (sp < end) { + if (depth >= max_depth || sp < low + 2 * sizeof(uintptr_t) || sp > high) return false; + const void* pc = stripPointer(((const void**)sp)[-1]); + VMNMethod* nm = CodeHeap::findNMethod(pc); + if (nm == nullptr) return false; + if (nm->isInterpreter()) { + uintptr_t fp; + uintptr_t fp_slot = sp - 2 * sizeof(uintptr_t); + if (!relativePointer(fp_slot, *(intptr_t*)fp_slot, low, high, fp) + || fp < low + 9 * sizeof(uintptr_t) || fp > high - 2 * sizeof(uintptr_t)) return false; + VMMethod* method = ((VMMethod**)fp)[InterpreterFrame::method_offset]; + if (method == nullptr) return false; + jmethodID id = method->validatedId(); + if (id == JMETHODID_NOT_WALKABLE) return false; + // Method::bcp_from accepts either a bytecode pointer or an index. + uintptr_t bcp = ((uintptr_t*)fp)[InterpreterFrame::bcp_offset()]; + uintptr_t bci = bcp <= 65535 ? bcp : bcp - (uintptr_t)method->bytecode(); + if (bci > 65535) return false; + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, id, method); + interpreted_frames++; + + uintptr_t locals; + // In both supported architectures locals immediately precedes bcp. + int locals_offset = InterpreterFrame::bcp_offset() + 1; + if (!relativePointer(fp, ((intptr_t*)fp)[locals_offset], low, high, locals)) return false; + if (locals + sizeof(uintptr_t) >= end) { + sp = end; + } else { + uintptr_t next_sp = fp + 2 * sizeof(uintptr_t); + if (next_sp <= sp || !relativePointer(fp, ((intptr_t*)fp)[InterpreterFrame::sender_sp_offset], + low, high, unextended_sp)) return false; + sp = next_sp; + } + } else if (nm->isNMethod()) { + int frame_size = nm->frameSize(); + if (frame_size <= 0 || frame_size > MAX_FRAME_SIZE_WORDS) return false; + // If the previous frame was interpreted, its sender_sp is the + // compiled caller's unextended SP (StackChunkFrameStream::next). + uintptr_t next_sp = unextended_sp + (uintptr_t)frame_size * sizeof(uintptr_t); + if (next_sp <= sp || next_sp > high) return false; + if (!appendCompiled(nm, pc, frames, depth, max_depth, details)) return false; + compiled_frames++; + sp = next_sp >= end - 2 * sizeof(uintptr_t) ? end : next_sp; + unextended_sp = sp; + } else { + return false; + } + if (sp < end) { + const void* next_pc = stripPointer(((const void**)sp)[-1]); + VMNMethod* next = CodeHeap::findNMethod(next_pc); + if (next != nullptr && next->isInterpreter()) { + uintptr_t fp; + uintptr_t fp_slot = sp - 2 * sizeof(uintptr_t); + if (!relativePointer(fp_slot, *(intptr_t*)fp_slot, low, high, fp) + || fp < low + 2 * sizeof(uintptr_t) || fp > high) return false; + if (!relativePointer(fp, ((intptr_t*)fp)[-2], low, high, unextended_sp)) return false; + } + } + } + uintptr_t parent = readOop(chunk, layout.parent); + if (parent == chunk) return false; + if (parent != 0) parent_chunks++; + chunk = parent; + } + Counters::increment(WALKVM_FROZEN_INTERPRETED, interpreted_frames); + Counters::increment(WALKVM_FROZEN_COMPILED, compiled_frames); + Counters::increment(WALKVM_FROZEN_PARENT, parent_chunks); + return chunk == 0; +} diff --git a/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.h b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.h new file mode 100644 index 0000000000..074eb8835b --- /dev/null +++ b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.h @@ -0,0 +1,21 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef _HOTSPOT_FROZENSTACKWALKER_H +#define _HOTSPOT_FROZENSTACKWALKER_H + +#include "hotspot/vmStructs.h" + +// Reads frozen callers while walkVM's fault recovery is installed. No JNI, +// allocation, heap mutation, or JVM calls are permitted during the walk. +class FrozenStackWalker : private VMStructs { +public: + static void init(); + static bool append(VMThread* thread, const void* sampled_pc, unsigned int continuation_depth, + ASGCT_CallFrame* frames, volatile int& depth, int max_depth, + bool details); +}; + +#endif // _HOTSPOT_FROZENSTACKWALKER_H diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index f2cbce8730..cfbde5364c 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -11,6 +11,7 @@ #include "frames.h" #include "guards.h" #include "hotspot/hotspotSupport.h" +#include "hotspot/frozenStackWalker.h" #include "hotspot/jitCodeCache.h" #include "hotspot/vmStructs.inline.h" #include "jvmSupport.inline.h" @@ -361,6 +362,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // Last ContinuationEntry crossed; advanced via parent() for nested continuations. VMContinuationEntry* cont_entry = nullptr; + unsigned int continuation_depth = 0; // Saved anchor data — preserved across anchor consumption so inline // recovery can redirect even after the anchor pointer has been set to NULL. @@ -388,7 +390,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // Advances through a continuation boundary to the carrier frame. // Without carrier_frames (default, cstack=vm): always stops with a "JVM Continuation" - // synthetic root frame; frozen VT callers are not reconstructed. + // synthetic root frame after recovering frozen callers where supported. // With carrier_frames (cstack=vmx): attempts to walk through; failures emit BCI_ERROR // so the sample is truthfully marked truncated. // Walks cont_entry->parent() on repeated calls to handle nested continuations @@ -404,11 +406,19 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // // Returns true to continue the walk, false to break. auto walkThroughContinuation = [&](bool all_frames_thawed) -> bool { - // A return barrier leaves older Java callers in heap chunks, which this - // walker skips even when carrier walking succeeds. - if (!all_frames_thawed && truncated != nullptr) { - *truncated = true; + if (!all_frames_thawed) { + bool was_truncated = truncated != nullptr && *truncated; + // A recovered fault can bypass append's return and the rest of this + // lambda. Keep the partial trace truncated unless recovery completes. + if (truncated != nullptr) *truncated = true; + if (FrozenStackWalker::append(vm_thread, entry_pc, continuation_depth, frames, depth, actual_max_depth, details)) { + Counters::increment(WALKVM_FROZEN_RECOVERED); + if (truncated != nullptr) *truncated = was_truncated; + } else { + Counters::increment(WALKVM_FROZEN_FAILED); + } } + continuation_depth++; if (depth >= actual_max_depth) return false; if (!features.carrier_frames) { fillFrame(frames[depth++], BCI_NATIVE_FRAME, CONT_ROOT_FRAME); diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index 0a04e46343..e82c92840e 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -9,6 +9,7 @@ #include #include #include "hotspot/vmStructs.inline.h" +#include "hotspot/frozenStackWalker.h" #include "vmEntry.h" #include "jniHelper.h" #include "jvmHeap.h" @@ -125,6 +126,7 @@ void VMStructs::init(CodeCache* libjvm) { // Run when VM is initialized and JNI is available void VMStructs::ready() { resolveOffsets(); + FrozenStackWalker::init(); patchSafeFetch(); } diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index c61ccbc3b3..f09d0bd19f 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -238,6 +238,7 @@ typedef void* address; field(_symbol_body_offset, offset, MATCH_SYMBOLS("_body")) \ type_end() \ type_begin(VMJavaThread, MATCH_SYMBOLS("JavaThread", "Thread")) \ + field_with_version(_thread_oop_offset, offset, 21, MAX_VERSION, MATCH_SYMBOLS("_threadObj")) \ field(_thread_osthread_offset, offset, MATCH_SYMBOLS("_osthread")) \ field(_thread_anchor_offset, offset, MATCH_SYMBOLS("_anchor")) \ field(_thread_state_offset, offset, MATCH_SYMBOLS("_thread_state")) \ @@ -317,6 +318,13 @@ typedef void* address; type_begin(VMOop, MATCH_SYMBOLS("oopDesc")) \ field(_oop_klass_offset, offset, MATCH_SYMBOLS("_metadata._klass", "_compressed_klass")) \ type_end() \ + type_begin(VMOopHandle, MATCH_SYMBOLS("OopHandle")) \ + field_with_version(_oop_handle_offset, offset, 21, MAX_VERSION, MATCH_SYMBOLS("_obj")) \ + type_end() \ + type_begin(VMCompressedOops, MATCH_SYMBOLS("CompressedOops")) \ + field_with_version(_narrow_oop_base_addr, address, 21, MAX_VERSION, MATCH_SYMBOLS("_narrow_oop._base", "_base")) \ + field_with_version(_narrow_oop_shift_addr, address, 21, MAX_VERSION, MATCH_SYMBOLS("_narrow_oop._shift", "_shift")) \ + type_end() \ type_begin(VMUniverse, MATCH_SYMBOLS("Universe", "CompressedKlassPointers")) \ field(_narrow_klass_base_addr, address, MATCH_SYMBOLS("_narrow_klass._base", "_base")) \ field(_narrow_klass_shift_addr, address, MATCH_SYMBOLS("_narrow_klass._shift", "_shift")) \ @@ -828,6 +836,13 @@ DECLARE(VMThread) JVMJavaThreadState state(); + // Only read under a collector/state guard and walkVM's fault recovery. + uintptr_t carrierThreadOop() { + if (_thread_oop_offset < 0 || _oop_handle_offset < 0) return 0; + uintptr_t* handle = *(uintptr_t**)at(_thread_oop_offset + _oop_handle_offset); + return handle == nullptr ? 0 : __atomic_load_n(handle, __ATOMIC_ACQUIRE); + } + bool inDeopt() { if (!isJavaThread(this)) return false; assert(_thread_vframe_offset >= 0); From 6697b3965ca3037e3fad539eef808fc8a2e24999 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 17:23:50 +0200 Subject: [PATCH 3/7] fix(profiler): recover inline scopes from deoptimized frozen frames --- .../main/cpp/hotspot/frozenStackWalker.cpp | 12 +- ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp | 28 +++ ddprof-lib/src/main/cpp/hotspot/vmStructs.h | 10 + .../src/test/cpp/frozenDeoptimization_ut.cpp | 141 +++++++++++ .../profiler/FrozenDeoptimizationTest.java | 237 ++++++++++++++++++ 5 files changed, 425 insertions(+), 3 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/frozenDeoptimization_ut.cpp create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java diff --git a/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp index 5c816cd1ba..7b624bb681 100644 --- a/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp @@ -51,11 +51,16 @@ bool relativePointer(uintptr_t base, intptr_t words, uintptr_t low, uintptr_t hi return true; } -bool appendCompiled(VMNMethod* nm, const void* pc, ASGCT_CallFrame* frames, +bool appendCompiled(VMNMethod* nm, const void* pc, uintptr_t unextended_sp, uintptr_t low, + uintptr_t frame_end, ASGCT_CallFrame* frames, volatile int& depth, int max_depth, bool details) { + const void* original_pc = nm->originalPC(pc, unextended_sp, low, frame_end); + if (original_pc == nullptr || (original_pc != pc && CodeHeap::findNMethod(original_pc) != nm)) return false; int level = nm->level(); FrameTypeId type = details && level >= 1 && level <= 3 ? FRAME_C1_COMPILED : FRAME_JIT_COMPILED; - int scope_offset = nm->findScopeOffset(pc); + int scope_offset = nm->findScopeOffset(original_pc); + // A deoptimized Java frame needs a decoded scope to account for its inline callers. + if (original_pc != pc && scope_offset <= 0) return false; if (scope_offset > 0) { ScopeDesc scope(nm); do { @@ -207,7 +212,8 @@ bool FrozenStackWalker::append(VMThread* thread, const void* sampled_pc, unsigne // compiled caller's unextended SP (StackChunkFrameStream::next). uintptr_t next_sp = unextended_sp + (uintptr_t)frame_size * sizeof(uintptr_t); if (next_sp <= sp || next_sp > high) return false; - if (!appendCompiled(nm, pc, frames, depth, max_depth, details)) return false; + if (!appendCompiled(nm, pc, unextended_sp, low, next_sp, + frames, depth, max_depth, details)) return false; compiled_frames++; sp = next_sp >= end - 2 * sizeof(uintptr_t) ? end : next_sp; unextended_sp = sp; diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index e82c92840e..bf517170e6 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -8,6 +8,7 @@ #include #include #include +#include "arch.h" #include "hotspot/vmStructs.inline.h" #include "hotspot/frozenStackWalker.h" #include "vmEntry.h" @@ -794,6 +795,33 @@ VMNMethod* CodeHeap::findNMethod(char* heap, const void* pc) { return nm; } +const void* VMNMethod::originalPC(const void* pc, uintptr_t unextended_sp, uintptr_t low, uintptr_t frame_end) { + if (_nmethod_orig_pc_offset < 0) return nullptr; + const void* deopt; + const void* deopt_mh; + if (_nmethod_deopt_handler_entry_offset >= 0) { + deopt = deopt_mh = at(*(int*)at(_nmethod_deopt_handler_entry_offset)); + } else if (_nmethod_deopt_handler_offset >= 0 && _nmethod_deopt_mh_handler_offset >= 0) { + deopt = at(*(int*)at(_nmethod_deopt_handler_offset)); + deopt_mh = at(*(int*)at(_nmethod_deopt_mh_handler_offset)); + } else if (_nmethod_deopt_handler_address >= 0 && _nmethod_deopt_mh_handler_address >= 0) { + deopt = *(const void**)at(_nmethod_deopt_handler_address); + deopt_mh = *(const void**)at(_nmethod_deopt_mh_handler_address); + } else { + return nullptr; + } + if (pc != deopt && pc != deopt_mh) return pc; + + // Mirrors StackChunkFrameStream::orig_pc(), including its use of unextended_sp(). + int offset = *(int*)at(_nmethod_orig_pc_offset); + if (offset < 0 || offset % sizeof(uintptr_t) != 0 || unextended_sp < low + || unextended_sp > frame_end || frame_end - unextended_sp < sizeof(uintptr_t) + || (uintptr_t)offset > frame_end - unextended_sp - sizeof(uintptr_t)) return nullptr; + const void* original = stripPointer(*(const void**)(unextended_sp + offset)); + return original >= code() && contains(original) && original != deopt && original != deopt_mh + ? original : nullptr; +} + int VMNMethod::findScopeOffset(const void* pc) { intptr_t pc_offset = (const char*)pc - code(); if (pc_offset < 0 || pc_offset > 0x7fffffff) { diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index e25d246652..756ecc6cee 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -202,6 +202,12 @@ typedef void* address; field_with_version(_nmethod_entry_address, offset, 8, 22, MATCH_SYMBOLS("_verified_entry_point")) \ field(_nmethod_state_offset, offset, MATCH_SYMBOLS("_state")) \ field(_nmethod_level_offset, offset, MATCH_SYMBOLS("_comp_level")) \ + field_with_version(_nmethod_orig_pc_offset, offset, 21, MAX_VERSION, MATCH_SYMBOLS("_orig_pc_offset")) \ + field_with_version(_nmethod_deopt_handler_address, offset, 21, 22, MATCH_SYMBOLS("_deopt_handler_begin")) \ + field_with_version(_nmethod_deopt_mh_handler_address, offset, 21, 22, MATCH_SYMBOLS("_deopt_mh_handler_begin")) \ + field_with_version(_nmethod_deopt_handler_offset, offset, 23, 25, MATCH_SYMBOLS("_deopt_handler_offset")) \ + field_with_version(_nmethod_deopt_mh_handler_offset, offset, 23, 25, MATCH_SYMBOLS("_deopt_mh_handler_offset")) \ + field_with_version(_nmethod_deopt_handler_entry_offset, offset, 26, MAX_VERSION, MATCH_SYMBOLS("_deopt_handler_entry_offset")) \ field_with_version(_nmethod_metadata_offset, offset, MIN_VERSION, 24, MATCH_SYMBOLS("_metadata_offset")) \ field_with_version(_nmethod_immutable_offset, offset, 23, MAX_VERSION, MATCH_SYMBOLS("_immutable_data")) \ field(_scopes_pcs_offset, offset, MATCH_SYMBOLS("_scopes_pcs_offset")) \ @@ -1172,6 +1178,10 @@ DECLARE(VMNMethod) } int findScopeOffset(const void* pc); + + // A deoptimized frozen frame stores its original PC relative to unextended SP. + // Returns nullptr if the saved slot or PC cannot be validated within this frame/nmethod. + const void* originalPC(const void* pc, uintptr_t unextended_sp, uintptr_t low, uintptr_t frame_end); DECLARE_END class CodeHeap : VMStructs { diff --git a/ddprof-lib/src/test/cpp/frozenDeoptimization_ut.cpp b/ddprof-lib/src/test/cpp/frozenDeoptimization_ut.cpp new file mode 100644 index 0000000000..99e2e1a70c --- /dev/null +++ b/ddprof-lib/src/test/cpp/frozenDeoptimization_ut.cpp @@ -0,0 +1,141 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include "hotspot/vmStructs.h" + +class VMStructsTestAccessor { +public: + static std::array deoptimizationOffsets() { + return {{&VMStructs::_nmethod_orig_pc_offset, + &VMStructs::_nmethod_deopt_handler_address, + &VMStructs::_nmethod_deopt_mh_handler_address, + &VMStructs::_nmethod_deopt_handler_offset, + &VMStructs::_nmethod_deopt_mh_handler_offset, + &VMStructs::_code_address, &VMStructs::_code_offset, + &VMStructs::_blob_size_offset, &VMStructs::_nmethod_deopt_handler_entry_offset}}; + } +}; + +namespace { + +// The same recovery code must handle absolute handler addresses on JDK 21/22 +// and relative offsets on JDK 23+, with a single handler entry on JDK 26+. +class FrozenDeoptimizationTest : public ::testing::TestWithParam { +protected: + struct Metadata { + int size; + int original_slot; + const void* code; + const void* handler; + const void* mh_handler; + int handler_offset; + int mh_handler_offset; + int code_offset; + }; + + alignas(16) char blob[512] = {}; + uintptr_t stack[16] = {}; + std::array fields = VMStructsTestAccessor::deoptimizationOffsets(); + std::array saved; + + void SetUp() override { + for (size_t i = 0; i < fields.size(); i++) saved[i] = *fields[i]; + *fields[0] = offsetof(Metadata, original_slot); + *fields[1] = GetParam() ? -1 : offsetof(Metadata, handler); + *fields[2] = GetParam() ? -1 : offsetof(Metadata, mh_handler); + *fields[3] = GetParam() == 1 ? offsetof(Metadata, handler_offset) : -1; + *fields[4] = GetParam() == 1 ? offsetof(Metadata, mh_handler_offset) : -1; + *fields[5] = GetParam() ? -1 : offsetof(Metadata, code); + *fields[6] = GetParam() ? offsetof(Metadata, code_offset) : -1; + *fields[7] = offsetof(Metadata, size); + *fields[8] = GetParam() == 2 ? offsetof(Metadata, handler_offset) : -1; + metadata().size = sizeof(blob); + metadata().original_slot = 0; + metadata().code = blob + 128; + metadata().handler = blob + 256; + metadata().mh_handler = blob + 288; + metadata().handler_offset = 256; + metadata().mh_handler_offset = 288; + metadata().code_offset = 128; + stack[4] = (uintptr_t)(blob + 160); + } + + void TearDown() override { + for (size_t i = 0; i < fields.size(); i++) *fields[i] = saved[i]; + } + + Metadata& metadata() { return *(Metadata*)blob; } + + const void* resolve(const void* pc) { + return ((VMNMethod*)blob)->originalPC(pc, (uintptr_t)&stack[4], + (uintptr_t)&stack[0], (uintptr_t)&stack[12]); + } +}; + +TEST_P(FrozenDeoptimizationTest, LeavesOrdinaryReturnPcUnchanged) { + EXPECT_EQ(blob + 176, resolve(blob + 176)); +} + +TEST_P(FrozenDeoptimizationTest, RestoresNormalAndMethodHandleHandlersAtSlotZero) { + EXPECT_EQ(blob + 160, resolve(blob + 256)); + if (GetParam() != 2) EXPECT_EQ(blob + 160, resolve(blob + 288)); +} + +TEST_P(FrozenDeoptimizationTest, UsesByteOffsetFromUnextendedSp) { + metadata().original_slot = 2 * sizeof(uintptr_t); + stack[4] = 0; + stack[6] = (uintptr_t)(blob + 176); + EXPECT_EQ(blob + 176, resolve(blob + 256)); +} + +TEST_P(FrozenDeoptimizationTest, AcceptsLastSlotButRejectsReadAcrossFrameEnd) { + metadata().original_slot = 7 * sizeof(uintptr_t); + stack[11] = (uintptr_t)(blob + 176); + EXPECT_EQ(blob + 176, resolve(blob + 256)); + metadata().original_slot++; + EXPECT_EQ(nullptr, resolve(blob + 256)); + metadata().original_slot = 8 * sizeof(uintptr_t); + EXPECT_EQ(nullptr, resolve(blob + 256)); +} + +TEST_P(FrozenDeoptimizationTest, RejectsNegativeAndUnalignedOffsets) { + metadata().original_slot = -1; + EXPECT_EQ(nullptr, resolve(blob + 256)); + metadata().original_slot = 1; + EXPECT_EQ(nullptr, resolve(blob + 256)); +} + +TEST_P(FrozenDeoptimizationTest, RejectsInvalidFrameBounds) { + auto* nm = (VMNMethod*)blob; + uintptr_t low = (uintptr_t)&stack[0]; + uintptr_t sp = (uintptr_t)&stack[4]; + EXPECT_EQ(nullptr, nm->originalPC(blob + 256, sp, sp + 8, sp + 64)); + EXPECT_EQ(nullptr, nm->originalPC(blob + 256, sp, low, sp - 8)); + EXPECT_EQ(nullptr, nm->originalPC(blob + 256, sp, low, sp)); +} + +TEST_P(FrozenDeoptimizationTest, RejectsHandlerNullMetadataAndForeignSavedPcs) { + for (const void* pc : std::array{{nullptr, blob + 256, GetParam() == 2 ? blob + 256 : blob + 288, + blob + 16, blob + sizeof(blob)}}) { + stack[4] = (uintptr_t)pc; + EXPECT_EQ(nullptr, resolve(blob + 256)); + } +} + +TEST_P(FrozenDeoptimizationTest, MissingMetadataDeclinesRecovery) { + *fields[0] = -1; + EXPECT_EQ(nullptr, resolve(blob + 256)); + *fields[0] = offsetof(Metadata, original_slot); + *fields[2] = *fields[4] = -1; + *fields[8] = -1; + EXPECT_EQ(nullptr, resolve(blob + 256)); +} + +INSTANTIATE_TEST_SUITE_P(HandlerLayouts, FrozenDeoptimizationTest, ::testing::Values(0, 1, 2)); + +} // namespace diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java new file mode 100644 index 0000000000..6e90db646b --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java @@ -0,0 +1,237 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.FutureTask; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.LockSupport; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** Exercises inline scope recovery after a mounted compiled frame is deoptimized and frozen. */ +public class FrozenDeoptimizationTest { + private static final String WORKLOAD = Workload.class.getName(); + private static volatile boolean entered; + private static volatile boolean released; + private static volatile boolean samplingReady; + private static volatile boolean collecting; + private static volatile long sink; + private static StackTraceElement[] logicalStack; + + /** Both VM modes must restore inline callers; the control never invalidates the nmethod. */ + @ParameterizedTest + @ValueSource(strings = {"control:vm", "deopt:vm", "deopt:vmx"}) + void recoversDeoptimizedInlineCallers(String scenario) throws Exception { + assumeTrue(Platform.isJavaVersionAtLeast(21) && !Platform.isJavaVersionAtLeast(28) + && !Platform.isJ9() && !Platform.isZing()); + assumeTrue(!Platform.isMusl() || Platform.isAarch64()); + Path directory = Files.createTempDirectory("frozen-deoptimization-"); + Path recording = directory.resolve("samples.jfr"); + Path compilation = directory.resolve("compilation.xml"); + Path output = directory.resolve("child.log"); + String[] options = scenario.split(":"); + String javaHome = System.getenv("JAVA_TEST_HOME"); + if (javaHome == null) javaHome = System.getProperty("java.home"); + List command = new ArrayList<>(Arrays.asList( + Paths.get(javaHome, "bin", "java").toString(), + "-Xms128m", "-Xmx512m", "-XX:+UseG1GC", "-XX:+UnlockDiagnosticVMOptions", + "-XX:+DebugNonSafepoints", "-XX:-TieredCompilation", "-XX:-UseTypeProfile", + "-Xbatch", "-XX:CompileThreshold=1000", "-XX:+LogCompilation", + "-XX:LogFile=" + compilation, + "-XX:CompileCommand=compileonly," + WORKLOAD + "::targetRoot", + "-XX:CompileCommand=dontinline," + WORKLOAD + "::targetRoot", + "-XX:CompileCommand=dontinline," + WORKLOAD + "::recurse", + "-XX:CompileCommand=dontinline," + WORKLOAD + "::leaf", + "-XX:CompileCommand=inline," + First.class.getName() + "::inlinedFirst", + "-XX:CompileCommand=inline," + First.class.getName() + "::inlinedSecond", + "-cp", System.getProperty("java.class.path"), getClass().getName(), options[0], + LibraryLoader.resolveLibraryPath(System.getProperty("java.io.tmpdir")).toString(), + recording.toString(), options[1])); + Process child = new ProcessBuilder(command).redirectErrorStream(true) + .redirectOutput(output.toFile()).start(); + boolean passed = false; + try { + assertTrue(child.waitFor(60, TimeUnit.SECONDS), "Child timed out: " + output); + assertEquals(0, child.exitValue(), "Child failed: " + output); + String compilerLog = new String(Files.readAllBytes(compilation), StandardCharsets.UTF_8); + Matcher compiled = Pattern.compile("]*compile_id='(\\d+)'[^>]*compiler='c2'[^>]*method='" + + Pattern.quote(WORKLOAD) + " targetRoot ").matcher(compilerLog); + assertTrue(compiled.find(), "The caller was never C2 compiled: " + compilation); + Matcher task = Pattern.compile("]*>(.*?)", Pattern.DOTALL).matcher(compilerLog); + assertTrue(task.find(), "No compilation task for the target caller"); + assertTrue(task.group(1).split("= 3, + "The target caller did not inline both methods"); + boolean invalidated = Pattern.compile("]*compile_id='" + + compiled.group(1) + "'").matcher(compilerLog).find(); + assertEquals(options[0].equals("deopt"), invalidated, + "The compiled caller must be invalidated only in the deoptimization scenario"); + if (invalidated) assertTrue(compilerLog.contains(" expected = new ArrayList<>(); + expected.add("leaf"); + for (int i = 0; i < 129; i++) expected.add("recurse"); + expected.addAll(Arrays.asList("inlinedSecond", "inlinedFirst", "targetRoot")); + int[] counts = JfrEvents.reduce(recording, "datadog.MethodSample"::equals, + () -> new int[5], (count, event) -> { + JfrStackTrace trace = event.getStackTrace(); + if (trace.isEmpty() || !"JAVA".equals(event.getEnumName("mode"))) return; + JfrFrame top = trace.frames().get(0); + if (!WORKLOAD.equals(top.className()) || !"leaf".equals(top.methodName())) return; + count[0]++; + List callers = new ArrayList<>(); + for (JfrFrame frame : trace.frames()) { + if (WORKLOAD.equals(frame.className()) || First.class.getName().equals(frame.className())) { + callers.add(frame.methodName()); + } + } + boolean full = expected.equals(callers); + if (full) count[1]++; + else if (!trace.isTruncated()) count[3]++; + // Seeing targetRoot proves the walker reached the physical compiled frame. + if (callers.contains("targetRoot") && !full) count[2]++; + if (full && cstack.equals("vm") && trace.isTruncated()) count[4]++; + }); + System.out.println("eligible=" + counts[0] + " restored=" + counts[1] + + " missingInline=" + counts[2] + " falselyComplete=" + counts[3]); + assertTrue(counts[0] > 0, "No Java-body samples exercised the workload"); + assertEquals(0, counts[2], "Compiled frames lost their inline callers"); + assertEquals(0, counts[3], "Incomplete stacks claimed completeness"); + assertTrue(counts[1] > 0, "No sample restored the complete inline chain"); + assertEquals(0, counts[4], "Restored VM stacks were marked truncated"); + } + + /** Child entry point: invalidate while mounted, then freeze before sampling the leaf. */ + public static void main(String[] args) throws Exception { + for (int i = 0; i < 20_000; i++) sink = Workload.targetRoot(0, false); + JavaProfiler profiler = JavaProfiler.getInstance(args[1], null); + FutureTask task = new FutureTask<>(() -> Workload.targetRoot(128, true)); + Thread worker = (Thread) Thread.class.getMethod("startVirtualThread", Runnable.class) + .invoke(null, task); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while ((!entered || worker.getState() != Thread.State.RUNNABLE) && System.nanoTime() < deadline) { + Thread.sleep(1); + } + if (!entered || worker.getState() != Thread.State.RUNNABLE) throw new AssertionError("Not mounted"); + try { + // Disabling type profiles makes C2 depend on the unique interface implementor. + // Resolving Second invalidates targetRoot without replacing any method bytecode. + if (args[0].equals("deopt")) Class.forName(FrozenDeoptimizationTest.class.getName() + "$Second"); + released = true; + deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!samplingReady && System.nanoTime() < deadline) Thread.sleep(1); + if (!samplingReady) throw new AssertionError("Worker did not remount"); + // Start only after the park: pre-park samples exercise the carrier walker instead. + profiler.execute("start,jfr,wall=1ms,walltpt=64,filter=,jstackdepth=512,cstack=" + + args[3] + ",file=" + args[2]); + } finally { + released = true; + collecting = true; + } + worker.join(15_000); + if (worker.isAlive()) throw new AssertionError("Worker did not finish"); + sink = task.get(); + profiler.stop(); + List logical = new ArrayList<>(); + for (StackTraceElement frame : logicalStack) { + if (WORKLOAD.equals(frame.getClassName()) || First.class.getName().equals(frame.getClassName())) { + logical.add(frame.getMethodName()); + } + } + if (logical.size() != 133 || !logical.contains("inlinedFirst") || !logical.contains("inlinedSecond")) { + throw new AssertionError("JVM logical callers were not preserved: " + logical); + } + if (profiler.getDebugCounters().getOrDefault("walkvm_frozen_recovered", 0L) <= 0) { + throw new AssertionError("No frozen callers were recovered"); + } + } + + /** Interface dispatch gives the compiled caller a class hierarchy dependency. */ + public interface Strategy { + /** Invokes the recursive workload through two inline scopes. */ + long inlinedFirst(int depth, boolean active); + } + + /** The sole loaded implementor during C2 compilation. */ + public static class First implements Strategy { + @Override + public long inlinedFirst(int depth, boolean active) { + return inlinedSecond(depth, active) + 13; + } + + private long inlinedSecond(int depth, boolean active) { + return Workload.recurse(depth, active) + 17; + } + } + + /** Loaded only when the mounted caller must be invalidated. */ + public static class Second implements Strategy { + @Override + public long inlinedFirst(int depth, boolean active) { + return 0; + } + } + + /** Only targetRoot is compiled; the deep interpreted chain keeps it frozen after remounting. */ + public static class Workload { + private static volatile Strategy strategy = new First(); + + static long targetRoot(int depth, boolean active) { + return strategy.inlinedFirst(depth, active) + 11; + } + + static long recurse(int depth, boolean active) { + long result = depth > 0 ? recurse(depth - 1, active) : leaf(active); + return result + depth; + } + + /** A real park after invalidation freezes the still-live compiled caller. */ + static long leaf(boolean active) { + if (!active) return 19; + entered = true; + while (!released) sink++; + LockSupport.parkNanos(500_000L); + logicalStack = Thread.currentThread().getStackTrace(); + samplingReady = true; + while (!collecting) sink++; + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(3); + long value = 23; + while (System.nanoTime() < deadline) { + for (int i = 0; i < 4096; i++) value = (value ^ i) * 0x5deece66dL + 11; + sink = value; + } + return value; + } + } +} From f42032e92286602eee1e4515e62dbf1cf6a12d68 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 18:02:28 +0200 Subject: [PATCH 4/7] test(profiler): select C2 for frozen-frame regression on GraalVM --- .../java/com/datadoghq/profiler/FrozenDeoptimizationTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java index 6e90db646b..9e88ce0194 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/FrozenDeoptimizationTest.java @@ -62,6 +62,9 @@ void recoversDeoptimizedInlineCallers(String scenario) throws Exception { "-cp", System.getProperty("java.class.path"), getClass().getName(), options[0], LibraryLoader.resolveLibraryPath(System.getProperty("java.io.tmpdir")).toString(), recording.toString(), options[1])); + if (Platform.isGraal()) { + command.add(1, "-XX:-UseJVMCICompiler"); + } Process child = new ProcessBuilder(command).redirectErrorStream(true) .redirectOutput(output.toFile()).start(); boolean passed = false; From d042c0b61d0782c880a5168dd2a6e075002343c2 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 23:25:31 +0200 Subject: [PATCH 5/7] fix(profiler): reject unverified nested continuation recovery --- .../main/cpp/hotspot/frozenStackWalker.cpp | 13 +- .../ContinuationTransitionSupport.java | 84 +++++++ .../NestedContinuationTransitionTest.java | 232 ++++++++++++++++++ 3 files changed, 322 insertions(+), 7 deletions(-) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContinuationTransitionSupport.java create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/NestedContinuationTransitionTest.java diff --git a/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp index 7b624bb681..6d56683f57 100644 --- a/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/frozenStackWalker.cpp @@ -132,7 +132,7 @@ bool FrozenStackWalker::append(VMThread* thread, const void* sampled_pc, unsigne ASGCT_CallFrame* frames, volatile int& depth, int max_depth, bool details) { if (!layout.supported || thread == nullptr || thread->state() != _thread_in_Java - || continuation_depth >= 64 || depth >= max_depth) return false; + || continuation_depth != 0 || depth >= max_depth) return false; // Freeze/thaw runs without a Java-to-VM transition. During doYield or its // runtime calls, copied frames can still be on the carrier as well as in a @@ -143,13 +143,12 @@ bool FrozenStackWalker::append(VMThread* thread, const void* sampled_pc, unsigne uintptr_t carrier = thread->carrierThreadOop(); if (carrier == 0) return false; - // Continuation.run publishes the innermost mounted continuation in the - // carrier Thread.cont, including when the current Java thread is virtual. + // Continuation.run publishes a child in Thread.cont before enterSpecial, + // and restores the parent after it returns. In those Java intervals the + // native boundary belongs to the parent. Without matching native-entry + // identity, recover only a single, outermost continuation. uintptr_t continuation = readOop(carrier, layout.carrier_cont); - for (unsigned int i = 0; i < continuation_depth && continuation != 0; i++) { - continuation = readOop(continuation, layout.cont_parent); - } - if (continuation == 0) return false; + if (continuation == 0 || readOop(continuation, layout.cont_parent) != 0) return false; uintptr_t chunk = readOop(continuation, layout.cont_tail); if (chunk == 0) return false; int interpreted_frames = 0; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContinuationTransitionSupport.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContinuationTransitionSupport.java new file mode 100644 index 0000000000..bedd0a0dc9 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ContinuationTransitionSupport.java @@ -0,0 +1,84 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +/** Extends the real Java intervals around a nested continuation's native entry in a forked JVM. */ +final class ContinuationTransitionSupport { + static final String HOOK = "ddprof.test.continuation-transition-hook"; + + private ContinuationTransitionSupport() {} + + /** Writes a java.base patch because HotSpot does not permit retransformation of Continuation. */ + static void writePatch(Path directory) throws Exception { + int[] sites = {0}; + ClassWriter writer; + try (InputStream input = ClassLoader.getSystemResourceAsStream("jdk/internal/vm/Continuation.class")) { + if (input == null) throw new IllegalStateException("Continuation class bytes are unavailable"); + ClassReader reader = new ClassReader(input); + writer = new ClassWriter(reader, ClassWriter.COMPUTE_FRAMES); + reader.accept(new ClassVisitor(Opcodes.ASM9, writer) { + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, + String signature, String[] exceptions) { + MethodVisitor visitor = super.visitMethod(access, name, descriptor, signature, exceptions); + if (!name.equals("run") || !descriptor.equals("()V")) return visitor; + return new MethodVisitor(Opcodes.ASM9, visitor) { + @Override + public void visitMethodInsn(int opcode, String owner, String name, + String descriptor, boolean isInterface) { + boolean entry = owner.equals("jdk/internal/vm/Continuation") + && name.equals("enterSpecial"); + if (entry) { + sites[0]++; + hook(1); + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + if (entry) hook(2); + } + + /** Dispatches through a java.base interface without adding a bootstrap helper. */ + private void hook(int phase) { + Label absent = new Label(); + Label done = new Label(); + super.visitMethodInsn(Opcodes.INVOKESTATIC, "java/lang/System", "getProperties", + "()Ljava/util/Properties;", false); + super.visitLdcInsn(HOOK); + super.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/util/Properties", "get", + "(Ljava/lang/Object;)Ljava/lang/Object;", false); + super.visitInsn(Opcodes.DUP); + super.visitJumpInsn(Opcodes.IFNULL, absent); + super.visitTypeInsn(Opcodes.CHECKCAST, "java/util/function/BiConsumer"); + super.visitVarInsn(Opcodes.ALOAD, 0); + super.visitLdcInsn(phase); + super.visitMethodInsn(Opcodes.INVOKESTATIC, "java/lang/Integer", "valueOf", + "(I)Ljava/lang/Integer;", false); + super.visitMethodInsn(Opcodes.INVOKEINTERFACE, "java/util/function/BiConsumer", "accept", + "(Ljava/lang/Object;Ljava/lang/Object;)V", true); + super.visitJumpInsn(Opcodes.GOTO, done); + super.visitLabel(absent); + super.visitInsn(Opcodes.POP); + super.visitLabel(done); + } + }; + } + }, ClassReader.EXPAND_FRAMES); + } + if (sites[0] != 2) throw new IllegalStateException("Expected both enterSpecial call sites, found " + sites[0]); + Path output = directory.resolve("jdk/internal/vm/Continuation.class"); + Files.createDirectories(output.getParent()); + Files.write(output, writer.toByteArray()); + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/NestedContinuationTransitionTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/NestedContinuationTransitionTest.java new file mode 100644 index 0000000000..e3594a3004 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/NestedContinuationTransitionTest.java @@ -0,0 +1,232 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.JavaProfiler; +import com.datadoghq.profiler.JfrEvent; +import com.datadoghq.profiler.JfrEvents; +import com.datadoghq.profiler.JfrFrame; +import com.datadoghq.profiler.JfrStackTrace; +import com.datadoghq.profiler.Platform; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import java.util.concurrent.FutureTask; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.LockSupport; +import java.util.function.BiConsumer; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** Checks heap-chunk ownership in the Java intervals before and after a nested native entry. */ +public class NestedContinuationTransitionTest { + private static volatile long sink; + private static Object child; + private static Object scope; + private static Method run; + private static Method yield; + + /** Suspended child frames must never be spliced into a partially thawed parent's stack. */ + @ParameterizedTest + @ValueSource(strings = {"vm", "vmx"}) + public void rejectsSuspendedChildChunks(String mode) throws Exception { + assumeTrue(Platform.isJavaVersionAtLeast(21) && !Platform.isJ9() && !Platform.isZing()); + assumeTrue(!(Boolean.getBoolean("ddprof_test.ci") && Platform.isMusl() && !Platform.isAarch64())); + Path directory = Files.createTempDirectory("nested-continuation-transition-"); + Path recording = directory.resolve("transition.jfr"); + Path log = directory.resolve("child.log"); + Path patch = directory.resolve("java-base-patch"); + ContinuationTransitionSupport.writePatch(patch); + + List command = Arrays.asList( + Paths.get(System.getProperty("java.home"), "bin", "java").toString(), + "--add-opens=java.base/jdk.internal.vm=ALL-UNNAMED", + "--add-opens=java.base/java.lang=ALL-UNNAMED", + "-Xmx256m", "-XX:+UseG1GC", "-XX:+UnlockDiagnosticVMOptions", "-XX:+DebugNonSafepoints", + "-XX:CompileCommand=dontinline,*NestedContinuationTransitionTest.*", + "--patch-module=java.base=" + patch, "-cp", System.getProperty("java.class.path"), + getClass().getName(), directory.toString(), + recording.toString(), mode); + boolean verified = false; + Process process = new ProcessBuilder(command).redirectErrorStream(true).redirectOutput(log.toFile()).start(); + try { + assertTrue(process.waitFor(60, TimeUnit.SECONDS), "Child timed out; see " + log); + assertEquals(0, process.exitValue(), "Child failed; see " + log); + assertTransitions(recording); + verified = true; + System.out.println("Nested transition " + mode + " recording: " + recording); + } finally { + if (process.isAlive()) { + process.destroyForcibly(); + process.waitFor(5, TimeUnit.SECONDS); + } + if (verified && !Boolean.getBoolean("ddprof_test.keep_jfrs")) { + try (Stream files = Files.walk(directory)) { + for (Path file : files.sorted(Comparator.reverseOrder()).toArray(Path[]::new)) Files.delete(file); + } + } + } + } + + /** Checks both engines in both intervals and distinguishes genuine parent frames from child chunks. */ + private void assertTransitions(Path recording) throws Exception { + int[][] samples = new int[2][2]; + int[][] missingParent = new int[2][2]; + int foreign = 0; + int falseComplete = 0; + for (int engine = 0; engine < 2; engine++) { + String event = engine == 0 ? "datadog.ExecutionSample" : "datadog.MethodSample"; + for (JfrEvent sample : JfrEvents.load(recording, event)) { + JfrStackTrace trace = sample.getStackTrace(); + if (trace == null) continue; + int phase = contains(trace, "beforeChildEntry") ? 0 : contains(trace, "afterChildExit") ? 1 : -1; + if (phase < 0) continue; + samples[engine][phase]++; + if (contains(trace, "suspendedChildRoot") || contains(trace, "suspendedChildCallers")) foreign++; + if (!contains(trace, "parentRoot")) { + missingParent[engine][phase]++; + if (!trace.isTruncated()) falseComplete++; + } + } + } + for (int[] engine : samples) { + assertTrue(engine[0] > 0, "The child-publication interval was not sampled"); + assertTrue(engine[1] > 0, "The child-return interval was not sampled"); + } + for (int[] engine : missingParent) { + assertTrue(engine[0] > 0 && engine[1] > 0, + "The parent must retain frozen callers during both sampled transitions"); + } + assertEquals(0, foreign, "Parent samples contain suspended child frames: " + recording); + assertEquals(0, falseComplete, "Missing parent callers were reported complete: " + recording); + } + + /** Matches an application method without relying on hidden-frame rendering. */ + private static boolean contains(JfrStackTrace trace, String method) { + for (JfrFrame frame : trace.frames()) { + if (NestedContinuationTransitionTest.class.getName().equals(frame.className()) + && method.equals(frame.methodName())) return true; + } + return false; + } + + /** Runs in isolation because the patch extends intervals inside a java.base class. */ + public static void main(String[] args) throws Exception { + Class continuation = Class.forName("jdk.internal.vm.Continuation"); + Class scopeType = Class.forName("jdk.internal.vm.ContinuationScope"); + scope = scopeType.getConstructor(String.class).newInstance("nested-profiler-regression"); + run = continuation.getMethod("run"); + yield = continuation.getMethod("yield", scopeType); + child = continuation.getConstructor(scopeType, Runnable.class) + .newInstance(scope, (Runnable) NestedContinuationTransitionTest::suspendedChildRoot); + run.invoke(child); // Leave distinct child callers frozen before mounting the parent. + + Field parent = continuation.getDeclaredField("parent"); + parent.setAccessible(true); + Field tail = continuation.getDeclaredField("tail"); + tail.setAccessible(true); + Field carrierCont = Thread.class.getDeclaredField("cont"); + carrierCont.setAccessible(true); + Method currentCarrier = Thread.class.getDeclaredMethod("currentCarrierThread"); + currentCarrier.setAccessible(true); + int[] phases = {0, 0}; + BiConsumer hook = (value, phase) -> { + if (value != child) return; + try { + if (parent.get(child) == null || tail.get(child) == null + || carrierCont.get(currentCarrier.invoke(null)) != child) { + throw new AssertionError("The real transition must publish a suspended child over its parent"); + } + phases[phase - 1]++; + if (phase == 1) beforeChildEntry(); + else afterChildExit(); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + }; + JavaProfiler profiler = JavaProfiler.getInstance(null, args[0]); + profiler.execute("start,cpu=1ms,event=itimer,wall=1ms,walltpt=64,filter=,jstackdepth=256,cstack=" + + args[2] + ",file=" + args[1]); + System.getProperties().put(ContinuationTransitionSupport.HOOK, hook); + try { + FutureTask task = new FutureTask<>(() -> { + parentRoot(); + return null; + }); + Thread worker = (Thread) Thread.class.getMethod("startVirtualThread", Runnable.class).invoke(null, task); + worker.join(30_000); + if (worker.isAlive()) throw new AssertionError("Nested continuation workload did not finish"); + task.get(); + } finally { + System.getProperties().remove(ContinuationTransitionSupport.HOOK); + profiler.stop(); + } + if (phases[0] != 1 || phases[1] != 1) throw new AssertionError("Both real transition hooks must run once"); + if (profiler.getDebugCounters().getOrDefault("walkvm_cont_barrier_hit", 0L) == 0) { + throw new AssertionError("The parent must retain frozen callers at a return barrier"); + } + } + + private static void parentRoot() throws Exception { + parentCallers(128); + } + + /** Forces partial thaw before entering the already suspended nested continuation. */ + private static void parentCallers(int depth) throws Exception { + if (depth > 0) parentCallers(depth - 1); + else { + LockSupport.parkNanos(1_000_000L); + run.invoke(child); + } + sink += depth; + } + + private static void suspendedChildRoot() { + suspendedChildCallers(64); + } + + /** Yields twice so both transition intervals have nonempty child chunks available. */ + private static void suspendedChildCallers(int depth) { + if (depth > 0) suspendedChildCallers(depth - 1); + else { + try { + yield.invoke(null, scope); + yield.invoke(null, scope); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } + sink += depth; + } + + private static void beforeChildEntry() { + spin(); + } + + private static void afterChildExit() { + spin(); + } + + /** Keeps the real Java transition open long enough for both asynchronous engines to sample it. */ + private static void spin() { + long deadline = System.nanoTime() + 2_000_000_000L; + long sum = 1; + while (System.nanoTime() < deadline) { + for (int i = 0; i < 1000; i++) sum = sum * 1664525 + 1013904223; + sink = sum; + } + } +} From 496d91618679acc0e6f79d4a93f84254cd7780cf Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Sat, 24 Oct 2026 00:28:22 +0200 Subject: [PATCH 6/7] fix(profiler): prevent anchor replay across continuation boundaries Clear live and saved Java-frame anchors when crossing a continuation boundary so fallback recovery cannot duplicate continuation roots or carrier frames. Add deterministic regressions and report the first invalid stack in the virtual-thread chaos test. --- .../src/main/cpp/hotspot/hotspotSupport.cpp | 4 + .../src/test/cpp/continuationWalk_ut.cpp | 73 ++++++++++++++++++- .../VirtualThreadCarrierChaosTest.java | 12 ++- 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 3e547be632..3701a86ccb 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -442,6 +442,10 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // // Returns true to continue the walk, false to break. auto walkThroughContinuation = [&](ContBoundary kind) -> bool { + // Neither inline recovery nor the post-loop fallback may replay the + // Java anchor after this continuation boundary has been reached. + anchor = NULL; + saved_anchor_sp = 0; if (!contBoundaryProvesComplete(kind)) { bool was_truncated = truncated != nullptr && *truncated; // A recovered fault can bypass append's return and the rest of this diff --git a/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp b/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp index 5ad071de42..737e6e624d 100644 --- a/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp +++ b/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp @@ -71,7 +71,9 @@ bool isThreadEntry(const char* name) { return strcmp(name, "continuation_thread_ class ContinuationWalkTest : public ::testing::Test, protected VMStructs { protected: static constexpr int MAX_DEPTH = 8; - struct ThreadLayout { void* entry; } thread{}; + struct AnchorLayout { uintptr_t sp; uintptr_t fp; const void* pc; }; + struct ThreadLayout { void* entry; AnchorLayout anchor; void* vframe; } thread{}; + const char* sampled_pc = LEAF_PC; alignas(16) uintptr_t stack[32]{}; int entry_frame_size = 8; ASGCT_CallFrame frames[MAX_DEPTH + 1]{}; @@ -101,6 +103,12 @@ class ContinuationWalkTest : public ::testing::Test, protected VMStructs { old_entry_offset = _cont_entry_offset; old_frame_size_offset = _frame_size_offset; old_entry_nm = _enter_special_nm; + old_anchor_offset = _thread_anchor_offset; + old_anchor_sp = _anchor_sp_offset; + old_anchor_fp = _anchor_fp_offset; + old_anchor_pc = _anchor_pc_offset; + old_vframe_offset = _thread_vframe_offset; + old_anchor_size = TYPE_SIZE_NAME(VMJavaFrameAnchor); old_entry_size = TYPE_SIZE_NAME(VMContinuationEntry); old_thread_size = TYPE_SIZE_NAME(VMThread); ASSERT_EQ(0, pthread_key_create(&owned_key, nullptr)); @@ -108,14 +116,20 @@ class ContinuationWalkTest : public ::testing::Test, protected VMStructs { VMTestAccessor::install(true, 27); ProfiledThread::initCurrentThread(); ASSERT_NE(nullptr, ProfiledThread::current()); - // No Java frames need decoding: only the thread's exported entry field - // is read. Suppress vtable/anchor introspection on this synthetic thread. + // No Java frames need decoding. Disable anchor introspection by default; + // tests that exercise the synthetic Java anchor enable it explicitly. ProfiledThread::current()->setJavaThread(false); thread.entry = &stack[6]; JVMThreadTestAccessor::install(owned_key, (jfieldID)1); _cont_entry_offset = offsetof(ThreadLayout, entry); _frame_size_offset = 0; _enter_special_nm = VMNMethod::cast_raw(&entry_frame_size); + _thread_anchor_offset = offsetof(ThreadLayout, anchor); + _anchor_sp_offset = offsetof(AnchorLayout, sp); + _anchor_fp_offset = offsetof(AnchorLayout, fp); + _anchor_pc_offset = offsetof(AnchorLayout, pc); + _thread_vframe_offset = offsetof(ThreadLayout, vframe); + TYPE_SIZE_NAME(VMJavaFrameAnchor) = sizeof(AnchorLayout); TYPE_SIZE_NAME(VMThread) = sizeof(ThreadLayout); TYPE_SIZE_NAME(VMContinuationEntry) = 0; _code_heap_low = CODE_BASE + 0x200; @@ -132,6 +146,12 @@ class ContinuationWalkTest : public ::testing::Test, protected VMStructs { _cont_entry_offset = old_entry_offset; _frame_size_offset = old_frame_size_offset; _enter_special_nm = old_entry_nm; + _thread_anchor_offset = old_anchor_offset; + _anchor_sp_offset = old_anchor_sp; + _anchor_fp_offset = old_anchor_fp; + _anchor_pc_offset = old_anchor_pc; + _thread_vframe_offset = old_vframe_offset; + TYPE_SIZE_NAME(VMJavaFrameAnchor) = old_anchor_size; TYPE_SIZE_NAME(VMContinuationEntry) = old_entry_size; TYPE_SIZE_NAME(VMThread) = old_thread_size; JVMThreadTestAccessor::install(old_key, old_tid); @@ -159,7 +179,7 @@ class ContinuationWalkTest : public ::testing::Test, protected VMStructs { context.uc_mcontext = &machine_context; #endif StackFrame frame(&context); - frame.pc() = (uintptr_t)LEAF_PC; + frame.pc() = (uintptr_t)sampled_pc; frame.sp() = (uintptr_t)stack; frame.fp() = (uintptr_t)&stack[8]; StackWalkFeatures features{}; @@ -183,6 +203,8 @@ class ContinuationWalkTest : public ::testing::Test, protected VMStructs { const void *old_low, *old_high, *old_barrier, *old_return_pc; int old_entry_offset; int old_frame_size_offset; + int old_anchor_offset, old_anchor_sp, old_anchor_fp, old_anchor_pc, old_vframe_offset; + uint64_t old_anchor_size; VMNMethod* old_entry_nm; uint64_t old_entry_size; uint64_t old_thread_size; @@ -267,6 +289,49 @@ TEST_F(ContinuationWalkTest, FrozenBoundaryWithoutCarrierFramesTruncates) { EXPECT_TRUE(truncated); } +TEST_F(ContinuationWalkTest, FrozenRootDoesNotReplayJavaAnchor) { + _cont_return_barrier = BOUNDARY_PC; + sampled_pc = BOUNDARY_PC; + ProfiledThread::current()->setJavaThread(true); + thread.anchor = {(uintptr_t)&stack[2], 0, LEAF_PC}; + auto before = Counters::getCounter(WALKVM_ANCHOR_FALLBACK); + bool truncated = false; + EXPECT_EQ(1, walk(false, &truncated, true)); + EXPECT_STREQ("JVM Continuation", (const char*)frames[0].method_id); + EXPECT_TRUE(truncated); + EXPECT_EQ(before, Counters::getCounter(WALKVM_ANCHOR_FALLBACK)); +} + +TEST_F(ContinuationWalkTest, ThawedRootDoesNotReplayJavaAnchor) { + _cont_entry_return_pc = BOUNDARY_PC; + sampled_pc = BOUNDARY_PC; + ProfiledThread::current()->setJavaThread(true); + thread.anchor = {(uintptr_t)&stack[2], 0, LEAF_PC}; + auto before = Counters::getCounter(WALKVM_ANCHOR_FALLBACK); + bool truncated = false; + EXPECT_EQ(1, walk(false, &truncated)); + EXPECT_STREQ("JVM Continuation", (const char*)frames[0].method_id); + EXPECT_FALSE(truncated); + EXPECT_EQ(before, Counters::getCounter(WALKVM_ANCHOR_FALLBACK)); +} + +TEST_F(ContinuationWalkTest, CarrierWalkDoesNotReplayJavaAnchor) { + sampled_pc = BOUNDARY_PC; + ProfiledThread::current()->setJavaThread(true); + for (bool frozen : {false, true}) { + _cont_return_barrier = frozen ? BOUNDARY_PC : nullptr; + _cont_entry_return_pc = frozen ? nullptr : BOUNDARY_PC; + TYPE_SIZE_NAME(VMContinuationEntry) = (frozen ? 6 : 2) * sizeof(uintptr_t); + thread.anchor = {(uintptr_t)&stack[2], 0, LEAF_PC}; + auto before = Counters::getCounter(WALKVM_ANCHOR_FALLBACK); + bool truncated = false; + EXPECT_EQ(1, walk(true, &truncated, frozen)); + EXPECT_STREQ("continuation_carrier", (const char*)frames[0].method_id); + EXPECT_EQ(frozen, truncated); + EXPECT_EQ(before, Counters::getCounter(WALKVM_ANCHOR_FALLBACK)); + } +} + TEST_F(ContinuationWalkTest, VerifiedThawedBoundaryPreservesExistingTruncation) { _cont_entry_return_pc = BOUNDARY_PC; bool truncated = true; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/VirtualThreadCarrierChaosTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/VirtualThreadCarrierChaosTest.java index ea09e9ace0..9d30958c16 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/VirtualThreadCarrierChaosTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/VirtualThreadCarrierChaosTest.java @@ -74,7 +74,8 @@ private void assertStacks(String eventType) { StackCounts counts = reduceEvents(eventType, StackCounts::new, StackCounts::accept); System.out.println(eventType + " carrier=" + carrierFrames + " " + counts); assertEquals(0, counts.continuationErrors, "Continuation recovery errors: " + counts); - assertEquals(0, counts.invalidAncestry, "Missing, duplicated or misplaced carrier frames: " + counts); + assertEquals(0, counts.invalidAncestry, "Missing, duplicated or misplaced carrier frames: " + counts + + (counts.firstInvalidAncestry == null ? "" : "\nFirst invalid stack: " + counts.firstInvalidAncestry)); assertEquals(0, counts.falseComplete, "Missing outer callers reported complete: " + counts); for (int layout = 0; layout < 2; layout++) { assertTrue(counts.eligible[layout] >= 10, @@ -88,6 +89,7 @@ private final class StackCounts { private final long[] otherStops = new long[2]; private long continuationErrors; private long invalidAncestry; + private String firstInvalidAncestry; private long falseComplete; private void accept(JfrEvent event) { @@ -132,7 +134,13 @@ private void accept(JfrEvent event) { eligible[layout]++; if (register && stack || (carrierFrames ? carriers != 1 || roots != 0 || carrierIndex <= lastApplicationIndex - : carriers != 0 || roots != 1)) invalidAncestry++; + : carriers != 0 || roots != 1)) { + invalidAncestry++; + if (firstInvalidAncestry == null) { + firstInvalidAncestry = "register=" + register + " stack=" + stack + " carriers=" + carriers + + " roots=" + roots + " truncated=" + trace.isTruncated() + "\n" + event.getStackTraceString(); + } + } if (!outer && !trace.isTruncated()) falseComplete++; } From a2b38e8e15f7397a71d9083784e417e1222260d5 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Sat, 24 Oct 2026 11:00:36 +0200 Subject: [PATCH 7/7] fix(profiler): prevent phantom ARM64 runtime callers Account for C1 temporary argument and save areas when recovering compiled callers. Use live LR for native DWARF entry and restored-return rules, and stop with truncation at rt_sigreturn when signal-frame CFI is unavailable. Add native regressions for caller stack adjustment, stale LR slots and unsupported signal-frame walks. Correct synthetic CFI fixtures to declare their saved return addresses. --- ddprof-lib/src/main/cpp/dwarf.cpp | 18 ++- ddprof-lib/src/main/cpp/dwarfStep.inline.h | 21 +++- .../src/main/cpp/hotspot/hotspotSupport.cpp | 2 +- .../src/main/cpp/stackFrame_aarch64.cpp | 27 ++++ ddprof-lib/src/main/cpp/stackWalker.cpp | 2 +- ddprof-lib/src/test/cpp/dwarf_ut.cpp | 51 ++++++++ .../test/cpp/returnAddressAttribution_ut.cpp | 115 ++++++++++++++---- .../src/test/cpp/stackFrame_aarch64_ut.cpp | 110 +++++++++++++++++ 8 files changed, 314 insertions(+), 32 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/stackFrame_aarch64_ut.cpp diff --git a/ddprof-lib/src/main/cpp/dwarf.cpp b/ddprof-lib/src/main/cpp/dwarf.cpp index 9a643306d8..c8fd8ae028 100644 --- a/ddprof-lib/src/main/cpp/dwarf.cpp +++ b/ddprof-lib/src/main/cpp/dwarf.cpp @@ -435,12 +435,12 @@ void DwarfParser::parseInstructions(u32 loc, const char *end, const CieInfo &cie u32 cfa_reg = DW_REG_SP; int cfa_off = EMPTY_FRAME_SIZE; int fp_off = DW_SAME_FP; - int pc_off = -EMPTY_FRAME_SIZE; + int pc_off = INITIAL_PC_OFFSET; u32 rem_cfa_reg = DW_REG_SP; int rem_cfa_off = EMPTY_FRAME_SIZE; int rem_fp_off = DW_SAME_FP; - int rem_pc_off = -EMPTY_FRAME_SIZE; + int rem_pc_off = INITIAL_PC_OFFSET; while (_ptr < end) { u8 op = get8(); @@ -476,8 +476,18 @@ void DwarfParser::parseInstructions(u32 loc, const char *end, const CieInfo &cie } break; case DW_CFA_restore_extended: + case DW_CFA_same_value: { + u32 reg = getLeb(); + if (reg == DW_REG_FP) { + fp_off = DW_SAME_FP; + } else if (EMPTY_FRAME_SIZE == 0 && reg == DW_REG_PC) { + // AArch64's epilogue restores LR before returning. Its old stack + // slot is below the live SP and can be overwritten by a signal. + pc_off = INITIAL_PC_OFFSET; + } + break; + } case DW_CFA_undefined: - case DW_CFA_same_value: if (getLeb() == DW_REG_FP) { fp_off = DW_SAME_FP; } @@ -587,6 +597,8 @@ void DwarfParser::parseInstructions(u32 loc, const char *end, const CieInfo &cie case DW_CFA_restore: if ((op & 0x3f) == DW_REG_FP) { fp_off = DW_SAME_FP; + } else if (EMPTY_FRAME_SIZE == 0 && (op & 0x3f) == DW_REG_PC) { + pc_off = INITIAL_PC_OFFSET; } break; } diff --git a/ddprof-lib/src/main/cpp/dwarfStep.inline.h b/ddprof-lib/src/main/cpp/dwarfStep.inline.h index 638f3c782a..f9be5dd486 100644 --- a/ddprof-lib/src/main/cpp/dwarfStep.inline.h +++ b/ddprof-lib/src/main/cpp/dwarfStep.inline.h @@ -15,6 +15,10 @@ #include "stackFrame.h" #include "stackWalker.inline.h" +#if defined(__linux__) && defined(__aarch64__) +#include +#endif + // One step of the DWARF unwind, shared by StackWalker::walkDwarf and // HotspotSupport::walkVM. The two had drifted into separate copies; 61959b06f // and the commit that added this reconciled the last of the differences so @@ -44,7 +48,7 @@ __attribute__((always_inline)) inline bool advanceDwarfFrame(WalkPc& walk_pc, uintptr_t& sp, uintptr_t& fp, uintptr_t bottom, int depth, - const StackFrame& frame, Profiler* profiler) { + const StackFrame& frame, Profiler* profiler, bool* truncated) { using StackWalkValidation::inDeadZone; using StackWalkValidation::aligned; using StackWalkValidation::MAX_FRAME_SIZE; @@ -60,6 +64,21 @@ inline bool advanceDwarfFrame(WalkPc& walk_pc, uintptr_t& sp, uintptr_t& fp, CodeCache* cc = profiler->findLibraryByAddress(attribution_pc); FrameDesc f = cc != NULL ? cc->findFrameDesc(attribution_pc) : FrameDesc::fallback_default_frame(); +#if defined(__linux__) && defined(__aarch64__) + // Without signal-frame CFI, rt_sigreturn's FP chain leads into the + // interrupted context without restoring its saved PC, fabricating callers. + if (!f.isSignalFrame()) { + const instruction_t* ip = (const instruction_t*)walk_pc.raw(); + instruction_t insn = SafeAccess::load32((int32_t*)ip); + const instruction_t sigreturn = 0x52800008 | (SYS_rt_sigreturn << 5); // mov w8/x8, #rt_sigreturn + if (((insn & 0x7fffffff) == sigreturn && SafeAccess::load32((int32_t*)(ip + 1)) == (int32_t)0xd4000001) + || (insn == 0xd4000001 && (SafeAccess::load32((int32_t*)(ip - 1)) & 0x7fffffff) == sigreturn)) { + if (truncated != nullptr) *truncated = true; + return false; + } + } +#endif + u8 cfa_reg = (u8)f.cfa; int cfa_off = f.cfa >> 8; if (cfa_reg == DW_REG_SP) { diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 3701a86ccb..f87409fc39 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -1031,7 +1031,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex have_prev_native_pc = true; if (!advanceDwarfFrame(walk_pc, sp, fp, bottom, depth, frame, - profiler)) { + profiler, truncated)) { break; } } diff --git a/ddprof-lib/src/main/cpp/stackFrame_aarch64.cpp b/ddprof-lib/src/main/cpp/stackFrame_aarch64.cpp index 11174114ce..36ef0b743d 100644 --- a/ddprof-lib/src/main/cpp/stackFrame_aarch64.cpp +++ b/ddprof-lib/src/main/cpp/stackFrame_aarch64.cpp @@ -85,6 +85,33 @@ NOSANALIGSANITIZE void StackFrame::adjustSP(const void* entry, const void* pc, u // ... // add sp, sp, #0x10 sp += 16; + return; + } + + if (ip <= entry) return; + bool direct_call = (ip[-1] & 0xfc000000) == 0x94000000; // bl + bool indirect_call = (ip[-1] & 0xfffffc1f) == 0xd63f0000; // blr + bool pops_pair = (ip[0] & 0xffff83e0) == 0xa8c103e0; // ldp ..., [sp], #16 + bool pops_single = (ip[0] & 0xffffffe0) == 0xf84107e0; // ldr ..., [sp], #16 + if (!(direct_call || indirect_call) || !(pops_pair || pops_single)) return; + + // C1 subtype-check callers push two arguments before far_call and pop + // them at its return PC. call_VM_leaf_base similarly saves x8/x12. + // These words are outside the nmethod frame size. Require the matching + // push, allowing only far_call's address setup between it and the call. + unsigned int call_reg = (ip[-1] >> 5) & 31; + for (int back = 2; back <= 5 && ip - (const instruction_t*)entry >= back; back++) { + instruction_t insn = ip[-back]; + if ((insn & 0xffff83e0) == 0xa9bf03e0) { // stp ..., [sp, #-16]! + // FP/LR saves are part of the fixed frame, not outgoing arguments. + if ((insn & 31) < 29 && ((insn >> 10) & 31) < 29) sp += 16; + return; + } + bool address_setup = (insn & 0xff80001f) == (0xd2800000 | call_reg) // movz + || (insn & 0xff80001f) == (0xf2800000 | call_reg) // movk + || (insn & 0x9f00001f) == (0x90000000 | call_reg) // adrp + || (insn & 0xffc003ff) == (0x91000000 | call_reg << 5 | call_reg); // add + if (!indirect_call || !address_setup) return; } } diff --git a/ddprof-lib/src/main/cpp/stackWalker.cpp b/ddprof-lib/src/main/cpp/stackWalker.cpp index 229e6827eb..15b9ac4870 100644 --- a/ddprof-lib/src/main/cpp/stackWalker.cpp +++ b/ddprof-lib/src/main/cpp/stackWalker.cpp @@ -194,7 +194,7 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth callchain[depth++] = attribution_pc; if (!advanceDwarfFrame(walk_pc, sp, fp, bottom, depth, frame, - profiler)) { + profiler, truncated)) { break; } } diff --git a/ddprof-lib/src/test/cpp/dwarf_ut.cpp b/ddprof-lib/src/test/cpp/dwarf_ut.cpp index 27faaa4f1d..34c3ccfe0a 100644 --- a/ddprof-lib/src/test/cpp/dwarf_ut.cpp +++ b/ddprof-lib/src/test/cpp/dwarf_ut.cpp @@ -805,4 +805,55 @@ TEST(DwarfEhFrame, FdeAfterValidCieProducesRows) { delete dwarf; } +#ifdef __aarch64__ + +TEST(DwarfEhFrame, ArmLeafReturnAddressComesFromLinkRegister) { + std::vector buf; + appendCie(buf); + appendFde(buf, 0, 64); + appendTerminator(buf); + DwarfParser* dwarf = parseBuf(buf); + ASSERT_EQ(2, dwarf->count()); + EXPECT_EQ(DW_LINK_REGISTER, dwarf->table()[0].pc_off); + free(dwarf->table()); + delete dwarf; +} + +// Model os::javaTimeNanos: stp fp/lr,[sp,#-32]! saves LR at CFA-24; +// ldp fp/lr,[sp],#32 restores the live registers before the final return. +static void checkRestoredArmLinkRegister(uint8_t restore_op) { + std::vector buf; + appendCie(buf); + size_t fde_start = buf.size(); + appendFde(buf, 0, 64); + std::vector instructions = {0x41, 0x0e, 32, 0x9d, 4, 0x9e, 3, 0x42, restore_op}; + if (restore_op != 0xde) instructions.push_back(30); + instructions.insert(instructions.end(), {0xdd, 0x0e, 0}); + buf.insert(buf.end(), instructions.begin(), instructions.end()); + uint32_t length = 13 + instructions.size(); + std::memcpy(buf.data() + fde_start, &length, sizeof(length)); + appendTerminator(buf); + DwarfParser* dwarf = parseBuf(buf); + ASSERT_EQ(4, dwarf->count()); + EXPECT_EQ(-24, dwarf->table()[1].pc_off); + EXPECT_EQ(DW_LINK_REGISTER, dwarf->table()[2].pc_off); + EXPECT_EQ(DW_SAME_FP, dwarf->table()[2].fp_off); + free(dwarf->table()); + delete dwarf; +} + +TEST(DwarfEhFrame, ArmRestoreReturnsToLiveLinkRegister) { + checkRestoredArmLinkRegister(0xde); // DW_CFA_restore r30 +} + +TEST(DwarfEhFrame, ArmExtendedRestoreReturnsToLiveLinkRegister) { + checkRestoredArmLinkRegister(0x06); // DW_CFA_restore_extended r30 +} + +TEST(DwarfEhFrame, ArmSameValueUsesLiveLinkRegister) { + checkRestoredArmLinkRegister(0x08); // DW_CFA_same_value r30 +} + +#endif // __aarch64__ + #endif // DWARF_SUPPORTED diff --git a/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp b/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp index 72c7dd1a3c..2369a22977 100644 --- a/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp +++ b/ddprof-lib/src/test/cpp/returnAddressAttribution_ut.cpp @@ -1206,6 +1206,7 @@ asm( "prof_ra_pcoff_seed:\n" ".cfi_startproc\n" ".cfi_def_cfa sp, 16\n" + ".cfi_offset 30, 0\n" "nop\nnop\n" ".cfi_endproc\n" ".size prof_ra_pcoff_seed, .-prof_ra_pcoff_seed\n" @@ -1348,24 +1349,10 @@ TEST_F(ReturnAddressAttributionTest, Test5b_DwPcOffsetAppliesOffsetToRawPc) { // =========================================================================== // Test 6 -- Link-register recovery sets the flag (coverage row 7). // -// Reachability, so nobody reads a green run here as coverage: -// - x86_64: structurally unreachable. EMPTY_FRAME_SIZE > 0 makes the -// guard "EMPTY_FRAME_SIZE > 0 || pc_off != DW_LINK_REGISTER" always true, -// so the arm never executes. -// - aarch64: reachable only with a frame table that actually carries -// pc_off == DW_LINK_REGISTER. DwarfParser never emits it -- it starts -// pc_off at -EMPTY_FRAME_SIZE and no DW_CFA_ handler assigns that value -- -// so no .eh_frame fixture, including the one below, can reach the arm. -// The only producer in the tree is SFrameParser (sframe.cpp, gated by -// SFrameParser.PerFRE_RA_Untracked in sframe_ut.cpp), which needs a -// .sframe section in the scanned ELF image; the gtest binary is not built -// with one, and CodeCache::setDwarfTable() cannot be used to inject one -// because it asserts the cache is unpublished. -// -// The fixture is kept so that it starts gating on its own if a .sframe- -// enabled build ever appears, and so the precondition check documents which -// row shape the arm needs. It deliberately does not increment -// g_gating_assertions_reached on the skip path. +// AArch64 leaf .eh_frame rows keep the return address in LR until it is +// spilled. This fixture reaches that rule through the real ELF parser and +// walker; the stack slot must not replace the live LR. +// x86_64 returns through a stack slot and skips this architecture-specific case. // =========================================================================== #if defined(__aarch64__) @@ -1401,13 +1388,8 @@ TEST_F(ReturnAddressAttributionTest, Test6_LinkRegisterRecoverySetsFlag) { GTEST_SKIP() << "prof_ra_lr_fn not visible to findLibraryByAddress in this build"; } FrameDesc row = lib->findFrameDesc((const void*)&prof_ra_lr_fn); - if (row.pc_off != DW_LINK_REGISTER || (row.fp_off & DW_PC_OFFSET)) { - GTEST_SKIP() << "expected: .eh_frame cannot express pc_off == DW_LINK_REGISTER, so this " - << "fixture's row parsed as (pc_off=" << row.pc_off << ", fp_off=" - << row.fp_off << ") and the link-register arm is not reached. The arm is " - << "covered by inspection only; its producer is gated by " - << "SFrameParser.PerFRE_RA_Untracked. See this test's banner comment."; - } + ASSERT_EQ(DW_LINK_REGISTER, row.pc_off); + ASSERT_EQ(0, row.fp_off & DW_PC_OFFSET); // Adjacency is not a precondition here: unlike Tests 1/3/4/5, the link // register below is set directly rather than derived from a real // call-site return address, so the fixture works wherever the assembler @@ -1438,6 +1420,85 @@ TEST_F(ReturnAddressAttributionTest, Test6_LinkRegisterRecoverySetsFlag) { #endif // __aarch64__ } +#if defined(__aarch64__) + +asm( + ".text\n" + ".globl prof_ra_restored_lr_fn\n" + ".type prof_ra_restored_lr_fn,%function\n" +"prof_ra_restored_lr_fn:\n" + ".cfi_startproc\n" + "stp x29, x30, [sp, #-32]!\n" + ".cfi_def_cfa_offset 32\n" + ".cfi_offset 29, -32\n" + ".cfi_offset 30, -24\n" + "ldp x29, x30, [sp], #32\n" + ".cfi_restore 29\n" + ".cfi_restore 30\n" + ".cfi_def_cfa_offset 0\n" + ".globl prof_ra_restored_lr_pc\n" +"prof_ra_restored_lr_pc:\n" + "nop\n" + "ret\n" + ".cfi_endproc\n" + ".size prof_ra_restored_lr_fn, .-prof_ra_restored_lr_fn\n" +); + +extern "C" void prof_ra_restored_lr_pc(void); + +TEST_F(ReturnAddressAttributionTest, RestoredLinkRegisterIgnoresStaleStackSlot) { + static uintptr_t scratch[64]; + ucontext_t uc = makeFabricatedContext((const void*)&prof_ra_restored_lr_pc, scratch, 64); + uc.uc_mcontext.regs[30] = (unsigned long)&prof_ra_lr_next; + // A signal can overwrite the callee's old LR slot after SP is restored. + // Populate it with a plausible code address to expose a phantom caller. + ((uintptr_t*)uc.uc_mcontext.sp)[-3] = (uintptr_t)&prof_ra_lr_fn + 1; + StackContext ctx{}; + bool truncated = false; + const void* chain[9]; + int depth = StackWalker::walkDwarf(&uc, chain, 8, &ctx, &truncated); + ASSERT_GE(depth, 2); + EXPECT_EQ((const void*)&prof_ra_restored_lr_pc, chain[0]); + EXPECT_EQ((const char*)&prof_ra_lr_next - 1, chain[1]); +} + +asm( + ".text\n" + ".globl prof_ra_sigreturn_without_signal_cfi\n" + ".type prof_ra_sigreturn_without_signal_cfi,%function\n" +"prof_ra_sigreturn_without_signal_cfi:\n" + ".cfi_startproc\n" + ".cfi_def_cfa x29, 0\n" + ".cfi_offset 29, 0\n" + ".cfi_offset 30, 8\n" + "mov x8, #139\n" + "svc #0\n" + ".cfi_endproc\n" + ".size prof_ra_sigreturn_without_signal_cfi, .-prof_ra_sigreturn_without_signal_cfi\n" +); + +extern "C" void prof_ra_sigreturn_without_signal_cfi(void); + +TEST_F(ReturnAddressAttributionTest, UnsupportedSigreturnDoesNotFollowInterruptedFp) { + static uintptr_t scratch[64]; + ucontext_t uc = makeFabricatedContext((const void*)&prof_ra_sigreturn_without_signal_cfi, scratch, 64); + // Model the FP-based fallback with a plausible but unrelated saved PC. + ((uintptr_t*)uc.uc_mcontext.regs[29])[0] = uc.uc_mcontext.regs[29] + 16; + ((uintptr_t*)uc.uc_mcontext.regs[29])[1] = (uintptr_t)&prof_ra_lr_next; + StackContext ctx{}; + bool truncated = false; + const void* chain[9]; + EXPECT_EQ(1, StackWalker::walkDwarf(&uc, chain, 8, &ctx, &truncated)); + EXPECT_TRUE(truncated); + // A sample may also land on the syscall instruction itself. + uc.uc_mcontext.pc += sizeof(instruction_t); + truncated = false; + EXPECT_EQ(1, StackWalker::walkDwarf(&uc, chain, 8, &ctx, &truncated)); + EXPECT_TRUE(truncated); +} + +#endif // __aarch64__ + // =========================================================================== // Test 8 -- A signal-frame row suppresses the attribution adjustment // (DWARF CIE augmentation 'S', emitted by .cfi_signal_frame). @@ -1472,7 +1533,9 @@ asm( ".cfi_startproc\n" ".cfi_signal_frame\n" ".cfi_def_cfa sp, 16\n" - "nop\nnop\n" + ".cfi_offset 30, 0\n" + "mov x8, #139\n" + "svc #0\n" ".cfi_endproc\n" ".size prof_ra_sig_seed, .-prof_ra_sig_seed\n" ); diff --git a/ddprof-lib/src/test/cpp/stackFrame_aarch64_ut.cpp b/ddprof-lib/src/test/cpp/stackFrame_aarch64_ut.cpp new file mode 100644 index 0000000000..d68396cad4 --- /dev/null +++ b/ddprof-lib/src/test/cpp/stackFrame_aarch64_ut.cpp @@ -0,0 +1,110 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "stackFrame.h" + +#ifdef __aarch64__ + +// Assembler-verified AArch64 sequences from C1's subtype-check callers and +// MacroAssembler::call_VM_leaf_base. The pending pop belongs to the caller's +// temporary arguments, outside the nmethod's fixed frame size. +TEST(StackFrameArgumentsTest, C1SubtypeCheckDiscardsCallerArguments) { + instruction_t code[] = {0xa9bf07e0, 0x94000000, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 2, sp); + EXPECT_EQ(0x1010u, sp); +} + +TEST(StackFrameArgumentsTest, C1PairResultDiscardsCallerArguments) { + instruction_t code[] = {0xa9bf07e0, 0x94000000, 0xa8c103e1}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 2, sp); + EXPECT_EQ(0x1010u, sp); +} + +TEST(StackFrameArgumentsTest, C1FarCallDiscardsCallerArguments) { + instruction_t code[] = {0xa9bf07e0, 0x90000008, 0x91000108, 0xd63f0100, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 4, sp); + EXPECT_EQ(0x1010u, sp); +} + +TEST(StackFrameArgumentsTest, VmLeafCallDiscardsTemporaryRegisters) { + instruction_t code[] = {0xa9bf33e8, 0xd2824688, 0xf2aacf08, 0xf2c24688, 0xd63f0100, 0xa8c133e8}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 5, sp); + EXPECT_EQ(0x1010u, sp); +} + +TEST(StackFrameArgumentsTest, DoesNotAdjustAfterThePop) { + instruction_t code[] = {0xa9bf07e0, 0x94000000, 0xf84107e0, 0xd503201f}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 3, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, DoesNotTreatTheFixedFrameAsArguments) { + instruction_t code[] = {0xa9bf7bfd, 0x94000000, 0xa8c17bfd}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 2, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, RequiresAMatchingArgumentPush) { + instruction_t code[] = {0xd503201f, 0x94000000, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 2, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, DoesNotSearchBeforeTheMethodEntry) { + instruction_t code[] = {0xa9bf07e0, 0x94000000, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code + 1, code + 2, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, RejectsUnrelatedAddressSetup) { + instruction_t code[] = {0xa9bf07e0, 0xd503201f, 0xd63f0100, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 3, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, RejectsSetupForADifferentCallRegister) { + instruction_t code[] = {0xa9bf07e0, 0x90000009, 0xd63f0100, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 3, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, RejectsADifferentPushSize) { + instruction_t code[] = {0xa9be07e0, 0x94000000, 0xf84107e0}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 2, sp); + EXPECT_EQ(0x1000u, sp); +} + +TEST(StackFrameArgumentsTest, PreservesC2NativeLeafAdjustment) { + instruction_t code[] = {0xa9bf27ff, 0xd63f0100, 0x910043ff}; + StackFrame frame(nullptr); + uintptr_t sp = 0x1000; + frame.adjustSP(code, code + 2, sp); + EXPECT_EQ(0x1010u, sp); +} + +#endif // __aarch64__