Skip to content

Commit 8dc6ad1

Browse files
nsrip-ddgopherbot
authored andcommitted
[release-branch.go1.21] 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. 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 <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
1 parent 06df329 commit 8dc6ad1

File tree

3 files changed

+87
-0
lines changed

3 files changed

+87
-0
lines changed

src/runtime/callers_test.go

+72
Original file line numberDiff line numberDiff line change
@@ -450,3 +450,75 @@ 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+
func() {
459+
// Make sure that frame pointer unwinding succeeds from a deferred
460+
// function run after recovering from a panic. It can fail if the
461+
// recovery does not properly restore the caller's frame pointer before
462+
// running the remaining deferred functions.
463+
//
464+
// Wrap this all in an extra function since the unwinding is most likely
465+
// to fail trying to unwind *after* the frame we're currently in (since
466+
// *that* bp will fail to be restored). Below we'll try to induce a crash,
467+
// but if for some reason we can't, let's make sure the stack trace looks
468+
// right.
469+
want := []string{
470+
"runtime_test.TestFPUnwindAfterRecovery.func1.1",
471+
"runtime_test.TestFPUnwindAfterRecovery.func1",
472+
"runtime_test.TestFPUnwindAfterRecovery",
473+
}
474+
defer func() {
475+
pcs := make([]uintptr, 32)
476+
for i := range pcs {
477+
// If runtime.recovery doesn't properly restore the
478+
// frame pointer before returning control to this
479+
// function, it will point somewhere lower in the stack
480+
// from one of the frames of runtime.gopanic() or one of
481+
// it's callees prior to recovery. So, we put some
482+
// non-zero values on the stack to try and get frame
483+
// pointer unwinding to crash if it sees the old,
484+
// invalid frame pointer.
485+
pcs[i] = 10
486+
}
487+
runtime.FPCallers(pcs)
488+
// If it didn't crash, let's symbolize. Something is going
489+
// to look wrong if the bp restoration just happened to
490+
// reference a valid frame. Look for
491+
var got []string
492+
frames := runtime.CallersFrames(pcs)
493+
for {
494+
frame, more := frames.Next()
495+
if !more {
496+
break
497+
}
498+
got = append(got, frame.Function)
499+
}
500+
// Check that we see the frames in want and in that order.
501+
// This is a bit roundabout because FPCallers doesn't do
502+
// filtering of runtime internals like Callers.
503+
i := 0
504+
for _, f := range got {
505+
if f != want[i] {
506+
continue
507+
}
508+
i++
509+
if i == len(want) {
510+
break
511+
}
512+
}
513+
if i != len(want) {
514+
t.Fatalf("bad unwind: got %v, want %v in that order", got, want)
515+
}
516+
}()
517+
defer func() {
518+
if recover() == nil {
519+
t.Fatal("did not recover from panic")
520+
}
521+
}()
522+
panic(1)
523+
}()
524+
}

src/runtime/export_test.go

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

1924+
const FramePointerEnabled = framepointer_enabled
1925+
19241926
var (
19251927
IsPinned = isPinned
19261928
GetPinCounter = pinnerGetPinCounter

src/runtime/panic.go

+13
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,19 @@ func recovery(gp *g) {
11271127
gp.sched.sp = sp
11281128
gp.sched.pc = pc
11291129
gp.sched.lr = 0
1130+
// Restore the bp on platforms that support frame pointers.
1131+
// N.B. It's fine to not set anything for platforms that don't
1132+
// support frame pointers, since nothing consumes them.
1133+
switch {
1134+
case goarch.IsAmd64 != 0:
1135+
// On x86, the architectural bp is stored 2 words below the
1136+
// stack pointer.
1137+
gp.sched.bp = *(*uintptr)(unsafe.Pointer(sp - 2*goarch.PtrSize))
1138+
case goarch.IsArm64 != 0:
1139+
// on arm64, the architectural bp points one word higher
1140+
// than the sp.
1141+
gp.sched.bp = sp - goarch.PtrSize
1142+
}
11301143
gp.sched.ret = 1
11311144
gogo(&gp.sched)
11321145
}

0 commit comments

Comments
 (0)