Skip to content

Commit b51a4dd

Browse files
nsrip-ddmknyszek
authored andcommitted
runtime: restore caller's frame pointer when recovering from panic
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 <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Felix Geisendörfer <[email protected]>
1 parent 94d36fb commit b51a4dd

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

src/runtime/callers_test.go

+36
Original file line numberDiff line numberDiff line change
@@ -450,3 +450,39 @@ func fpCallersCached(b *testing.B, n int) int {
450450
}
451451
return 1 + fpCallersCached(b, n-1)
452452
}
453+
454+
func TestFPUnwindAfterRecovery(t *testing.T) {
455+
if !runtime.FramePointerEnabled {
456+
t.Skip("frame pointers not supported for this architecture")
457+
}
458+
// Make sure that frame pointer unwinding succeeds from a deferred
459+
// function run after recovering from a panic. It can fail if the
460+
// recovery does not properly restore the caller's frame pointer before
461+
// running the remaining deferred functions.
462+
//
463+
// This test does not verify the accuracy of the call stack (it
464+
// currently includes a frame from runtime.deferreturn which would
465+
// normally be omitted). It is only intended to check that producing the
466+
// call stack won't crash.
467+
defer func() {
468+
pcs := make([]uintptr, 32)
469+
for i := range pcs {
470+
// If runtime.recovery doesn't properly restore the
471+
// frame pointer before returning control to this
472+
// function, it will point somewhere lower in the stack
473+
// from one of the frames of runtime.gopanic() or one of
474+
// it's callees prior to recovery. So, we put some
475+
// non-zero values on the stack to ensure that frame
476+
// pointer unwinding will crash if it sees the old,
477+
// invalid frame pointer.
478+
pcs[i] = 10
479+
}
480+
runtime.FPCallers(pcs)
481+
}()
482+
defer func() {
483+
if recover() == nil {
484+
t.Fatal("did not recover from panic")
485+
}
486+
}()
487+
panic(1)
488+
}

src/runtime/export_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -1922,6 +1922,8 @@ func FPCallers(pcBuf []uintptr) int {
19221922
return fpTracebackPCs(unsafe.Pointer(getfp()), pcBuf)
19231923
}
19241924

1925+
const FramePointerEnabled = framepointer_enabled
1926+
19251927
var (
19261928
IsPinned = isPinned
19271929
GetPinCounter = pinnerGetPinCounter

src/runtime/panic.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ var paniclk mutex
898898
// defers instead.
899899
func recovery(gp *g) {
900900
p := gp._panic
901-
pc, sp := p.retpc, uintptr(p.sp)
901+
pc, sp, fp := p.retpc, uintptr(p.sp), uintptr(p.fp)
902902
p0, saveOpenDeferState := p, p.deferBitsPtr != nil && *p.deferBitsPtr != 0
903903

904904
// Unwind the panic stack.
@@ -990,6 +990,16 @@ func recovery(gp *g) {
990990
gp.sched.sp = sp
991991
gp.sched.pc = pc
992992
gp.sched.lr = 0
993+
// fp points to the stack pointer at the caller, which is the top of the
994+
// stack frame. The frame pointer used for unwinding is the word
995+
// immediately below it.
996+
gp.sched.bp = fp - goarch.PtrSize
997+
if !usesLR {
998+
// on x86, fp actually points one word higher than the top of
999+
// the frame since the return address is saved on the stack by
1000+
// the caller
1001+
gp.sched.bp -= goarch.PtrSize
1002+
}
9931003
gp.sched.ret = 1
9941004
gogo(&gp.sched)
9951005
}

0 commit comments

Comments
 (0)