]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: restore caller's frame pointer when recovering from panic
authorNick Ripley <nick.ripley@datadoghq.com>
Fri, 4 Aug 2023 21:31:43 +0000 (17:31 -0400)
committerMichael Knyszek <mknyszek@google.com>
Tue, 15 Aug 2023 14:52:21 +0000 (14:52 +0000)
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.

Fixes #61766

Change-Id: I45f41aedcc397133560164ab520ca638bbd93c4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/516157
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
src/runtime/callers_test.go
src/runtime/export_test.go
src/runtime/panic.go

index 7e2c6c82385312cbabea3e03b284c532e79efe86..42091b04fca24c73f70e1a5f445760feabe7e75d 100644 (file)
@@ -450,3 +450,39 @@ 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")
+       }
+       // 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.
+       //
+       // This test does not verify the accuracy of the call stack (it
+       // currently includes a frame from runtime.deferreturn which would
+       // normally be omitted). It is only intended to check that producing the
+       // call stack won't crash.
+       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 ensure that frame
+                       // pointer unwinding will crash if it sees the old,
+                       // invalid frame pointer.
+                       pcs[i] = 10
+               }
+               runtime.FPCallers(pcs)
+       }()
+       defer func() {
+               if recover() == nil {
+                       t.Fatal("did not recover from panic")
+               }
+       }()
+       panic(1)
+}
index bc08b2333c25a0b47d469af0355dc23ef012c164..a89220e0dd0a5986214cef4623433a3390337595 100644 (file)
@@ -1922,6 +1922,8 @@ func FPCallers(pcBuf []uintptr) int {
        return fpTracebackPCs(unsafe.Pointer(getfp()), pcBuf)
 }
 
+const FramePointerEnabled = framepointer_enabled
+
 var (
        IsPinned      = isPinned
        GetPinCounter = pinnerGetPinCounter
index 5b7f35a0a503052ab7c389f19cfe5a533bcff599..59241143d0ec99c6b692684b9bca7637c588f7f5 100644 (file)
@@ -898,7 +898,7 @@ var paniclk mutex
 // defers instead.
 func recovery(gp *g) {
        p := gp._panic
-       pc, sp := p.retpc, uintptr(p.sp)
+       pc, sp, fp := p.retpc, uintptr(p.sp), uintptr(p.fp)
        p0, saveOpenDeferState := p, p.deferBitsPtr != nil && *p.deferBitsPtr != 0
 
        // Unwind the panic stack.
@@ -990,6 +990,16 @@ 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 {
+               // 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.ret = 1
        gogo(&gp.sched)
 }