From c2ae5c7443fc8bda1d2b06390d4b439e81fb4b09 Mon Sep 17 00:00:00 2001 From: David Chase Date: Wed, 19 Feb 2025 16:47:31 -0500 Subject: cmd/compile, runtime: use PC of deferreturn for panic transfer this removes the old conditional-on-register-value handshake from the deferproc/deferprocstack logic. The "line" for the recovery-exit frame itself (not the defers that it runs) is the closing brace of the function. Reduces code size slightly (e.g. go command is 0.2% smaller) Sample output showing effect of this change, also what sort of code it requires to observe the effect: ``` package main import "os" func main() { g(len(os.Args) - 1) // stack[0] } var gi int var pi *int = &gi //go:noinline func g(i int) { switch i { case 0: defer func() { println("g0", i) q() // stack[2] if i == 0 }() for j := *pi; j < 1; j++ { defer func() { println("recover0", recover().(string)) }() } default: for j := *pi; j < 1; j++ { defer func() { println("g1", i) q() // stack[2] if i == 1 }() } defer func() { println("recover1", recover().(string)) }() } p() } // stack[1] (deferreturn) //go:noinline func p() { panic("p()") } //go:noinline func q() { panic("q()") // stack[3] } /* Sample output for "./foo foo": recover1 p() g1 1 panic: q() goroutine 1 [running]: main.q() .../main.go:46 +0x2c main.g.func3() .../main.go:29 +0x48 main.g(0x1?) .../main.go:37 +0x68 main.main() .../main.go:6 +0x28 */ ``` Change-Id: Ie39ea62ecc244213500380ea06d44024cadc2317 Reviewed-on: https://go-review.googlesource.com/c/go/+/650795 Reviewed-by: Cherry Mui LUCI-TryBot-Result: Go LUCI --- src/runtime/panic.go | 48 ++++++++++++++---------------------------------- 1 file changed, 14 insertions(+), 34 deletions(-) (limited to 'src/runtime/panic.go') diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 1ed2503320..c31cfd6e1a 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -285,16 +285,6 @@ func deferproc(fn func()) { // storing it to d.sp because GetCallerSP's result is a // uintptr stack pointer. d.sp = sys.GetCallerSP() - - // deferproc returns 0 normally. - // a deferred func that stops a panic - // makes the deferproc return 1. - // the code the compiler generates always - // checks the return value and jumps to the - // end of the function if deferproc returns != 0. - return0() - // No code can go here - the C return register has - // been set and must not be clobbered. } var rangeDoneError = error(errorString("range function continued iteration after function for loop body returned false")) @@ -391,15 +381,10 @@ func deferrangefunc() any { throw("defer on system stack") } - fn := findfunc(sys.GetCallerPC()) - if fn.deferreturn == 0 { - throw("no deferreturn") - } - d := newdefer() d.link = gp._defer gp._defer = d - d.pc = fn.entry() + uintptr(fn.deferreturn) + d.pc = sys.GetCallerPC() // We must not be preempted between calling GetCallerSP and // storing it to d.sp because GetCallerSP's result is a // uintptr stack pointer. @@ -434,9 +419,6 @@ func deferprocat(fn func(), frame any) { break } } - - // Must be last - see deferproc above. - return0() } // deferconvert converts the rangefunc defer list of d0 into an ordinary list @@ -484,6 +466,7 @@ func deferprocStack(d *_defer) { // go code on the system stack can't defer throw("defer on system stack") } + // fn is already set. // The other fields are junk on entry to deferprocStack and // are initialized here. @@ -506,10 +489,6 @@ func deferprocStack(d *_defer) { *(*uintptr)(unsafe.Pointer(&d.link)) = uintptr(unsafe.Pointer(gp._defer)) *(*uintptr)(unsafe.Pointer(&d.head)) = 0 *(*uintptr)(unsafe.Pointer(&gp._defer)) = uintptr(unsafe.Pointer(d)) - - return0() - // No code can go here - the C return register has - // been set and must not be clobbered. } // Each P holds a pool for defers. @@ -927,9 +906,6 @@ func (p *_panic) nextDefer() (func(), bool) { fn := d.fn - // TODO(mdempsky): Instead of having each deferproc call have - // its own "deferreturn(); return" sequence, we should just make - // them reuse the one we emit for open-coded defers. p.retpc = d.pc // Unlink and free. @@ -1159,6 +1135,15 @@ func recovery(gp *g) { pc, sp, fp := p.retpc, uintptr(p.sp), uintptr(p.fp) p0, saveOpenDeferState := p, p.deferBitsPtr != nil && *p.deferBitsPtr != 0 + // The linker records the f-relative address of a call to deferreturn in f's funcInfo. + // Assuming a "normal" call to recover() inside one of f's deferred functions + // invoked for a panic, that is the desired PC for exiting f. + f := findfunc(pc) + if f.deferreturn == 0 { + throw("no deferreturn") + } + gotoPc := f.entry() + uintptr(f.deferreturn) + // Unwind the panic stack. for ; p != nil && uintptr(p.startSP) < sp; p = p.link { // Don't allow jumping past a pending Goexit. @@ -1181,7 +1166,7 @@ func recovery(gp *g) { // With how subtle defer handling is, this might not actually be // worthwhile though. if p.goexit { - pc, sp = p.startPC, uintptr(p.startSP) + gotoPc, sp = p.startPC, uintptr(p.startSP) saveOpenDeferState = false // goexit is unwinding the stack anyway break } @@ -1242,11 +1227,9 @@ func recovery(gp *g) { throw("bad recovery") } - // Make the deferproc for this d return again, - // this time returning 1. The calling function will - // jump to the standard return epilogue. + // branch directly to the deferreturn gp.sched.sp = sp - gp.sched.pc = pc + gp.sched.pc = gotoPc 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 @@ -1263,9 +1246,6 @@ func recovery(gp *g) { // only gets us to the caller's fp. gp.sched.bp = sp - goarch.PtrSize } - // The value in ret is delivered IN A REGISTER, even if there is a - // stack ABI. - gp.sched.ret = 1 gogo(&gp.sched) } -- cgit v1.3-5-g9baa