From f7a87e32997345b7acaabbf725d1b210e1cfe327 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 28 Aug 2023 19:26:46 +0000 Subject: [PATCH] runtime: fix bp restoration in panic recovery for arm64 Previously, the frame pointer wouldn't be restored at all, which could cause panics during frame pointer unwinding. As of CL 516157, the frame pointer is restored, but it's restored incorrectly on arm64: on arm64, the frame pointer points one word below SP, but here it's one below panic.fp which is the stack pointer of the caller's frame (nothing to do with the architectural bp). For #61766. Change-Id: I86504b85a4d741df5939b51c914d9e7c8d6edaad Reviewed-on: https://go-review.googlesource.com/c/go/+/523697 TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui Run-TryBot: Michael Knyszek LUCI-TryBot-Result: Go LUCI --- src/runtime/callers_test.go | 1 + src/runtime/panic.go | 33 +++++++++++++++++++-------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/runtime/callers_test.go b/src/runtime/callers_test.go index 42091b04fc..49a1d5a6f7 100644 --- a/src/runtime/callers_test.go +++ b/src/runtime/callers_test.go @@ -478,6 +478,7 @@ func TestFPUnwindAfterRecovery(t *testing.T) { pcs[i] = 10 } runtime.FPCallers(pcs) + t.Logf("%v", pcs) }() defer func() { if recover() == nil { diff --git a/src/runtime/panic.go b/src/runtime/panic.go index acbbaa718f..cb624ec9ef 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -313,10 +313,10 @@ func deferproc(fn func()) { // The g._defer list is now a linked list of deferred calls, // but an atomic list hanging off: // -// g._defer => d4 -> d3 -> drangefunc -> d2 -> d1 -> nil -// | .head -// | -// +--> dY -> dX -> nil +// g._defer => d4 -> d3 -> drangefunc -> d2 -> d1 -> nil +// | .head +// | +// +--> dY -> dX -> nil // // with each -> indicating a d.link pointer, and where drangefunc // has the d.rangefunc = true bit set. @@ -340,10 +340,10 @@ func deferproc(fn func()) { // // That is, deferconvert changes this list: // -// g._defer => drangefunc -> d2 -> d1 -> nil -// | .head -// | -// +--> dY -> dX -> nil +// g._defer => drangefunc -> d2 -> d1 -> nil +// | .head +// | +// +--> dY -> dX -> nil // // into this list: // @@ -1149,15 +1149,20 @@ func recovery(gp *g) { gp.sched.sp = sp gp.sched.pc = pc gp.sched.lr = 0 - // fp points to the stack pointer at the caller, which is the top of the - // stack frame. The frame pointer used for unwinding is the word - // immediately below it. - gp.sched.bp = fp - goarch.PtrSize - if !usesLR { + // Restore the bp on platforms that support frame pointers. + // N.B. It's fine to not set anything for platforms that don't + // support frame pointers, since nothing consumes them. + switch { + case goarch.IsAmd64 != 0: // on x86, fp actually points one word higher than the top of // the frame since the return address is saved on the stack by // the caller - gp.sched.bp -= goarch.PtrSize + gp.sched.bp = fp - 2*goarch.PtrSize + case goarch.IsArm64 != 0: + // on arm64, the architectural bp points one word higher + // than the sp. fp is totally useless to us here, because it + // only gets us to the caller's fp. + gp.sched.bp = sp - goarch.PtrSize } gp.sched.ret = 1 gogo(&gp.sched) -- 2.48.1