diff options
| author | Keith Randall <khr@golang.org> | 2026-01-12 15:05:24 -0800 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2026-03-06 10:08:12 -0800 |
| commit | 50d988e4e037d9d41ac223a62706dfea47a100e4 (patch) | |
| tree | cccebaec15e091c18a1da50a0c0253c9809642be /src | |
| parent | bf84b002d64d0b150818268e520fee0172a5c462 (diff) | |
| download | go-50d988e4e037d9d41ac223a62706dfea47a100e4.tar.xz | |
runtime: when panicking, skip ahead to previous panic
While looking up the stack for a defer to run, if we come across
a panic frame we can skip ahead (up) to where the previous panic
was looking for a defer to run.
Switch from keeping LR (the caller's pc) to PC (the frame's PC).
Seems easier to reason about.
Fixes #77062
Change-Id: Idb39411ebad8c072c8f65c62a518da848bddbd61
Reviewed-on: https://go-review.googlesource.com/c/go/+/738041
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Diffstat (limited to 'src')
| -rw-r--r-- | src/runtime/panic.go | 95 | ||||
| -rw-r--r-- | src/runtime/panic_test.go | 50 | ||||
| -rw-r--r-- | src/runtime/runtime2.go | 5 |
3 files changed, 143 insertions, 7 deletions
diff --git a/src/runtime/panic.go b/src/runtime/panic.go index d467e9305d..1429279ea2 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -920,7 +920,7 @@ func (p *_panic) start(pc uintptr, sp unsafe.Pointer) { // caller instead, we avoid needing to unwind through an extra // frame. It also somewhat simplifies the terminating condition for // deferreturn. - p.lr, p.fp = pc, sp + p.pc, p.sp = pc, sp p.nextFrame() } @@ -995,7 +995,7 @@ func (p *_panic) nextDefer() (func(), bool) { // nextFrame finds the next frame that contains deferred calls, if any. func (p *_panic) nextFrame() (ok bool) { - if p.lr == 0 { + if p.pc == 0 { return false } @@ -1007,10 +1007,10 @@ func (p *_panic) nextFrame() (ok bool) { } var u unwinder - u.initAt(p.lr, uintptr(p.fp), 0, gp, 0) + u.initAt(p.pc, uintptr(p.sp), 0, gp, 0) for { if !u.valid() { - p.lr = 0 + p.pc = 0 return // ok == false } @@ -1027,10 +1027,25 @@ func (p *_panic) nextFrame() (ok bool) { break // found a frame with open-coded defers } + if p.link != nil && uintptr(u.frame.sp) == uintptr(p.link.startSP) && uintptr(p.link.sp) > u.frame.sp { + // Skip ahead to where the next panic up the stack was last looking + // for defers. See issue 77062. + // + // The startSP condition is to check when we have walked up the stack + // to where the next panic up the stack started. If so, the processing + // of that panic has run all the defers up to its current scanning + // position. + // + // The final condition is just to make sure that the line below + // is actually helpful. + u.initAt(p.link.pc, uintptr(p.link.sp), 0, gp, 0) + continue + } + u.next() } - p.lr = u.frame.lr + p.pc = u.frame.pc p.sp = unsafe.Pointer(u.frame.sp) p.fp = unsafe.Pointer(u.frame.fp) @@ -1702,3 +1717,73 @@ func isAbortPC(pc uintptr) bool { } return f.funcID == abi.FuncID_abort } + +// For debugging only. +//go:noinline +//go:nosplit +func dumpPanicDeferState(where string, gp *g) { + systemstack(func() { + println("DUMPPANICDEFERSTATE", where) + p := gp._panic + d := gp._defer + var u unwinder + for u.init(gp, 0); u.valid(); u.next() { + // Print frame. + println(" frame sp=", hex(u.frame.sp), "fp=", hex(u.frame.fp), "pc=", pcName(u.frame.pc), "+", pcOff(u.frame.pc)) + // Print panic. + for p != nil && uintptr(p.sp) == u.frame.sp { + println(" panic", p, "sp=", p.sp, "fp=", p.fp, "arg=", p.arg, "recovered=", p.recovered, "pc=", pcName(p.pc), "+", pcOff(p.pc), "retpc=", pcName(p.retpc), "+", pcOff(p.retpc), "startsp=", p.startSP, "gopanicfp=", p.gopanicFP, "startPC=", hex(p.startPC), pcName(p.startPC), "+", pcOff(p.startPC)) + p = p.link + } + + // Print linked defers. + for d != nil && d.sp == u.frame.sp { + println(" defer(link)", "heap=", d.heap, "rangefunc=", d.rangefunc, fnName(d.fn)) + d = d.link + } + + // Print open-coded defers. + // (A function is all linked or all open-coded, so we don't + // need to interleave this loop with the one above.) + fd := funcdata(u.frame.fn, abi.FUNCDATA_OpenCodedDeferInfo) + if fd != nil { + deferBitsOffset, fd := readvarintUnsafe(fd) + m := *(*uint8)(unsafe.Pointer(u.frame.varp - uintptr(deferBitsOffset))) + slotsOffset, fd := readvarintUnsafe(fd) + slots := u.frame.varp - uintptr(slotsOffset) + for i := 7; i >= 0; i-- { + if m>>i&1 == 0 { + continue + } + fn := *(*func())(unsafe.Pointer(slots + uintptr(i)*goarch.PtrSize)) + println(" defer(open)", fnName(fn)) + } + } + + } + if p != nil { + println(" REMAINING PANICS!", p) + } + if d != nil { + println(" REMAINING DEFERS!") + } + }) +} + +func pcName(pc uintptr) string { + fn := findfunc(pc) + if !fn.valid() { + return "<unk>" + } + return funcname(fn) +} +func pcOff(pc uintptr) hex { + fn := findfunc(pc) + if !fn.valid() { + return 0 + } + return hex(pc - fn.entry()) +} +func fnName(fn func()) string { + return pcName(**(**uintptr)(unsafe.Pointer(&fn))) +} diff --git a/src/runtime/panic_test.go b/src/runtime/panic_test.go index 2b06bce45d..5a4b6ae633 100644 --- a/src/runtime/panic_test.go +++ b/src/runtime/panic_test.go @@ -5,8 +5,10 @@ package runtime_test import ( + "slices" "strings" "testing" + "time" ) // Test that panics print out the underlying value @@ -48,3 +50,51 @@ func TestPanicWithDirectlyPrintableCustomTypes(t *testing.T) { }) } } + +func TestPanicRecoverSpeed(t *testing.T) { + // For issue 77062. + t.Skip("This test is too flaky at the moment. But it does normally pass. Suggestions for making it less flaky are welcome.") + + // Recursive function that does defer/recover/repanic. + var f func(int) + f = func(n int) { + if n == 0 { + panic("done") + } + defer func() { + err := recover() + panic(err) + }() + f(n - 1) + } + + time := func(f func()) time.Duration { + var times []time.Duration + for range 10 { + start := time.Now() + f() + times = append(times, time.Since(start)) + } + slices.Sort(times) + times = times[1 : len(times)-1] // skip high and low, to reduce noise + var avg time.Duration + for _, v := range times { + avg += v / time.Duration(len(times)) + } + return avg + } + + a := time(func() { + defer func() { recover() }() + f(1024) + }) + b := time(func() { + defer func() { recover() }() + f(2048) + }) + m := b.Seconds() / a.Seconds() + t.Logf("a: %v, b: %v, m: %v", a, b, m) + if m > 3.5 { + t.Errorf("more than 2x time increase: %v", m) + } +} diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 95dc834717..9fec6c6a1e 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -1177,16 +1177,17 @@ type _panic struct { link *_panic // link to earlier panic // startPC and startSP track where _panic.start was called. + // (These are the SP and PC of the gopanic frame itself.) startPC uintptr startSP unsafe.Pointer // The current stack frame that we're running deferred calls for. + pc uintptr sp unsafe.Pointer - lr uintptr fp unsafe.Pointer // retpc stores the PC where the panic should jump back to, if the - // function last returned by _panic.next() recovers the panic. + // function last returned by _panic.nextDefer() recovers the panic. retpc uintptr // Extra state for handling open-coded defers. |
