From 08376e1a9c57d4b07a77d156810d3fec46caba79 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 1 Jul 2025 15:29:12 -0700 Subject: [PATCH] runtime: iterate through inlinings when processing recover() We care about the wrapper-ness of logical frames, not physical frames. Fixes #73916 Fixes #73917 Fixex #73920 Change-Id: Ia17c8390e71e6c0e13e23dcbb7bc7273ef25da90 Reviewed-on: https://go-review.googlesource.com/c/go/+/685375 Reviewed-by: Keith Randall Reviewed-by: Cuong Manh Le LUCI-TryBot-Result: Go LUCI Reviewed-by: Cherry Mui --- src/runtime/panic.go | 25 ++++++++++++---------- test/fixedbugs/issue73916.go | 32 ++++++++++++++++++++++++++++ test/fixedbugs/issue73916b.go | 34 +++++++++++++++++++++++++++++ test/fixedbugs/issue73917.go | 40 +++++++++++++++++++++++++++++++++++ test/fixedbugs/issue73920.go | 40 +++++++++++++++++++++++++++++++++++ 5 files changed, 160 insertions(+), 11 deletions(-) create mode 100644 test/fixedbugs/issue73916.go create mode 100644 test/fixedbugs/issue73916b.go create mode 100644 test/fixedbugs/issue73917.go create mode 100644 test/fixedbugs/issue73920.go diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 1e61c90aef..bf79b6518e 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -1142,18 +1142,21 @@ func gorecover(_ uintptr) any { nonWrapperFrames := 0 loop: for ; u.valid(); u.next() { - switch u.frame.fn.funcID { - case abi.FuncIDWrapper: - continue - case abi.FuncID_gopanic: - if u.frame.fp == uintptr(p.gopanicFP) && nonWrapperFrames > 0 { - canRecover = true - } - break loop - default: - nonWrapperFrames++ - if nonWrapperFrames > 1 { + for iu, f := newInlineUnwinder(u.frame.fn, u.symPC()); f.valid(); f = iu.next(f) { + sf := iu.srcFunc(f) + switch sf.funcID { + case abi.FuncIDWrapper: + continue + case abi.FuncID_gopanic: + if u.frame.fp == uintptr(p.gopanicFP) && nonWrapperFrames > 0 { + canRecover = true + } break loop + default: + nonWrapperFrames++ + if nonWrapperFrames > 1 { + break loop + } } } } diff --git a/test/fixedbugs/issue73916.go b/test/fixedbugs/issue73916.go new file mode 100644 index 0000000000..6060c3f235 --- /dev/null +++ b/test/fixedbugs/issue73916.go @@ -0,0 +1,32 @@ +// run + +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +func callRecover() { + if recover() != nil { + println("recovered") + } +} + +func F(int) { callRecover() } + +func main() { + mustPanic(func() { + defer F(1) + panic("XXX") + }) +} + +func mustPanic(f func()) { + defer func() { + r := recover() + if r == nil { + panic("didn't panic") + } + }() + f() +} diff --git a/test/fixedbugs/issue73916b.go b/test/fixedbugs/issue73916b.go new file mode 100644 index 0000000000..29393fa502 --- /dev/null +++ b/test/fixedbugs/issue73916b.go @@ -0,0 +1,34 @@ +// run + +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +func callRecover() { + func() { + if recover() != nil { + println("recovered") + } + }() +} + +func F() int { callRecover(); return 0 } + +func main() { + mustPanic(func() { + defer F() + panic("XXX") + }) +} + +func mustPanic(f func()) { + defer func() { + r := recover() + if r == nil { + panic("didn't panic") + } + }() + f() +} diff --git a/test/fixedbugs/issue73917.go b/test/fixedbugs/issue73917.go new file mode 100644 index 0000000000..9c0330d19a --- /dev/null +++ b/test/fixedbugs/issue73917.go @@ -0,0 +1,40 @@ +// run + +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +func callRecover() { + if recover() != nil { + println("recovered") + } +} + +type T int + +func (*T) M() { callRecover() } + +type S struct{ *T } // has a wrapper S.M wrapping (*T.M) + +var p = S{new(T)} + +var fn = S.M // using a function pointer to force using the wrapper + +func main() { + mustPanic(func() { + defer fn(p) + panic("XXX") + }) +} + +func mustPanic(f func()) { + defer func() { + r := recover() + if r == nil { + panic("didn't panic") + } + }() + f() +} diff --git a/test/fixedbugs/issue73920.go b/test/fixedbugs/issue73920.go new file mode 100644 index 0000000000..f0a711b045 --- /dev/null +++ b/test/fixedbugs/issue73920.go @@ -0,0 +1,40 @@ +// run + +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +func callRecover() { + if recover() != nil { + println("recovered") + } +} + +type T int + +func (*T) M() { callRecover() } + +type S struct{ *T } // has a wrapper (*S).M wrapping (*T.M) + +var p = &S{new(T)} + +var fn = (*S).M // using a function pointer to force using the wrapper + +func main() { + mustPanic(func() { + defer fn(p) + panic("XXX") + }) +} + +func mustPanic(f func()) { + defer func() { + r := recover() + if r == nil { + panic("didn't panic") + } + }() + f() +} -- 2.51.0