Select rows and symbols in walkVM by attribution address - #813

Targets main directly. #786 has merged (squashed as 0d06533), so attributionPC() is on main and the earlier stacking is gone — this is now a single commit on top of main. The ASAN chain-buffer fix that #786 needed is also on main, so it is not part of this diff.

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 tracks per-frame whether the pc came out of a return-address slot and routes the range-based lookups through attributionPC().

Which pcs get adjusted, and which deliberately do not

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 — clean
• buildRelease — clean
• gtestDebug — 64 test binaries, 0 failures, including Fix: PROF-15934 - native stack walker return-address attribution #786's returnAddressAttribution_ut
• No test gates the new behaviour. This is a real gap, not an oversight.

walkVM's affected paths only fire inside a live JVM, and resolveNativeFrameForWalkVM resolves through the Libraries singleton whose _native_libs is private with no injection seam. Adding a test that exercises adjacent code rather than the changed code would be false confidence, so none was added. The passing suite establishes no regression, not correctness. Real coverage needs the harness described in step 4 of the ticket — a synthetic CodeCache/Libraries seam, or a live-JVM fixture forcing a noreturn call as the last instruction of a native function reached from Java — and that is its own piece of work.

Reviewers: the judgement calls worth pushing back on are (a) the raw-vs-attribution split in the table above, and (b) whether shipping this without behavioural coverage is acceptable given walkVM is the default path.

🤖 Generated with Claude Code