Select rows and symbols in walkVM by attribution address

[rkennke]

Addresses PROF-15955.

Summary

HotspotSupport::walkVM fed the raw walking pc to findLibraryByAddress, findFrameDesc and the DW_REG_PLT stub-offset test. Past the leaf that pc is a return address, so a call that is the last instruction of its caller selected the following function's CFA row, derived a sender sp from it, and could miss a MARK_THREAD_ENTRY sitting on the caller. Same defect #786 fixed in StackWalker::walkFP/walkDwarf — except this is the path CSTACK_DEFAULT actually resolves to whenever the JVM exposes VMStructs on Linux, so it is the common production configuration rather than a fallback.

walkVM now carries the pc and whether it came out of a return-address slot as a
single WalkPc value — the type #786 introduced in walkFP/walkDwarf for
exactly this hazard — and routes the range-based lookups through its
attribution(). Every pc source states what it produced (setReturnAddress,
setRecoveredPc, setExactAddress, setSeed), so adding one is a compile-time
decision rather than something a reviewer has to catch.

Adopting it surfaced a second defect, now also fixed here: walkVM is entered
either from a ucontext (pc is the exact interrupted address) or from
callerPC() (a genuine return address wherever CALLER_PC_IS_RETURN_ADDRESS
holds, i.e. everywhere but aarch64). Both arrived as a bare pointer and were
treated as exact, so the callerPC() entry never got the adjustment. The
private overload now takes that distinction from its caller, the same way
walkFP/walkDwarf seed themselves.

Which pcs get adjusted, and which deliberately do not

Consumer	Address used	Why
findLibraryByAddress, findFrameDesc	attribution	range lookups; the whole point of the fix
DW_REG_PLT stub-offset test	attribution	must be decided on the same address the row was selected with
isContReturnBarrier, isContEntryReturnPc, isEntryFrame	raw	exact-equality comparisons against return addresses
DW_PC_OFFSET arithmetic	raw	DW_OP_breg<PC> names the pc register value
no-progress guard, inDeadZone	raw	needs the exact address
isFrameCompleteAt, findScopeOffset (PcDesc)	raw	see below

A signal-frame CIE suppresses the adjustment exactly as it does in walkDwarf (pc_is_ra = !f.isSignalFrame()).

Every site in walkVM that writes pc — 16 of them — sets the flag. Two were missing from the original enumeration in the ticket: both in-loop anchor->getFrame(pc, sp, fp) calls silently rewrite pc to lastJavaPC(), which is a return address.

The wire format does not move

resolveNativeFrameForWalkVM was using one address for two jobs: range lookups and the emitted remote-symbolication pc_offset. It now takes the attribution address for findLibraryByAddress/binarySearch while pc_offset keeps deriving from the raw pc, so the emitted wire value is byte-identical.

This matters for review: it means this PR does not depend on the unresolved cross-team question about pc_offset semantics raised in #786. It is effectively option (b) from that PR's own list of options, applied locally.

Two findings that correct the ticket's premises

The suspected PcDesc double-subtraction bug does not exist. PROF-15955 flagged that x64's ad hoc -1 being re-fed into isFrameCompleteAt/findScopeOffset might be a second latent bug. It is not: findScopeOffset (vmStructs.cpp:798) is a ceiling search — on an exact miss it returns the first PcDesc with _pc >= pc_offset — and HotSpot records PcDescs at return addresses, so RA-1 and RA resolve to the same entry. isFrameCompleteAt is a monotone threshold. Both absorb the decrement, so the PcDesc sites are safe on the raw pc.

There is a different, real x86_64 bug — filed as PROF-16033, not fixed here. unwindPrologue/unwindEpilogue/unwindStub fold -1 into the pc they return on x86_64 but not on aarch64, so on x86_64 the three exact-equality comparisons above always fail for pcs arriving through those helpers: continuation-boundary and entry-frame detection silently do not fire. The adjustment is also branch-inconsistent (unwindPrologue's isFrameComplete branch at hotspotStackFrame_x64.cpp:155 omits it), which is why a correct flag cannot be threaded without changing those helpers' contract.

Those three call sites are therefore flagged pc_is_ra = false here — conservative, cannot double-adjust, and no behaviour change on either arch. The arch files are untouched. Fixing them properly means touching x86_64/aarch64 pattern-matching code shared with the legacy getJavaTraceAsync/AsyncGetCallTrace path, which is a different blast radius and belongs in its own change.

Deliberately left untouched

Per the ticket's step 5, calling these out so they do not read as oversights:

• JVM-internal metadata lookups (CodeHeap::findNMethod, isFrameCompleteAt, findScopeOffset → PcDesc) stay on the raw pc — correct by HotSpot's own convention, confirmed above.
• unwindPrologue/unwindEpilogue/unwindStub — PROF-16033.
• unwindCompiled — same inconsistency, but only reachable from the AGCT path, not walkVM. Covered by PROF-16033.
• PerfEvents::walkKernel — carries a related defect but is outside PROF-15955's scope; it is a pure consumer of the kernel's ring buffer and would be a much smaller separate change.

Test plan

• buildDebug / buildRelease — clean
• gtestDebug — 67 test binaries, 0 failures
• walkVmAttribution_ut.cpp gates the address split (new, 3 tests)
• walkVM's own per-frame flag threading is still ungated — see below

resolveNativeFrameForWalkVM uses two addresses for two jobs, and both halves
are now pinned at a synthetic zero-gap symbol boundary, the shape where the two
disagree:

test	pins
ReturnAddressAtZeroGapBoundaryResolvesToTheCaller	flagging the pc as a return address moves the resolved symbol from the following function back to the caller
PcOffsetStaysDerivedFromTheRawPc	the same flag leaves the emitted pc_offset untouched — the attribution address is a lookup detail and must not reach the wire
AddressInsideAFunctionResolvesTheSameEitherWay	the adjustment is not a blanket shift; well inside a function both flags agree

Both assertions were mutation-checked against the production code:

mutation	result
lookups reverted to the raw pc	boundary test fails, other two pass
pc_offset derived from the attribution address	wire test fails, other two pass
neither	all three pass

Each mutation kills exactly its intended test, so the coverage is targeted
rather than incidental.

The fixtures publish synthetic CodeCaches via a new test-only
Libraries::addLibraryForTest() (matching the existing *ForTest idiom in
profiler.h, os.h, callTraceHashTable.h). That is what makes a controlled
zero-gap boundary possible; using the test binary's own symbols cannot produce
one. It also keeps these tests free of the GNU-as/ELF-CFI and
updateSymbols() dependencies that confine returnAddressAttribution_ut to
Linux, so they run on macOS dev builds too.

Still ungated: the 21 per-frame WalkPc mutator calls inside walkVM. The
WalkPc conversion removes the drift hazard (a pc can no longer be assigned
without restating its nature) but does not prove each individual choice is the
right one.

Reaching them from a test is not viable in-process, which I confirmed rather
than assumed: walkVM calls VMThread::current() unconditionally, which asserts
_jvm_thread.isKeyValid() — a key only created by JVMThread::initialize(), which
needs JNI into a live JVM. In a debug gtest that aborts; with the profiler's SIGSEGV
handler installed it becomes an infinite signal loop. Defeating that assert would mean
disabling the very check that says the call is invalid, so I did not.

The coupling is the real problem, and it is now tracked as PROF-16059: walkVM's
dwarf_unwind block and walkDwarf's loop body are two copies of one algorithm
(22 of walkDwarf's 33 code lines appear verbatim in walkVM's 56), and extracting
the shared step yields something testable with synthetic CodeCaches — the mechanism
this PR's new tests already use. That ticket blocks PROF-16033, which has no way to
verify itself for the same reason.

Reviewers: the judgement calls worth pushing back on are (a) the
raw-vs-attribution split in the table above, (b) the new test-only entry point
on Libraries, (c) setExactAddress as the right choice where the arch unwind
helpers already fold in their own adjustment (see PROF-16033), and (d) whether
the remaining walkVM-level gap should block merge.

🤖 Generated with Claude Code

[dd-octo-sts]

CI Test Results

Run: #36020394834 | Commit: 82ddc50 | Duration: 15m 30s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK	glibc-aarch64/debug	glibc-amd64/debug	musl-aarch64/debug	musl-amd64/debug
8	-	✅	-	-
8-ibm	-	✅	-	-
8-j9	✅	✅	-	-
8-librca	-	-	✅	✅
8-orcl	-	✅	-	-
11	-	✅	-	-
11-j9	✅	✅	-	-
11-librca	-	-	✅	✅
17	✅	✅	-	-
17-graal	✅	✅	-	-
17-j9	✅	✅	-	-
17-librca	-	-	✅	✅
21	✅	✅	-	-
21-graal	✅	✅	-	-
21-librca	-	-	✅	✅
25	✅	✅	-	-
25-graal	✅	✅	-	-
25-librca	-	-	✅	✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0

---

Updated: 2026-09-24 15:47:46 UTC

[dd-octo-sts]

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 77538df1

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63de2f9c71

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve return-address state after arm64 JIT unwinding

On supported arm64, unwindEpilogue, unwindPrologue, and unwindStub assign pc from StackFrame::link(), which is the caller's raw return address; this records it as exact, so the following native resolution and DWARF lookup do not subtract one byte. A sample that leaves a generated frame into a native caller whose call site ends at a symbol/FDE boundary is therefore still attributed to the following symbol/row (the same classification is repeated at lines 676 and 732). Mark these outputs as return addresses, or have the unwind helpers report whether they already adjusted the PC.

AGENTS.md reference: AGENTS.md:L199-L203

Useful? React with 👍 / 👎.

[datadog-datadog-prod-us1]

More details

The change keeps raw PCs for exact comparisons and wire offsets. It uses attribution PCs for library, symbol, and unwind-row range lookups.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 63de2f9 · @DataDog review to ask questions}