Hand back raw return addresses from the unwind helpers

[rkennke]

Addresses PROF-16033. Split out of PROF-15955; unblocked by PROF-16059.

The defect

unwindStub, unwindPrologue, unwindEpilogue and unwindCompiled recover a sender pc from a stack slot or the link register. x86_64 subtracted one from it inside the helper; aarch64 did not. Nothing in the signature said which, so walkVM guessed from the target architecture — that is what recordUnwoundPc and its #ifdef were.

The guess was wrong in a way that mattered. On x86_64 the pc reaching the top of the walk loop was already decremented, so these three — all comparing for exact equality against genuine return addresses — could never match:

• VMStructs::isContReturnBarrier
• VMStructs::isContEntryReturnPc
• VMNMethod::isEntryFrame

Continuation boundaries and entry frames stopped being detected on x86_64. Silently: worse unwinding, no visible failure.

x86_64 was not even self-consistent. unwindPrologue's isFrameComplete arm and unwindCompiled's entry-barrier arm returned the address unadjusted while their siblings subtracted one, so no single rule described what a caller had been handed. That inconsistency is why PROF-15955 could not simply flag these results as return addresses — it would have double-adjusted the majority of branches.

The change

All eleven folded adjustments removed. Every helper returns the address exactly as it sat in the slot; walkVM applies the attribution adjustment itself via the WalkPc it already carries.

	lookups (findLibraryByAddress, findFrameDesc)	exact-equality consumers
x86_64 before	RA-1 ✅	RA-1 ❌
x86_64 after	RA-1 ✅ (unchanged)	RA ✅ fixed
aarch64 before/after	RA-1 ✅	RA ✅

So lookup behaviour is identical everywhere, and the only behavioural delta is x86_64's exact-equality comparisons starting to match.

recordUnwoundPc and its #ifdef __aarch64__ are deleted — the last place the two architectures disagreed. Net −38 lines of production code.

Please push back on this one

unwindCompiled is reached only from getJavaTraceAsync (the legacy AsyncGetCallTrace path). That path now receives raw addresses on x86_64 where it previously got adjusted ones. aarch64 already handed it raw, so this makes the two agree rather than creating a new split — but it is a behaviour change on a path no test covers, and it was the open question when PROF-16033 was first split out. I went with convergence for coherence; a reviewer who knows AGCT should decide whether that is right.

Testing

unwindHelperContract_ut.cpp (new, 5 tests) drives the helpers directly — a fabricated frame, a few bytes of synthetic code, and a 32-byte buffer standing in for an nmethod. No JVM is started, and no __linux__ gate.

That is deliberate. Reaching these helpers through walkVM means getting past vm_thread == NULL (hotspotSupport.cpp:509, which sits above every consumer) and then faking a JavaThread and a CodeHeap range as well. I mapped that cascade before writing this; the further such a fixture drifts from a real JVM layout, the likelier it is to pass for the wrong reason.

test	helper	branch	sender pc read from
UnwindStubAtEntryRecoversTheRawSenderPc	unwindStub	ip == entry	sp[0] / link register
HelperNeverAppliesTheAdjustmentItself	unwindStub	—	states the property arch-independently
UnwindPrologueAtEntryRecoversTheRawSenderPc	unwindPrologue	ip <= entry	sp[0] / link register
UnwindPrologueAfterFrameSetupRecoversTheRawSenderPc	unwindPrologue	past the push %rbp / stp x29,x30	sp[1], both arches
UnwindEpilogueAtReturnRecoversTheRawSenderPc	unwindEpilogue	on the ret	sp[0] / link register

The AfterFrameSetup one is the interesting branch: it is where x86_64 read a different stack slot than its sibling, and both arches happen to source from sp[1] and pop two, so a single assertion covers both. The adjacent slot holds a distinct sentinel, so an off-by-one-slot fails too, not just an off-by-one-byte.

The two nmethod-taking helpers need entry() and frameSize(), so the fixture points _frame_size_offset / _nmethod_entry_address at its own layout for the duration of the test and restores them in TearDown, via a per-TU VMStructsTestAccessor — the same idiom as hotspotMethodId_ut.cpp and hotspot_crash_protection_ut.cpp.

Mutation-checked: each of the five sites was mutated by reintroducing a folded - 1 there. Every mutation was caught, each by exactly one test.

What is not covered

• The x86_64 fix is not exercised locally. I develop on aarch64, which is precisely the arch this does not change; my green run proves only that the unchanged path still works. gtest-release-amd64 and the amd64 sanitizer jobs are what actually test this.
• The symptom, end to end — that a pc from these helpers subsequently matches isEntryFrame/isContReturnBarrier — still needs the full walkVM cascade above. Worth recording on the ticket rather than building for.
• unwindPrologue's isFrameComplete arm is not covered. That one additionally needs frameCompleteOffset() and a plausible instruction stream, i.e. a good deal more fixture for one more branch.
• unwindCompiled has no test at all. Found incidentally while mutation-checking: a - 1 reintroduced at hotspotStackFrame_aarch64.cpp:157 — the same branch shape the prologue test covers — is caught by nothing. It is the same signature, so it is reachable with the fixture this PR adds. I left it out because hotspotStackFrame.h:90 marks unwindCompiled for removal once vm becomes the default walking mode, and it is also the helper flagged under Please push back on this one above. Happy to add it if a reviewer would rather have the net there before that removal lands.
• The nmethod fixture pins the pre-JDK-23 field shape. entry() branches on _nmethod_entry_offset != -1, and the older arm is a single indirection rather than a code_offset/verified_entry_offset pair. What is under test is the addressing convention, not the field layout, but the tests say so explicitly so nobody reads them as covering both nmethod shapes.

Local: full gtestDebug and gtestRelease suites green on aarch64.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c21727a7f

ℹ️ 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".

[datadog-prod-us1-5]

The x86_64 AGCT recovery path now retries HotSpot with an unadjusted sender return address. Because that path bypasses WalkPc, recovered callers can be attributed to the instruction after the call and receive the wrong bytecode.

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 7c21727 · @DataDog review to ask questions}

[datadog-prod-us1-5]

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 4ee10c4 | Docs | View more details | Give us feedback!}

[dd-octo-sts]

CI Test Results

Run: #36724788720 | Commit: 85b0d5a | Duration: 16m 0s (longest job)

❌ 1 of 32 test jobs failed

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

Failed Jobs

• glibc-aarch64/debug / 17 — view logs

Summary: Total: 32 | Passed: 31 | Failed: 1

---

Updated: 2026-09-30 14:11:55 UTC

[dd-octo-sts]

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 4ee10c47

[jbachorik]

Nice reconciliation!

[kaahos]

this looks good to me, thanks for the changes!