Skip to content

Commit 22d28a2

Browse files
committed
runtime: force segv for nil defer function to be in deferreturn()
If the defer function pointer is nil, force the seg fault to happen in deferreturn rather than in jmpdefer. jmpdefer is used fairly infrequently now because most functions have open-coded defers. The open-coded defer implementation calls gentraceback() with a callback when looking for the first open-coded defer frame. gentraceback() throws an error if it is called with a callback on an LR architecture and jmpdefer is on the stack, because the stack trace can be incorrect in that case - see issue #8153. So, we want to make sure that we don't have a seg fault in jmpdefer. Fixes #36050 Change-Id: Ie25e6f015d8eb170b40248dedeb26a37b7f9b38d Reviewed-on: https://go-review.googlesource.com/c/go/+/210978 Reviewed-by: Keith Randall <[email protected]> Run-TryBot: Dan Scales <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 100bf44 commit 22d28a2

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

src/runtime/callers_test.go

+31-3
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,8 @@ func TestCallersDivZeroPanic(t *testing.T) {
254254
func TestCallersDeferNilFuncPanic(t *testing.T) {
255255
// Make sure we don't have any extra frames on the stack. We cut off the check
256256
// at runtime.sigpanic, because non-open-coded defers (which may be used in
257-
// non-opt or race checker mode) include an extra 'jmpdefer' frame (which is
258-
// where the nil pointer deref happens). We could consider hiding jmpdefer in
259-
// tracebacks.
257+
// non-opt or race checker mode) include an extra 'deferreturn' frame (which is
258+
// where the nil pointer deref happens).
260259
state := 1
261260
want := []string{"runtime.Callers", "runtime_test.TestCallersDeferNilFuncPanic.func1",
262261
"runtime.gopanic", "runtime.panicmem", "runtime.sigpanic"}
@@ -279,3 +278,32 @@ func TestCallersDeferNilFuncPanic(t *testing.T) {
279278
// function exit, rather than at the defer statement.
280279
state = 2
281280
}
281+
282+
// Same test, but forcing non-open-coded defer by putting the defer in a loop. See
283+
// issue #36050
284+
func TestCallersDeferNilFuncPanicWithLoop(t *testing.T) {
285+
state := 1
286+
want := []string{"runtime.Callers", "runtime_test.TestCallersDeferNilFuncPanicWithLoop.func1",
287+
"runtime.gopanic", "runtime.panicmem", "runtime.sigpanic", "runtime.deferreturn", "runtime_test.TestCallersDeferNilFuncPanicWithLoop"}
288+
289+
defer func() {
290+
if r := recover(); r == nil {
291+
t.Fatal("did not panic")
292+
}
293+
pcs := make([]uintptr, 20)
294+
pcs = pcs[:runtime.Callers(0, pcs)]
295+
testCallersEqual(t, pcs, want)
296+
if state == 1 {
297+
t.Fatal("nil defer func panicked at defer time rather than function exit time")
298+
}
299+
300+
}()
301+
302+
for i := 0; i < 1; i++ {
303+
var f func()
304+
defer f()
305+
}
306+
// Use the value of 'state' to make sure nil defer func f causes panic at
307+
// function exit, rather than at the defer statement.
308+
state = 2
309+
}

src/runtime/panic.go

+6
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,12 @@ func deferreturn(arg0 uintptr) {
561561
d.fn = nil
562562
gp._defer = d.link
563563
freedefer(d)
564+
// If the defer function pointer is nil, force the seg fault to happen
565+
// here rather than in jmpdefer. gentraceback() throws an error if it is
566+
// called with a callback on an LR architecture and jmpdefer is on the
567+
// stack, because the stack trace can be incorrect in that case - see
568+
// issue #8153).
569+
_ = fn.fn
564570
jmpdefer(fn, uintptr(unsafe.Pointer(&arg0)))
565571
}
566572

0 commit comments

Comments
 (0)