From e3bf2d379a98dd227359f4da8e3c893cd4c67e0d Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Mon, 19 Oct 2026 23:43:19 +0200 Subject: [PATCH 1/4] fix(profiler): mark skipped frozen virtual-thread callers as truncated --- .../src/main/cpp/hotspot/hotspotSupport.cpp | 7 +- .../FrozenVirtualThreadStackTest.java | 153 ++++++++++++++++++ 2 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/FrozenVirtualThreadStackTest.java diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index a811682936..f2cbce8730 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -388,7 +388,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 — VT frames are complete, carrier internals are noise. + // synthetic root frame; frozen VT callers are not reconstructed. // 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,6 +404,11 @@ __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 (depth >= actual_max_depth) return false; if (!features.carrier_frames) { fillFrame(frames[depth++], BCI_NATIVE_FRAME, CONT_ROOT_FRAME); 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 new file mode 100644 index 0000000000..e4337e2b46 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/FrozenVirtualThreadStackTest.java @@ -0,0 +1,153 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.wallclock; + +import com.datadoghq.profiler.CStackAwareAbstractProfilerTest; +import com.datadoghq.profiler.JfrFrame; +import com.datadoghq.profiler.JfrStackTrace; +import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.junit.CStack; +import java.util.concurrent.FutureTask; +import java.util.concurrent.locks.LockSupport; +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; + +/** Checks stack completeness 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; + + /** Selects the VM walker with or without carrier frames. */ + public FrozenVirtualThreadStackTest(@CStack String cstack) { + super(cstack); + } + + @Override + protected boolean isPlatformSupported() { + // CStackInjector falls back to the first mode when every mode is excluded. + // Keep that fallback from re-enabling VM walking on musl x64 in CI. + return Platform.isJavaVersionAtLeast(21) && !Platform.isJ9() && !Platform.isZing() + && !(isInCI() && Platform.isMusl() && !Platform.isAarch64()); + } + + @Override + protected String getProfilerCommand() { + return "wall=1ms,walltpt=64,filter=,jstackdepth=256"; + } + + /** Missing logical callers must be marked truncated in both VM walking modes. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void marksSkippedFrozenCallersTruncated(@CStack String cstack) throws Exception { + runWorkload(true, true); + assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_cont_barrier_hit", 0L) > 0, + "The workload must encounter a continuation return barrier"); + + int[] counts = reduceEvents("datadog.MethodSample", () -> new int[2], (count, sample) -> { + JfrStackTrace trace = sample.getStackTrace(); + if (containsMethod(trace, "recursiveWorkload") && !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"); + } + + /** The same parked workload on a platform thread keeps its application callers on the stack. */ + @TestTemplate + @ValueSource(strings = {"vm", "vmx"}) + public void keepsPlatformCallersComplete(@CStack String cstack) throws Exception { + runWorkload(false, true); + assertCompleteCallers(); + } + + /** Ending at a fully thawed continuation is a complete logical stack in the ordinary VM mode. */ + @TestTemplate + @ValueSource(strings = {"vm"}) + public void keepsFullyThawedContinuationComplete(@CStack String cstack) throws Exception { + runWorkload(true, false); + assertCompleteCallers(); + } + + /** Runs the same recursion on either kind of thread and verifies that its callers really exist. */ + private void runWorkload(boolean virtual, boolean park) throws Exception { + waitForProfilerReady(2_000); + FutureTask task = new FutureTask<>(() -> { + outerWorkload(park); + return null; + }); + Thread worker; + if (virtual) { + // Reflection keeps the test compatible with the Java 8 compilation target. + worker = (Thread) Thread.class.getMethod("startVirtualThread", Runnable.class) + .invoke(null, task); + } else { + worker = new Thread(task, "platform-stack-control"); + worker.start(); + } + worker.join(10_000); + assertFalse(worker.isAlive(), "The stack workload did not finish"); + task.get(); + stopProfiler(); + + int recursion = 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("outerWorkload")) outer = true; + } + assertEquals(RECURSION_DEPTH + 1, recursion, "The JVM must preserve all recursive callers"); + assertTrue(outer, "The outer caller must be present in the JVM logical stack"); + } + + /** Checks the negative controls without letting a run with no sampled callers pass. */ + private void assertCompleteCallers() { + int[] counts = reduceEvents("datadog.MethodSample", () -> new int[2], (count, sample) -> { + JfrStackTrace trace = sample.getStackTrace(); + if (containsMethod(trace, "recursiveWorkload") && containsMethod(trace, "outerWorkload")) { + count[0]++; + if (trace.isTruncated()) count[1]++; + } + }); + assertTrue(counts[0] > 0, "No sample retained the outer application caller"); + assertEquals(0, counts[1], "Fully captured control stacks were marked truncated"); + } + + private boolean containsMethod(JfrStackTrace trace, String method) { + for (JfrFrame frame : trace.frames()) { + if (getClass().getName().equals(frame.className()) && method.equals(frame.methodName())) { + return true; + } + } + return false; + } + + private void outerWorkload(boolean park) { + long deadline = System.nanoTime() + 2_000_000_000L; + while (System.nanoTime() < deadline) recursiveWorkload(RECURSION_DEPTH, park); + } + + /** Work after the recursive call keeps callers alive; deep recursion exceeds partial-thaw sizes. */ + private void recursiveWorkload(int depth, boolean park) { + if (depth > 0) { + recursiveWorkload(depth - 1, park); + } else { + 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(); + } + sink += depth; + } +} From 235f1e4924f9d4a5b1c7f23d5228a810623d8e8d Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 22 Oct 2026 15:23:13 +0200 Subject: [PATCH 2/4] test: fix CI failures --- .../profiler/wallclock/FrozenVirtualThreadStackTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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..cd4a68b0f8 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 @@ -47,7 +47,7 @@ protected String getProfilerCommand() { /** Missing logical callers must be marked truncated in both VM walking modes. */ @TestTemplate @ValueSource(strings = {"vm", "vmx"}) - public void marksSkippedFrozenCallersTruncated(@CStack String cstack) throws Exception { + public void marksSkippedFrozenCallersTruncated() throws Exception { runWorkload(true, true); assertTrue(profiler.getDebugCounters().getOrDefault("walkvm_cont_barrier_hit", 0L) > 0, "The workload must encounter a continuation return barrier"); @@ -66,7 +66,7 @@ public void marksSkippedFrozenCallersTruncated(@CStack String cstack) throws Exc /** The same parked workload on a platform thread keeps its application callers on the stack. */ @TestTemplate @ValueSource(strings = {"vm", "vmx"}) - public void keepsPlatformCallersComplete(@CStack String cstack) throws Exception { + public void keepsPlatformCallersComplete() throws Exception { runWorkload(false, true); assertCompleteCallers(); } @@ -74,7 +74,7 @@ public void keepsPlatformCallersComplete(@CStack String cstack) throws Exception /** Ending at a fully thawed continuation is a complete logical stack in the ordinary VM mode. */ @TestTemplate @ValueSource(strings = {"vm"}) - public void keepsFullyThawedContinuationComplete(@CStack String cstack) throws Exception { + public void keepsFullyThawedContinuationComplete() throws Exception { runWorkload(true, false); assertCompleteCallers(); } From aa66961dad6e2720b5b0201fc7ff27083ebbf94d Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 14:22:17 +0200 Subject: [PATCH 3/4] sphinx: address review feedback on PR #877 --- .../src/main/cpp/hotspot/contBoundary.h | 41 +++++++++++ .../src/main/cpp/hotspot/hotspotSupport.cpp | 70 ++++++++++-------- ddprof-lib/src/test/cpp/contBoundary_ut.cpp | 72 +++++++++++++++++++ 3 files changed, 155 insertions(+), 28 deletions(-) create mode 100644 ddprof-lib/src/main/cpp/hotspot/contBoundary.h create mode 100644 ddprof-lib/src/test/cpp/contBoundary_ut.cpp diff --git a/ddprof-lib/src/main/cpp/hotspot/contBoundary.h b/ddprof-lib/src/main/cpp/hotspot/contBoundary.h new file mode 100644 index 0000000000..5bde3c4242 --- /dev/null +++ b/ddprof-lib/src/main/cpp/hotspot/contBoundary.h @@ -0,0 +1,41 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef _HOTSPOT_CONTBOUNDARY_H +#define _HOTSPOT_CONTBOUNDARY_H + +// Classification of a virtual-thread continuation boundary crossed by walkVM. +// Two independent facts are derived from it: +// - completeness: whether the crossing proves no older Java callers remain +// frozen in a heap StackChunk (only Thawed does; everything else marks +// the sample truncated); +// - layout (JDK 21-26, no ContinuationEntry in vmStructs): whether the +// enterSpecial frame FP is the current fp (Thawed, Unverified) or the +// saved caller FP at *fp (Frozen). +enum class ContBoundary : unsigned char { + Thawed, // verified: bottom VT frame returns to cont_entry_return_pc / enterSpecial + Frozen, // verified: bottom VT frame returns to cont_returnBarrier + Unverified, // speculative: unknown CodeHeap PC on a carrier; state unknown +}; + +// Only a verified fully-thawed boundary proves completeness. Any other value, +// including future enumerators, defaults to "not proven". +constexpr bool contBoundaryProvesComplete(ContBoundary kind) { + return kind == ContBoundary::Thawed; +} + +// JDK 21-26 entry_fp derivation: fp itself, unless verified frozen (then *fp). +constexpr bool contBoundaryEntryFpIsCurrentFp(ContBoundary kind) { + return kind != ContBoundary::Frozen; +} + +// Per-call-site classification used by walkVM. Call sites must pass one of +// these, never a bare enumerator. +inline constexpr ContBoundary CONT_BOUNDARY_AT_RETURN_BARRIER = ContBoundary::Frozen; +inline constexpr ContBoundary CONT_BOUNDARY_AT_ENTRY_RETURN_PC = ContBoundary::Thawed; +inline constexpr ContBoundary CONT_BOUNDARY_AT_ENTER_SPECIAL_NM = ContBoundary::Thawed; +inline constexpr ContBoundary CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM = ContBoundary::Unverified; + +#endif // _HOTSPOT_CONTBOUNDARY_H diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 26e3d55aca..b2970d92b9 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -11,6 +11,7 @@ #include "faultInjection.h" #include "frames.h" #include "guards.h" +#include "hotspot/contBoundary.h" #include "hotspot/hotspotSupport.h" #include "hotspot/jitCodeCache.h" #include "hotspot/klassTraceId.h" @@ -424,18 +425,23 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // (_parent not triggered by standard single-level VTs today, but required // once any runtime layers continuations on top of VTs). // - // all_frames_thawed: true when the bottom VT frame's return PC is - // cont_entry_return_pc (all VT frames are thawed — CPU-bound VT), - // false when it is cont_returnBarrier (frozen frames remain in the - // StackChunk — VT parked and just remounted). - // Needed to derive entry_fp on JDK 21-26 where ContinuationEntry - // type size is absent from vmStructs and contEntry() returns nullptr. + // kind: how the caller classified the boundary (see hotspot/contBoundary.h). + // Completeness: unless kind proves completeness (verified Thawed), older + // Java callers may remain frozen in a heap StackChunk and be skipped by + // this walker, so the sample is marked truncated. That covers Frozen + // (return barrier) and Unverified (speculative unknown-nmethod crossing). + // Layout: on JDK 21-26, where ContinuationEntry type size is absent from + // vmStructs and contEntry() returns nullptr, entry_fp is fp for + // Thawed/Unverified and *fp for Frozen. For Unverified this is a + // best-effort thawed guess; if it is wrong, the fp/sp validation below + // emits BCI_ERROR. // // 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) { + auto walkThroughContinuation = [&](ContBoundary kind) -> bool { + // Unless the boundary is verified fully thawed, older Java callers may + // remain in heap StackChunks, which this walker skips even when carrier + // walking succeeds. + if (!contBoundaryProvesComplete(kind) && truncated != nullptr) { *truncated = true; } if (depth >= actual_max_depth) return false; @@ -461,13 +467,13 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex } else { // ContinuationEntry absent from vmStructs (JDK 21-26). // Derive the enterSpecial frame FP from the current fp: - // all frames thawed (pc == cont_entry_return_pc): fp IS the - // enterSpecial frame FP. - // frozen frames remain (pc == cont_returnBarrier): the saved - // caller FP at *fp leads to the enterSpecial frame on the - // carrier stack. + // thawed or unverified (pc == cont_entry_return_pc, or a + // speculative crossing): fp is taken as the enterSpecial + // frame FP. + // frozen (pc == cont_returnBarrier): the saved caller FP at *fp + // leads to the enterSpecial frame on the carrier stack. // Nested continuation tracking is unavailable without type_size(). - entry_fp = all_frames_thawed ? fp : (uintptr_t)SafeAccess::load((void**)fp); + entry_fp = contBoundaryEntryFpIsCurrentFp(kind) ? fp : (uintptr_t)SafeAccess::load((void**)fp); } if (!StackWalkValidation::isValidFP(entry_fp)) { @@ -524,21 +530,21 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex break; } have_prev_native_pc = false; // we are in JVM code, no previous 'native' PC - // Both continuation boundary PCs are JVM stubs whose findNMethod() - // returns NULL; detect them by exact-PC match before the nmethod - // dispatch below. + // Detect both continuation boundary PCs by exact-PC match before the + // nmethod dispatch below (findNMethod would otherwise classify them + // as ordinary stub/nmethod blobs). // cont_returnBarrier: bottom thawed frame returns here when frozen // frames remain in the StackChunk (blocking/remounted VT). // cont_entry_return_pc: bottom thawed frame returns here when the // continuation is fully thawed (CPU-bound VT, never yielded). if (!CONT_UNWIND_DISABLED && VMStructs::isContReturnBarrier(walk_pc.raw())) { Counters::increment(WALKVM_CONT_BARRIER_HIT); - if (walkThroughContinuation(false)) continue; + if (walkThroughContinuation(CONT_BOUNDARY_AT_RETURN_BARRIER)) continue; break; } if (!CONT_UNWIND_DISABLED && VMStructs::isContEntryReturnPc(walk_pc.raw())) { Counters::increment(WALKVM_ENTER_SPECIAL_HIT); - if (walkThroughContinuation(true)) continue; + if (walkThroughContinuation(CONT_BOUNDARY_AT_ENTRY_RETURN_PC)) continue; break; } VMNMethod* nm = CodeHeap::findNMethod(walk_pc.raw()); @@ -546,18 +552,26 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // On JDK 21+ builds, the continuation entry PC may be absent // from vmStructs OR resolved but pointing to the wrong address // (some distributions expose the symbol at the wrong address, so - // the exact-PC check above never fires). Attempt a fully-thawed - // continuation walk whenever we see an unknown nmethod after - // collecting Java frames. walkThroughContinuation validates the - // fp chain and emits BCI_ERROR cleanly on mismatch, so false - // positives are safe. + // the exact-PC check above never fires). Attempt a continuation + // walk whenever we see an unknown nmethod after collecting Java + // frames. The crossing is UNVERIFIED: nothing here proves + // whether frozen frames remain in the StackChunk. The walk uses + // the thawed layout guess (entry_fp = fp); the lambda's fp/sp + // validation emits BCI_ERROR on mismatch. The sample is always + // marked truncated, because only a verified fully-thawed + // boundary proves completeness (a possible false positive is + // preferred over silently dropping frozen callers). + // Known limitation: for a frozen continuation reached here on + // JDK 21-26 the layout guess may be off by one frame. if (!CONT_UNWIND_DISABLED && features.carrier_frames && VM::hotspot_version() >= 21 && depth > 0 && vm_thread != NULL && vm_thread->isCarryingVirtualThread()) { Counters::increment(WALKVM_CONT_SPECULATIVE_HIT); - if (walkThroughContinuation(true)) continue; + static_assert(!contBoundaryProvesComplete(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM), + "speculative continuation crossing must mark the sample truncated"); + if (walkThroughContinuation(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM)) continue; break; } if (anchor == NULL) { @@ -654,7 +668,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // and navigate to the carrier thread via ContinuationEntry. if (!CONT_UNWIND_DISABLED && nm == VMStructs::enterSpecialNMethod()) { Counters::increment(WALKVM_ENTER_SPECIAL_HIT); - if (walkThroughContinuation(true)) continue; + if (walkThroughContinuation(CONT_BOUNDARY_AT_ENTER_SPECIAL_NM)) continue; break; } // Check if deoptimization is in progress before walking compiled frames diff --git a/ddprof-lib/src/test/cpp/contBoundary_ut.cpp b/ddprof-lib/src/test/cpp/contBoundary_ut.cpp new file mode 100644 index 0000000000..93315a2172 --- /dev/null +++ b/ddprof-lib/src/test/cpp/contBoundary_ut.cpp @@ -0,0 +1,72 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +// Included first: the TU fails to compile if the header is not self-contained. +#include "hotspot/contBoundary.h" + +#include + +// Compile-time checks: the predicates and per-site constants are constexpr. +static_assert(!contBoundaryProvesComplete(ContBoundary::Frozen), "Frozen must not prove completeness"); +static_assert(!contBoundaryEntryFpIsCurrentFp(ContBoundary::Frozen), "Frozen uses *fp layout"); +static_assert(contBoundaryProvesComplete(ContBoundary::Thawed), "Thawed proves completeness"); +static_assert(contBoundaryEntryFpIsCurrentFp(ContBoundary::Thawed), "Thawed uses fp layout"); +static_assert(!contBoundaryProvesComplete(ContBoundary::Unverified), "Unverified must not prove completeness"); +static_assert(contBoundaryEntryFpIsCurrentFp(ContBoundary::Unverified), "Unverified keeps the thawed fp layout guess"); +static_assert(!contBoundaryProvesComplete(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM), + "speculative crossing must mark the sample truncated"); +static_assert(contBoundaryEntryFpIsCurrentFp(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM), + "speculative crossing keeps the thawed fp layout guess"); +static_assert(!contBoundaryProvesComplete(CONT_BOUNDARY_AT_RETURN_BARRIER), + "return barrier crossing must mark the sample truncated"); +static_assert(!contBoundaryEntryFpIsCurrentFp(CONT_BOUNDARY_AT_RETURN_BARRIER), + "return barrier crossing uses *fp layout"); + +// R4: return barrier (frozen frames remain) marks truncated and uses *fp. +TEST(ContBoundary, FrozenMarksTruncatedAndUsesSavedFp) { + EXPECT_FALSE(contBoundaryProvesComplete(ContBoundary::Frozen)); + EXPECT_FALSE(contBoundaryEntryFpIsCurrentFp(ContBoundary::Frozen)); +} + +// R5/R6: verified fully-thawed boundary is complete and uses fp. +TEST(ContBoundary, ThawedIsCompleteAndUsesCurrentFp) { + EXPECT_TRUE(contBoundaryProvesComplete(ContBoundary::Thawed)); + EXPECT_TRUE(contBoundaryEntryFpIsCurrentFp(ContBoundary::Thawed)); +} + +// R7/R10: speculative (unverified) crossing marks truncated but keeps the +// thawed layout guess. Pre-fix semantics treated it as "thawed => complete". +TEST(ContBoundary, UnverifiedMarksTruncatedButKeepsThawedLayout) { + EXPECT_FALSE(contBoundaryProvesComplete(ContBoundary::Unverified)); + EXPECT_TRUE(contBoundaryEntryFpIsCurrentFp(ContBoundary::Unverified)); +} + +// R4-R7: per-call-site constants classify each walkVM boundary. +TEST(ContBoundary, CallSiteConstantsClassifyEachBoundary) { + EXPECT_EQ(CONT_BOUNDARY_AT_RETURN_BARRIER, ContBoundary::Frozen); + EXPECT_EQ(CONT_BOUNDARY_AT_ENTRY_RETURN_PC, ContBoundary::Thawed); + EXPECT_EQ(CONT_BOUNDARY_AT_ENTER_SPECIAL_NM, ContBoundary::Thawed); + EXPECT_EQ(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM, ContBoundary::Unverified); + + EXPECT_FALSE(contBoundaryProvesComplete(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM)); + EXPECT_TRUE(contBoundaryEntryFpIsCurrentFp(CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM)); + EXPECT_FALSE(contBoundaryProvesComplete(CONT_BOUNDARY_AT_RETURN_BARRIER)); + EXPECT_FALSE(contBoundaryEntryFpIsCurrentFp(CONT_BOUNDARY_AT_RETURN_BARRIER)); + EXPECT_TRUE(contBoundaryProvesComplete(CONT_BOUNDARY_AT_ENTRY_RETURN_PC)); + EXPECT_TRUE(contBoundaryEntryFpIsCurrentFp(CONT_BOUNDARY_AT_ENTRY_RETURN_PC)); + EXPECT_TRUE(contBoundaryProvesComplete(CONT_BOUNDARY_AT_ENTER_SPECIAL_NM)); + EXPECT_TRUE(contBoundaryEntryFpIsCurrentFp(CONT_BOUNDARY_AT_ENTER_SPECIAL_NM)); +} + +// R9: completeness is an allow-list — only Thawed proves it; any other value, +// including an out-of-range one, defaults to "not proven" (truncated). +TEST(ContBoundary, OnlyThawedProvesCompleteness) { + const ContBoundary all[] = {ContBoundary::Thawed, ContBoundary::Frozen, ContBoundary::Unverified}; + for (ContBoundary k : all) { + EXPECT_EQ(contBoundaryProvesComplete(k), k == ContBoundary::Thawed); + } + ContBoundary unknown = static_cast(0xFE); + EXPECT_FALSE(contBoundaryProvesComplete(unknown)); +} From 9d06c307b0369a21fd70c4444e9fab8b3ad98c61 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Fri, 23 Oct 2026 15:15:50 +0200 Subject: [PATCH 4/4] fix(profiler): preserve explicit truncation in JFR stacks Keep the walker truncation flag when a stack ends at a Java entry method. Exercise speculative continuation recovery and stack-pool header encoding with native regression tests, and rename the never-parked virtual-thread control. --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 25 +- ddprof-lib/src/main/cpp/flightRecorder.h | 6 + ddprof-lib/src/main/cpp/jvmThread.h | 1 + ddprof-lib/src/main/cpp/threadLocal.h | 1 + .../src/test/cpp/continuationWalk_ut.cpp | 252 ++++++++++++++++++ .../test/cpp/stackTraceSerialization_ut.cpp | 63 +++++ .../FrozenVirtualThreadStackTest.java | 4 +- 7 files changed, 340 insertions(+), 12 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/continuationWalk_ut.cpp create mode 100644 ddprof-lib/src/test/cpp/stackTraceSerialization_ut.cpp diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 64c158df8e..6a1964b740 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -1922,6 +1922,17 @@ void Recording::writeThreads(Buffer *buf) { } } +void Recording::writeStackTraceHeader(Buffer *buf, const CallTrace *trace, + const MethodInfo *root) { + buf->putVar64(trace->trace_id); + if (root != nullptr) { + bool truncated = trace->truncated || + (root->_type < FRAME_NATIVE && !root->_is_entry); + buf->put8(truncated); + } + buf->putVar64(trace->num_frames); +} + int Recording::writeStackTraces(Buffer *buf, Lookup *lookup) { // Reset all referenced flags before processing for (MethodMap::iterator it = _method_map.begin(); it != _method_map.end(); ++it) { @@ -1942,18 +1953,12 @@ int Recording::writeStackTraces(Buffer *buf, Lookup *lookup) { buf->putVar64(traces.size()); for (auto it = traces.begin(); it != traces.end(); ++it) { CallTrace *trace = *it; - buf->putVar64(trace->trace_id); + MethodInfo *root = nullptr; if (trace->num_frames > 0) { - MethodInfo *mi = - lookup->resolveMethod(trace->frames[trace->num_frames - 1]); - mi->_referenced = true; // Mark method as referenced - if (mi->_type < FRAME_NATIVE) { - buf->put8(mi->_is_entry ? 0 : 1); - } else { - buf->put8(trace->truncated); - } + root = lookup->resolveMethod(trace->frames[trace->num_frames - 1]); + root->_referenced = true; } - buf->putVar64(trace->num_frames); + writeStackTraceHeader(buf, trace, root); for (int i = 0; i < trace->num_frames; i++) { MethodInfo *mi = lookup->resolveMethod(trace->frames[i]); mi->_referenced = true; // Mark method as referenced diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index 5c827a2ced..c393472823 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -37,6 +37,7 @@ class VMSymbol; // hotspot/vmStructs.h class ProfiledThread; +struct CallTrace; const u64 MAX_JLONG = 0x7fffffffffffffffULL; const u64 MIN_JLONG = 0x8000000000000000ULL; @@ -280,6 +281,11 @@ class Recording { static size_t countSerializableChildren( const std::vector &children, int depth); + // Encodes the stack-pool record header. A Java entry root can establish + // completeness only when the walker has not already reported missing frames. + static void writeStackTraceHeader(Buffer *buf, const CallTrace *trace, + const MethodInfo *root); + RecordingBuffer _buf[CONCURRENCY_LEVEL]; // we have several tables to avoid lock contention // we have a second dimension to allow a switch in the active table diff --git a/ddprof-lib/src/main/cpp/jvmThread.h b/ddprof-lib/src/main/cpp/jvmThread.h index 2f5bd69104..f2c0358cc3 100644 --- a/ddprof-lib/src/main/cpp/jvmThread.h +++ b/ddprof-lib/src/main/cpp/jvmThread.h @@ -15,6 +15,7 @@ * JVMThread represents a native JVM thread that is JVM implementation agnostic */ class JVMThread { + friend class JVMThreadTestAccessor; private: static jfieldID _tid; static ThreadLocal _jvm_thread; diff --git a/ddprof-lib/src/main/cpp/threadLocal.h b/ddprof-lib/src/main/cpp/threadLocal.h index 764a80c685..cf704b4a74 100644 --- a/ddprof-lib/src/main/cpp/threadLocal.h +++ b/ddprof-lib/src/main/cpp/threadLocal.h @@ -187,6 +187,7 @@ class JVMThread; */ template <> class ThreadLocal { + friend class JVMThreadTestAccessor; protected: pthread_key_t _key; diff --git a/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp b/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp new file mode 100644 index 0000000000..c676f27429 --- /dev/null +++ b/ddprof-lib/src/test/cpp/continuationWalk_ut.cpp @@ -0,0 +1,252 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include "callTraceHashTable.h" +#include "codeCache.h" +#include "counters.h" +#include "dwarf.h" +#include "flightRecorder.h" +#include "hotspot/hotspotSupport.h" +#include "hotspot/vmStructs.h" +#include "jvmThread.h" +#include "libraries.h" +#include "stackFrame.h" +#include "threadLocalData.inline.h" + +class JVMThreadTestAccessor { +public: + static jfieldID tid() { return JVMThread::_tid; } + static void install(pthread_key_t key, jfieldID tid) { + JVMThread::_tid = tid; + JVMThread::_jvm_thread._key = key; + } +}; + +class VMTestAccessor { +public: + static bool hotspot() { return VM::_hotspot; } + static int version() { return VM::_hotspot_version; } + static void install(bool hotspot, int version) { + VM::_hotspot = hotspot; + VM::_hotspot_version = version; + } +}; + +class HotspotSupportTestAccessor { +public: + static int walk(void* context, ASGCT_CallFrame* frames, StackWalkFeatures features, + int max_depth, bool* truncated) { + return HotspotSupport::walkVM(context, frames, max_depth, features, + EXECUTION_SAMPLE, 0, truncated); + } +}; + +class RecordingTestAccessor { +public: + static void writeHeader(Buffer* buffer, const CallTrace* trace, const MethodInfo* root) { + Recording::writeStackTraceHeader(buffer, trace, root); + } +}; + +namespace { + +const char* const CODE_BASE = (const char*)0x5a6c00000000ULL; +const char* const LEAF_PC = CODE_BASE + 0x108; +const char* const BOUNDARY_PC = CODE_BASE + 0x208; +const char* const CARRIER_PC = CODE_BASE + 0x308; +const char* const THREAD_ENTRY_PC = CODE_BASE + 0x408; + +bool isThreadEntry(const char* name) { return strcmp(name, "continuation_thread_entry") == 0; } + +// The code heap contains the boundary PC, but none of its heaps owns an nmethod +// there. Native frames before/after it use an explicit SP-based unwind row. +// This drives the real nm == NULL branch and successful carrier recovery without +// depending on a particular installed JVM's continuation metadata exports. +class ContinuationWalkTest : public ::testing::Test, protected VMStructs { +protected: + static constexpr int MAX_DEPTH = 8; + struct ThreadLayout { void* entry; } thread{}; + uintptr_t stack[32]{}; + ASGCT_CallFrame frames[MAX_DEPTH + 1]{}; + + static void SetUpTestSuite() { + static CodeCache library("continuation_walk", -1, CODE_BASE, CODE_BASE + 0x1000); + library.add(CODE_BASE + 0x100, 0x20, "continuation_sampled_leaf"); + library.add(CODE_BASE + 0x300, 0x20, "continuation_carrier"); + library.add(CODE_BASE + 0x400, 0x20, "continuation_thread_entry"); + library.mark(isThreadEntry, MARK_THREAD_ENTRY); + library.sort(); + static const FrameDesc row{0, DW_REG_SP | (2 * sizeof(uintptr_t) << 8), + -2 * (int)sizeof(uintptr_t), -(int)sizeof(uintptr_t), 0}; + library.setDwarfTable(nullptr, 0, row); + ASSERT_TRUE(Libraries::instance()->addLibraryForTest(&library)); + } + + void SetUp() override { + old_hotspot = VMTestAccessor::hotspot(); + old_version = VMTestAccessor::version(); + old_key = JVMThread::key(); + old_tid = JVMThreadTestAccessor::tid(); + old_low = _code_heap_low; + old_high = _code_heap_high; + old_barrier = _cont_return_barrier; + old_return_pc = _cont_entry_return_pc; + old_entry_offset = _cont_entry_offset; + old_entry_size = TYPE_SIZE_NAME(VMContinuationEntry); + old_thread_size = TYPE_SIZE_NAME(VMThread); + ASSERT_EQ(0, pthread_key_create(&owned_key, nullptr)); + ASSERT_EQ(0, pthread_setspecific(owned_key, &thread)); + 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. + ProfiledThread::current()->setJavaThread(false); + thread.entry = &stack[6]; + JVMThreadTestAccessor::install(owned_key, (jfieldID)1); + _cont_entry_offset = offsetof(ThreadLayout, entry); + TYPE_SIZE_NAME(VMThread) = sizeof(ThreadLayout); + TYPE_SIZE_NAME(VMContinuationEntry) = 0; + _code_heap_low = CODE_BASE + 0x200; + _code_heap_high = CODE_BASE + 0x220; + _cont_return_barrier = nullptr; + _cont_entry_return_pc = CODE_BASE + 0x210; // deliberately does not match + } + + void TearDown() override { + _code_heap_low = old_low; + _code_heap_high = old_high; + _cont_return_barrier = old_barrier; + _cont_entry_return_pc = old_return_pc; + _cont_entry_offset = old_entry_offset; + TYPE_SIZE_NAME(VMContinuationEntry) = old_entry_size; + TYPE_SIZE_NAME(VMThread) = old_thread_size; + JVMThreadTestAccessor::install(old_key, old_tid); + if (owned_key != INVALID_KEY) pthread_key_delete(owned_key); + VMTestAccessor::install(old_hotspot, old_version); + ProfiledThread::release(); + } + + int walk(bool carrier_frames, bool* truncated, bool frozen = false, + bool valid_carrier = true, int max_depth = MAX_DEPTH) { + // First native frame recovers a boundary PC plus the bottom VT FP. + stack[0] = (uintptr_t)&stack[8]; + stack[1] = (uintptr_t)BOUNDARY_PC; + int entry_fp = frozen ? 12 : 8; + if (frozen) stack[8] = (uintptr_t)&stack[12]; + stack[entry_fp] = valid_carrier ? (uintptr_t)&stack[24] : 0; + stack[entry_fp + FRAME_PC_SLOT] = (uintptr_t)CARRIER_PC; + int carrier_sp = entry_fp + FRAME_PC_SLOT + 1; + stack[carrier_sp] = (uintptr_t)&stack[26]; + stack[carrier_sp + 1] = (uintptr_t)THREAD_ENTRY_PC; + ucontext_t context{}; +#ifdef __APPLE__ + _STRUCT_MCONTEXT64 machine_context{}; + context.uc_mcontext = &machine_context; +#endif + StackFrame frame(&context); + frame.pc() = (uintptr_t)LEAF_PC; + frame.sp() = (uintptr_t)stack; + frame.fp() = (uintptr_t)&stack[8]; + StackWalkFeatures features{}; + features.carrier_frames = carrier_frames; + return HotspotSupportTestAccessor::walk(&context, frames, features, max_depth, truncated); + } + + void assertCarrier(int depth) { + ASSERT_EQ(2, depth); + EXPECT_EQ(BCI_NATIVE_FRAME, frames[0].bci); + EXPECT_EQ(BCI_NATIVE_FRAME, frames[1].bci); + EXPECT_STREQ("continuation_sampled_leaf", (const char*)frames[0].method_id); + EXPECT_STREQ("continuation_carrier", (const char*)frames[1].method_id); + } + + bool old_hotspot; + int old_version; + pthread_key_t old_key; + pthread_key_t owned_key = INVALID_KEY; + jfieldID old_tid; + const void *old_low, *old_high, *old_barrier, *old_return_pc; + int old_entry_offset; + uint64_t old_entry_size; + uint64_t old_thread_size; +}; + +TEST_F(ContinuationWalkTest, SpeculativeCrossingRetainsCarrierAndSerializesTruncation) { + bool truncated = false; + auto before = Counters::getCounter(WALKVM_CONT_SPECULATIVE_HIT); + int depth = walk(true, &truncated); + assertCarrier(depth); + EXPECT_EQ(before + 1, Counters::getCounter(WALKVM_CONT_SPECULATIVE_HIT)); + ASSERT_TRUE(truncated); + + // A successful carrier walk may ultimately end at a Java Thread.run root. + // Feed that resolved root and the actual walker's flag to the production + // stack-pool header encoder, not a second copy of its completeness rule. + CallTrace trace(truncated, depth, 42); + MethodInfo root; + root._type = FRAME_INTERPRETED; + root._is_entry = true; + Buffer buffer; + RecordingTestAccessor::writeHeader(&buffer, &trace, &root); + ASSERT_EQ(3, buffer.offset()); + EXPECT_EQ(42, (unsigned char)buffer.data()[0]); + EXPECT_EQ(1, (unsigned char)buffer.data()[1]); + EXPECT_EQ(depth, (unsigned char)buffer.data()[2]); +} + +TEST_F(ContinuationWalkTest, VerifiedThawedCrossingRetainsCarrierWithoutTruncation) { + _cont_entry_return_pc = BOUNDARY_PC; + bool truncated = false; + assertCarrier(walk(true, &truncated)); + EXPECT_FALSE(truncated); +} + +TEST_F(ContinuationWalkTest, VerifiedFrozenCrossingRetainsCarrierAndTruncates) { + _cont_return_barrier = BOUNDARY_PC; + bool truncated = false; + assertCarrier(walk(true, &truncated, true)); + EXPECT_TRUE(truncated); +} + +TEST_F(ContinuationWalkTest, ExportedEntryLayoutRetainsCarrierAndTruncates) { + TYPE_SIZE_NAME(VMContinuationEntry) = 2 * sizeof(uintptr_t); + bool truncated = false; + assertCarrier(walk(true, &truncated)); + EXPECT_TRUE(truncated); +} + +TEST_F(ContinuationWalkTest, FrozenBoundaryWithoutCarrierFramesTruncates) { + _cont_return_barrier = BOUNDARY_PC; + bool truncated = false; + ASSERT_EQ(2, walk(false, &truncated, true)); + EXPECT_STREQ("JVM Continuation", (const char*)frames[1].method_id); + EXPECT_TRUE(truncated); +} + +TEST_F(ContinuationWalkTest, VerifiedThawedBoundaryPreservesExistingTruncation) { + _cont_entry_return_pc = BOUNDARY_PC; + bool truncated = true; + assertCarrier(walk(true, &truncated)); + EXPECT_TRUE(truncated); +} + +TEST_F(ContinuationWalkTest, SpeculativeCarrierFailureIsTruncated) { + bool truncated = false; + ASSERT_EQ(2, walk(true, &truncated, false, false)); + EXPECT_EQ(BCI_ERROR, frames[1].bci); + EXPECT_STREQ("break_cont_carrier_sp", (const char*)frames[1].method_id); + EXPECT_TRUE(truncated); +} + +TEST_F(ContinuationWalkTest, SpeculativeCrossingAcceptsNullTruncationPointer) { + assertCarrier(walk(true, nullptr)); +} + +} // namespace diff --git a/ddprof-lib/src/test/cpp/stackTraceSerialization_ut.cpp b/ddprof-lib/src/test/cpp/stackTraceSerialization_ut.cpp new file mode 100644 index 0000000000..4dd0717093 --- /dev/null +++ b/ddprof-lib/src/test/cpp/stackTraceSerialization_ut.cpp @@ -0,0 +1,63 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "callTraceHashTable.h" +#include "flightRecorder.h" + +class RecordingTestAccessor { +public: + static void writeHeader(Buffer* buffer, const CallTrace* trace, const MethodInfo* root) { + Recording::writeStackTraceHeader(buffer, trace, root); + } +}; + +namespace { + +class StackTraceSerializationTest : public ::testing::Test { +protected: + void check(bool walker_truncated, FrameTypeId root_type, bool entry, bool expected) { + CallTrace trace(walker_truncated, 3, 300); + MethodInfo root; + root._type = root_type; + root._is_entry = entry; + Buffer buffer; + RecordingTestAccessor::writeHeader(&buffer, &trace, &root); + ASSERT_EQ(4, buffer.offset()); + // Decode the variable-length pool id before inspecting the serialized + // flag, so a field-order or varint-boundary error cannot pass unnoticed. + const unsigned char* bytes = (const unsigned char*)buffer.data(); + EXPECT_EQ(0xac, bytes[0]); + EXPECT_EQ(0x02, bytes[1]); + EXPECT_EQ(expected ? 1 : 0, bytes[2]); + EXPECT_EQ(3, bytes[3]); + } +}; + +TEST_F(StackTraceSerializationTest, ExplicitTruncationSurvivesJavaEntryRoot) { + check(true, FRAME_INTERPRETED, true, true); +} + +TEST_F(StackTraceSerializationTest, JavaEntryRootWithoutMissingFramesIsComplete) { + check(false, FRAME_INTERPRETED, true, false); +} + +TEST_F(StackTraceSerializationTest, JavaNonEntryRootIsIncompleteWithoutWalkerFlag) { + check(false, FRAME_INTERPRETED, false, true); +} + +TEST_F(StackTraceSerializationTest, JavaNonEntryRootPreservesWalkerFlag) { + check(true, FRAME_INTERPRETED, false, true); +} + +TEST_F(StackTraceSerializationTest, NativeRootPreservesExplicitTruncation) { + check(true, FRAME_NATIVE, false, true); +} + +TEST_F(StackTraceSerializationTest, NativeRootWithoutWalkerFlagIsComplete) { + check(false, FRAME_NATIVE, false, false); +} + +} // namespace 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 cd4a68b0f8..592ac5aba4 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 @@ -71,10 +71,10 @@ public void keepsPlatformCallersComplete() throws Exception { assertCompleteCallers(); } - /** Ending at a fully thawed continuation is a complete logical stack in the ordinary VM mode. */ + /** A never-parked virtual thread retains its callers without freezing or thawing them. */ @TestTemplate @ValueSource(strings = {"vm"}) - public void keepsFullyThawedContinuationComplete() throws Exception { + public void keepsNeverParkedVirtualThreadComplete() throws Exception { runWorkload(true, false); assertCompleteCallers(); }