fix: save LR in arm64 non-G0 asmcall trampoline (root-cause of darwin/arm64 hang) - #193
Merged
Merged
Conversation
…arm64 asmcall/arm64.s: the ASMCALL macro (CALL R8; RET) never saved LR (x30). On arm64, BLR overwrites x30 with the trampoline's own return address, so after the C callee returned, the trampoline's RET jumped to itself and spun forever (observed as the darwin/arm64 hang found by task 4.3 CI; pc == lr == CallFuncP0+8 confirmed with lldb). It is an arm64 ISA bug, not darwin-specific: linux/arm64 would hang identically but was never covered by CI. Save/restore x30 (and 16-byte-align SP per AAPCS) around the call, mirroring what the G0 variant already does. amd64 is unaffected (return addresses live on the stack). asmcall/calltest/call_test.go: gate tests (linux || darwin) instead of linux-only; the root cause is fixed so darwin runs the trampoline tests. .github/workflows/ci.yml: add an ubuntu-24.04-arm leg so both ISAs get real trampoline coverage; update the macOS leg comment. docs/ci.md: document the linux/arm64 leg and the updated macOS leg.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #193 +/- ##
=======================================
Coverage 97.09% 97.09%
=======================================
Files 24 24
Lines 3955 3955
=======================================
Hits 3840 3840
Misses 115 115 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
asmcall/amd64.s already has a Win64 calling-convention path (arguments in CX/DX/R8, selected via GOOS_windows), and amd64 CALL/RET is stack-based so the arm64 LR bug class does not apply -- but no CI leg ever executed it. asmcall/calltest: relax build tags to (linux || darwin || windows) && (amd64 || arm64) so the trampoline tests run on windows/amd64. .github/workflows/ci.yml: add a windows-latest leg. mem-ring is unix-only and test/go depends on it, so the leg runs go test ./asmcall/... only; the runner image's MSYS2 mingw-w64 gcc covers the cgo-built calltest callees. docs/ci.md: document the windows leg.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Root-caused and fixed the darwin/arm64
asmcallnon-G0 trampoline hang found by task 4.3 CI (TestCallFuncP0timing out on macos-latest), and extended CI so all supported OS/ISA combos get real trampoline coverage.Root cause
The arm64
ASMCALLmacro wasCALL R8; RET— it never saved LR (x30). Unlike amd64, where return addresses live on the stack, on arm64BLR R8overwrites x30 with the trampoline's own return address (CallFuncP*+8). The C callee (a leaf) returns correctly to+8, but the trampoline's ownRETthen jumps to x30 — which still holds+8— and spins forever jumping to itself.Reproduced locally on macOS arm64 and confirmed with lldb: the hung thread's
pc == lr == CallFuncP0+8(theRETinstruction), process state R (spinning, not blocked).This is an arm64 ISA bug, not darwin-specific: linux/arm64 would hang identically — it was simply never covered (CI only ran the tests on linux/amd64). The G0 variants passed because
G0ASMCALLalready saves/restores x30. SP alignment, PAC/arm64e, and stack protectors were all ruled out.Changes
asmcall/arm64.s:ASMCALLnow saves/restores x30 around the call (and 16-byte-aligns SP per AAPCS), mirroring the G0 variant. amd64 is untouched.asmcall/calltest/call_test.go,asmcall/calltest/calltest.go: build tags relaxed to(linux || darwin || windows) && (amd64 || arm64); comments updated with the root cause. macOS CI now runs the trampoline tests instead of compile/vet-only..github/workflows/ci.yml:ubuntu-24.04-armleg — the arm64 LR bug class was invisible to linux/amd64;windows-latest(amd64) leg —amd64.shas a Win64 calling-convention path (args in CX/DX/R8 viaGOOS_windows) no leg ever executed. mem-ring is unix-only andtest/godepends on it, so this leg runsgo test ./asmcall/...only; the runner image's MSYS2 mingw-w64 gcc covers the cgo-built calltest callees.docs/ci.md: documented the new legs.This supersedes the previously considered fallback of restricting the asm fast path to Linux — macOS and Windows keep the fast path.
Testing
go test ./asmcall/...passes, including all 8 trampoline variants on darwin (previously hanging).CallFuncP{0..3}withGC()/Gosched()interleaved — pass.gofmt -lclean;go vet -unsafeptr=false ./...clean.ubuntu-24.04-arm,windows-latestand updatedmacos-latestlegs validate on this PR's CI run.