From f9feaffdf51967bc5cf3c9363db9ddee98c9b3a0 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sat, 12 Nov 2016 23:01:37 -0500 Subject: [PATCH] runtime: do not print runtime panic frame at top of user stack The expected default behavior (no explicit GOTRACEBACK setting) is for the stack trace to start in user code, eliding unnecessary runtime frames that led up to the actual trace printing code. The idea was that the first line number printed was the one that crashed. For #5832 we added code to show 'panic' frames so that if code panics and then starts running defers and then we trace from there, the panic frame can help explain why the code seems to have made a call not present in the code. But that's only needed for panics between two different call frames, not the panic at the very top of the stack trace. Fix the fix to again elide the runtime code at the very top of the stack trace. Simple panic: package main func main() { var x []int println(x[1]) } Before this CL: panic: runtime error: index out of range goroutine 1 [running]: panic(0x1056980, 0x1091bf0) /Users/rsc/go/src/runtime/panic.go:531 +0x1cf main.main() /tmp/x.go:5 +0x5 After this CL: panic: runtime error: index out of range goroutine 1 [running]: main.main() /tmp/x.go:5 +0x5 Panic inside defer triggered by panic: package main func main() { var x []int defer func() { println(x[1]) }() println(x[2]) } Before this CL: panic: runtime error: index out of range panic: runtime error: index out of range goroutine 1 [running]: panic(0x1056aa0, 0x1091bf0) /Users/rsc/go/src/runtime/panic.go:531 +0x1cf main.main.func1(0x0, 0x0, 0x0) /tmp/y.go:6 +0x62 panic(0x1056aa0, 0x1091bf0) /Users/rsc/go/src/runtime/panic.go:489 +0x2cf main.main() /tmp/y.go:8 +0x59 The middle panic is important: it explains why main.main ended up calling main.main.func1 on a line that looks like a call to println. The top panic is noise. After this CL: panic: runtime error: index out of range panic: runtime error: index out of range goroutine 1 [running]: main.main.func1(0x0, 0x0, 0x0) /tmp/y.go:6 +0x62 panic(0x1056ac0, 0x1091bf0) /Users/rsc/go/src/runtime/panic.go:489 +0x2cf main.main() /tmp/y.go:8 +0x59 Fixes #17901. Change-Id: Id6d7c76373f7a658a537a39ca32b7dc23e1e76aa Reviewed-on: https://go-review.googlesource.com/33165 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor Reviewed-by: Brad Fitzpatrick --- src/runtime/crash_test.go | 2 +- src/runtime/traceback.go | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 1f7aa521e1..9ec0ae468b 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -418,7 +418,7 @@ func TestPanicTraceback(t *testing.T) { } // Check functions in the traceback. - fns := []string{"panic", "main.pt1.func1", "panic", "main.pt2.func1", "panic", "main.pt2", "main.pt1"} + fns := []string{"main.pt1.func1", "panic", "main.pt2.func1", "panic", "main.pt2", "main.pt1"} for _, fn := range fns { re := regexp.MustCompile(`(?m)^` + regexp.QuoteMeta(fn) + `\(.*\n`) idx := re.FindStringIndex(output) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index c2bd90898c..0049e82d63 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -380,7 +380,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } } if printing { - if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp) { + if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp, nprint == 0) { // Print during crash. // main(0x1, 0x2, 0x3) // /home/rsc/go/src/runtime/x.go:23 +0xf @@ -632,7 +632,7 @@ func printcreatedby(gp *g) { // Show what created goroutine, except main goroutine (goid 1). pc := gp.gopc f := findfunc(pc) - if f != nil && showframe(f, gp) && gp.goid != 1 { + if f != nil && showframe(f, gp, false) && gp.goid != 1 { print("created by ", funcname(f), "\n") tracepc := pc // back up to CALL instruction for funcline. if pc > f.entry { @@ -712,7 +712,7 @@ func gcallers(gp *g, skip int, pcbuf []uintptr) int { return gentraceback(^uintptr(0), ^uintptr(0), 0, gp, skip, &pcbuf[0], len(pcbuf), nil, nil, 0) } -func showframe(f *_func, gp *g) bool { +func showframe(f *_func, gp *g, firstFrame bool) bool { g := getg() if g.m.throwing > 0 && gp != nil && (gp == g.m.curg || gp == g.m.caughtsig.ptr()) { return true @@ -720,10 +720,12 @@ func showframe(f *_func, gp *g) bool { level, _, _ := gotraceback() name := funcname(f) - // Special case: always show runtime.gopanic frame, so that we can - // see where a panic started in the middle of a stack trace. + // Special case: always show runtime.gopanic frame + // in the middle of a stack trace, so that we can + // see the boundary between ordinary code and + // panic-induced deferred code. // See golang.org/issue/5832. - if name == "runtime.gopanic" { + if name == "runtime.gopanic" && !firstFrame { return true } -- 2.50.0