[release-branch.go1.21] runtime: restore caller's frame pointer when recovering from...

When recovering from a panic, restore the caller's frame pointer before
returning control to the caller. Otherwise, if the function proceeds to
run more deferred calls before returning, the deferred functions will
get invalid frame pointers pointing to an address lower in the stack.
This can cause frame pointer unwinding to crash, such as if an execution
trace event is recorded during the deferred call on architectures which
support frame pointer unwinding.

Original CL by Nick Ripley, includes fix from CL 523697, and includes a
test update from CL 524315.

This CL also deviates from the original fix by doing some extra
computation to figure out the fp from the sp, since we don't have the fp
immediately available to us in `recovery` on the Go 1.21 branch, and it
would probably be complicated to plumb that through its caller.

For #61766
Fixes #62046

Change-Id: I5a99ca4f909f6b6e209a330d595d1c99987d4359
Reviewed-on: https://go-review.googlesource.com/c/go/+/523698
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/runtime/callers_test.go b/src/runtime/callers_test.go
index 7e2c6c82385312cbabea3e03b284c532e79efe86..d316ee97a1a9edb620916cc377c80f07561d24b9 100644 (file)
--- a/src/runtime/callers_test.go
+++ b/src/runtime/callers_test.go
@@ -450,3 +450,75 @@ func fpCallersCached(b *testing.B, n int) int {
        }
        return 1 + fpCallersCached(b, n-1)
 }
+
+func TestFPUnwindAfterRecovery(t *testing.T) {
+       if !runtime.FramePointerEnabled {
+               t.Skip("frame pointers not supported for this architecture")
+       }
+       func() {
+               // Make sure that frame pointer unwinding succeeds from a deferred
+               // function run after recovering from a panic. It can fail if the
+               // recovery does not properly restore the caller's frame pointer before
+               // running the remaining deferred functions.
+               //
+               // Wrap this all in an extra function since the unwinding is most likely
+               // to fail trying to unwind *after* the frame we're currently in (since
+               // *that* bp will fail to be restored). Below we'll try to induce a crash,
+               // but if for some reason we can't, let's make sure the stack trace looks
+               // right.
+               want := []string{
+                       "runtime_test.TestFPUnwindAfterRecovery.func1.1",
+                       "runtime_test.TestFPUnwindAfterRecovery.func1",
+                       "runtime_test.TestFPUnwindAfterRecovery",
+               }
+               defer func() {
+                       pcs := make([]uintptr, 32)
+                       for i := range pcs {
+                               // If runtime.recovery doesn't properly restore the
+                               // frame pointer before returning control to this
+                               // function, it will point somewhere lower in the stack
+                               // from one of the frames of runtime.gopanic() or one of
+                               // it's callees prior to recovery.  So, we put some
+                               // non-zero values on the stack to try and get frame
+                               // pointer unwinding to crash if it sees the old,
+                               // invalid frame pointer.
+                               pcs[i] = 10
+                       }
+                       runtime.FPCallers(pcs)
+                       // If it didn't crash, let's symbolize. Something is going
+                       // to look wrong if the bp restoration just happened to
+                       // reference a valid frame. Look for
+                       var got []string
+                       frames := runtime.CallersFrames(pcs)
+                       for {
+                               frame, more := frames.Next()
+                               if !more {
+                                       break
+                               }
+                               got = append(got, frame.Function)
+                       }
+                       // Check that we see the frames in want and in that order.
+                       // This is a bit roundabout because FPCallers doesn't do
+                       // filtering of runtime internals like Callers.
+                       i := 0
+                       for _, f := range got {
+                               if f != want[i] {
+                                       continue
+                               }
+                               i++
+                               if i == len(want) {
+                                       break
+                               }
+                       }
+                       if i != len(want) {
+                               t.Fatalf("bad unwind: got %v, want %v in that order", got, want)
+                       }
+               }()
+               defer func() {
+                       if recover() == nil {
+                               t.Fatal("did not recover from panic")
+                       }
+               }()
+               panic(1)
+       }()
+}

diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 979eb74332148016e845f81fecc0418db9c4aef7..f7ce5033f5879661fba7b3e1a806db1ee34fca73 100644 (file)
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -1921,6 +1921,8 @@ func FPCallers(pcBuf []uintptr) int {
        return fpTracebackPCs(unsafe.Pointer(getfp()), pcBuf)
 }
 
+const FramePointerEnabled = framepointer_enabled
+
 var (
        IsPinned      = isPinned
        GetPinCounter = pinnerGetPinCounter

diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index 64fa27238541db6fa0b9a384990710cd4f7ab2af..39c27a44780477f6c1b29469c97f6cf92c407528 100644 (file)
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -1127,6 +1127,19 @@ func recovery(gp *g) {
        gp.sched.sp = sp
        gp.sched.pc = pc
        gp.sched.lr = 0
+       // 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, the architectural bp is stored 2 words below the
+               // stack pointer.
+               gp.sched.bp = *(*uintptr)(unsafe.Pointer(sp - 2*goarch.PtrSize))
+       case goarch.IsArm64 != 0:
+               // on arm64, the architectural bp points one word higher
+               // than the sp.
+               gp.sched.bp = sp - goarch.PtrSize
+       }
        gp.sched.ret = 1
        gogo(&gp.sched)
 }