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 52bf894354..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" @@ -417,22 +418,32 @@ __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 // (_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 { + 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; if (!features.carrier_frames) { fillFrame(frames[depth++], BCI_NATIVE_FRAME, CONT_ROOT_FRAME); @@ -456,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)) { @@ -519,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()); @@ -541,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) { @@ -649,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)); +} 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..cd4a68b0f8 --- /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() 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() 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() 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; + } +} diff --git a/docs/sphinx/specs/2026-10-23-sphinx-review-medium-the-fix-marks-trunc.md b/docs/sphinx/specs/2026-10-23-sphinx-review-medium-the-fix-marks-trunc.md new file mode 100644 index 0000000000..48b0bfe9fc --- /dev/null +++ b/docs/sphinx/specs/2026-10-23-sphinx-review-medium-the-fix-marks-trunc.md @@ -0,0 +1,179 @@ +# Spec: sphinx-review-medium-the-fix-marks-trunc + +run_id: 1767213197-srs_rXoji4Z8Jop4LlhpXRRNgZ5F +Design: .sphinx/address/design.md (converged after design adversary round 2) + +## Problem +`HotspotSupport::walkVM` (ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp) +crosses a virtual-thread continuation boundary via the local lambda +`walkThroughContinuation(bool all_frames_thawed)`. That one bool drives two +independent decisions: +(a) frame layout: on JDK 21-26 `entry_fp` is `fp` (thawed) or `*fp` (frozen), and +(b) completeness: whether older frozen callers in a heap StackChunk are skipped, +so the sample must be marked `*truncated = true`. + +The speculative branch (unknown CodeHeap PC, `nm == NULL`, on a carrier, guarded +by `features.carrier_frames && hotspot_version >= 21 && depth > 0 && +isCarryingVirtualThread()`, hotspotSupport.cpp:547-563) calls +`walkThroughContinuation(true)`. It wants the thawed layout guess for (a), but +it has no evidence for (b). If frozen frames remain, the walk moves on to the +carrier stack, the frozen callers are silently skipped, and the sample is never +marked truncated (F-dbad9075ec51). + +## Correct behaviour +The boundary classification is explicit: `ContBoundary { Thawed, Frozen, Unverified }`. +Two pure predicates decide everything inside the lambda. + +Ownership +- R1. `walkThroughContinuation` is the only place that applies "boundary may + hide callers ⇒ `*truncated = true`". Call sites classify the boundary. They + never write `*truncated` themselves. +- R2. `*truncated` is only ever set to `true` and never cleared. The post-loop + depth-overflow and root-`BCI_ERROR` rules stay unchanged. +- R3. `truncated` may be `nullptr`. Every write is guarded by `truncated != nullptr`. + +Transitions (one per call site) +- R4. Exact `isContReturnBarrier` match ⇒ `Frozen`: layout `*fp`, completeness + NOT proven ⇒ truncated. (Unchanged.) +- R5. Exact `isContEntryReturnPc` match ⇒ `Thawed`: layout `fp`, complete ⇒ the + boundary does not mark truncated. (Unchanged.) +- R6. `nm == VMStructs::enterSpecialNMethod()` ⇒ `Thawed`: same as R5. (Unchanged.) +- R7. Speculative unknown-nmethod on a carrier ⇒ `Unverified`: layout `fp` + (thawed guess, unchanged), completeness NOT proven ⇒ truncated. **(The fix.)** +- R8. Truncation is applied on entry to the crossing. That is before the + `depth >= actual_max_depth` early return, independent of + `features.carrier_frames`, and independent of whether the carrier walk then + succeeds or emits `BCI_ERROR`. +- R9. Only `Thawed` proves completeness. Any other value, including a future + enumerator or an out-of-range value, defaults to "not proven" (truncated). + +Edge policy +- R10. An unverified boundary is reported truncated (possible false positive for + fully-thawed VTs reached via the speculative branch, cstack=vmx only) rather + than complete (silent false negative). + +Concurrency / signal safety +- R11. walkVM runs in a signal handler under sigsetjmp crash protection. The + change adds no allocation, locks, syscalls, new memory dereferences, shared + state or counters. The classification is `constexpr` / local only. + `WALKVM_CONT_SPECULATIVE_HIT` keeps incrementing as before. + +## Constraints +- C++17 (`inline constexpr` is available). Header-only helper with the repo's standard + Datadog copyright/SPDX header, no includes, no state. +- No counter name or ID changes (counters.h:141-144). No Java API change. No new profiler + option. No new DEBUG env knob (design A3). +- No behaviour change for the barrier, entry-return-pc or enterSpecial sites, the + cstack=vm path, or the JDK 27+ `type_size() > 0` layout path (only the + completeness rule applies there, as today). +- Existing `FrozenVirtualThreadStackTest` must stay green with no test changes. + +## Scope + +### Primary fixes +- ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp:560 (F-dbad9075ec51): + completeness claimed for an unverified continuation boundary, because the + layout and completeness facts were folded into one bool. Fix: replace the + `bool all_frames_thawed` lambda parameter with `ContBoundary kind`, using + predicates from a new `hotspot/contBoundary.h`. Each of the four call sites + passes a named per-site constant. The speculative site passes + `CONT_BOUNDARY_AT_SPECULATIVE_UNKNOWN_NM` (= `Unverified`) and gets a + `static_assert` beside it. Update the stale comments (425-433, 527-533, + 548-555). + +### Auto-expanded sibling fixes +None identified. + +Candidates examined (all from the design's Sibling Defect Assessment). None was +kept as a code-change sibling. Each is recorded below with the reason. + +- S0: hotspotSupport.cpp:749-816, generic stub path for an unmatched barrier PC. + Defect class: a boundary is skipped without proof that the StackChunk is empty. + ``` + FLOW: walkVM loop -> CodeHeap::contains(pc) -> isContReturnBarrier(pc) false + (_cont_return_barrier unresolved: on JDK 21-26 it is resolved only via the + .symtab symbol _ZN12StubRoutines19_cont_returnBarrierE, vmStructs.cpp:417-420, + so it can be null on a stripped libjvm) -> findNMethod returns the + StubRoutines BufferBlob (no kind check) -> anchor cleared (cont_unwind_active) + -> stub else-branch -> frame.unwindStub(start, name, ...) -> ... + PRECONDITION: barrier symbol unresolved or misresolved AND unwindStub (or the + frameSize fallback) returns true for the barrier PC AND the next unwound + words lead to a walk that ends without a root BCI_ERROR. + (i) BufferBlob frame_size is 0 in HotSpot, so the frameSize fallback + normally does not fire. (ii) CAN succeed per source: + x64 hotspotStackFrame_x64.cpp:18-24 returns true when ip == entry (the + barrier PC is its own stub's start if JitCodeCache registered the stub), + or via the push-rbp prologue path at :26-38; aarch64 + hotspotStackFrame_aarch64.cpp:221-268 returns true on ip == entry + (pc = link()) or via StubUnwindInfo phases. The walk after that depends on + runtime stack contents at *sp / LR. + REACHABLE: no — latent (end-to-end un-truncated outcome unverifiable without + runtime evidence) + CONCLUSION: low; not fixed. A blanket "stub crossing on a carrying VT at + depth>0 ⇒ truncated" would mark every VT sample that walks through a + runtime stub with native VM frames above it (e.g. C2 runtime calls) as + truncated, a broad false-positive regression outside this item. The + design says not to fix without a verified path. Recorded as residual + risk RR2. + ``` +- walkVM unknown-nmethod fallthrough when the speculative guard is false + (hotspotSupport.cpp:564-568). + ``` + FLOW: nm == NULL with cstack=vm or depth == 0 -> anchor == NULL: "unknown_nmethod" + BCI_ERROR root -> post-loop sets truncated (hotspotSupport.cpp:1059-1063); + anchor != NULL: break -> anchor recovery (990-1035) redirects to lastJavaSP, + the TOP Java frame, and resumes the walk downward. + PRECONDITION: the recovery would have to jump past a continuation boundary. + REACHABLE: no — structurally impossible (the anchor still being non-NULL means + no eligible CodeHeap nmethod frame was consumed yet, so recovery restarts + above, not below, any boundary; anchor_sp == 0 means no Java frames, so no + boundary was crossed) + CONCLUSION: not a sibling. + ``` +- `walkThroughContinuation` with `!features.carrier_frames` and `Unverified`. + The speculative call requires `features.carrier_frames` (hotspotSupport.cpp:556), + so this combination is unreachable today. If it is ever reached, R8 still marks + truncated before emitting the "JVM Continuation" root. REACHABLE: no — + structurally impossible. Not a sibling. +- ASGCT post-processing `asyncJavaTraceWithPostProcessing` (hotspotSupport.cpp:1334-1341) + appends a synthetic "JVM Continuation" root with no truncation decision. Whether + AsyncGetCallTrace omits frozen StackChunk frames is HotSpot behaviour that cannot + be verified from this repo. Different walker, different defect owner. Out of + scope and recorded only (RR4). +- Nested continuations (JDK 27+, `cont_entry->parent()`): each crossing goes through + the same lambda and is classified independently. No extra sibling. + +## Assumptions +- A1 (≥90%): marking truncated on every speculative crossing is the intended + resolution. The accepted cost is possible false-positive truncation on fully + thawed VTs in cstack=vmx when `cont_entry_return_pc` is unresolved or misresolved. +- A2 (≥90%): a resolved `_cont_return_barrier` is not trusted to rule out a frozen + boundary on the speculative path. +- A3 (≥80%, deliberate choice): no DEBUG env seam to force the speculative path. + Coverage is a JVM-free gtest of the mapping and the named constants, plus a + `static_assert` at the call site. Call-site wiring is residual risk RR1. +- A4 (≥90%): the speculative path keeps the thawed `entry_fp` layout guess (R7). +- A5: the speculative branch is entered only on `nm == NULL`. The fix does not + depend on which condition produced it. +- A6 (planner, ≥90%): the helper lives at + `ddprof-lib/src/main/cpp/hotspot/contBoundary.h`, and the test at + `ddprof-lib/src/test/cpp/contBoundary_ut.cpp`, which the gtest plugin + discovers automatically (one binary per `*_ut.cpp`; `src/main/cpp` is on the + include path, as `stubUnwindInfo_ut.cpp` shows with `#include "hotspot/stubUnwindInfo.h"`). +- A7 (planner, ≥90%): `contBoundaryProvesComplete` is written as + `kind == ContBoundary::Thawed`, not `kind != Frozen && kind != Unverified`, so + R9's safe default holds for out-of-range or future values. + +## Residual Risks +- RR1: the speculative call-site wiring is not unit-tested. It is mitigated by + the named constants, the `static_assert`, and review. +- RR2: S0 (stub path for an unresolved barrier) may continue the walk + un-truncated. This is unverifiable without runtime evidence. +- RR3: false-positive truncation for fully thawed VTs on the speculative path + (cstack=vmx). Accepted. +- RR4: the ASGCT path's synthetic continuation root never marks truncated. This + is out of scope and depends on HotSpot behaviour. +- RR5: JFR only writes `trace->truncated` when the bottom frame is not a Java + frame (flightRecorder.cpp:1946-1953). A speculative crossing that ends on a + carrier Java entry frame would not show R7's flag in JFR. That is acceptable + for this item: the walker contract is what changes.