diff --git a/src/runtime/extern.go b/src/runtime/extern.go index 66b9527802..2c20e0d8af 100644 --- a/src/runtime/extern.go +++ b/src/runtime/extern.go @@ -178,11 +178,11 @@ func Caller(skip int) (pc uintptr, file string, line int, ok bool) { // We asked for one extra, so skip that one. If this is sigpanic, // stepping over this frame will set up state in Frames so the // next frame is correct. - callers, _, ok = stackExpander.next(callers) + callers, _, ok = stackExpander.next(callers, true) if !ok { return } - _, frame, _ := stackExpander.next(callers) + _, frame, _ := stackExpander.next(callers, true) pc = frame.PC file = frame.File line = frame.Line diff --git a/src/runtime/stack_test.go b/src/runtime/stack_test.go index 8e7c7d47a8..cb0e08256b 100644 --- a/src/runtime/stack_test.go +++ b/src/runtime/stack_test.go @@ -7,6 +7,7 @@ package runtime_test import ( "bytes" "fmt" + "reflect" . "runtime" "strings" "sync" @@ -650,6 +651,8 @@ func (s structWithMethod) stack() string { return string(buf[:Stack(buf, false)]) } +func (s structWithMethod) nop() {} + func TestStackWrapperCaller(t *testing.T) { var d structWithMethod // Force the compiler to construct a wrapper method. @@ -687,6 +690,81 @@ func TestStackWrapperStack(t *testing.T) { } } +type I interface { + M() +} + +func TestStackWrapperStackPanic(t *testing.T) { + t.Run("sigpanic", func(t *testing.T) { + // nil calls to interface methods cause a sigpanic. + testStackWrapperPanic(t, func() { I.M(nil) }, "runtime_test.I.M") + }) + t.Run("panicwrap", func(t *testing.T) { + // Nil calls to value method wrappers call panicwrap. + wrapper := (*structWithMethod).nop + testStackWrapperPanic(t, func() { wrapper(nil) }, "runtime_test.(*structWithMethod).nop") + }) +} + +func testStackWrapperPanic(t *testing.T, cb func(), expect string) { + // Test that the stack trace from a panicking wrapper includes + // the wrapper, even though elide these when they don't panic. + t.Run("CallersFrames", func(t *testing.T) { + defer func() { + err := recover() + if err == nil { + t.Fatalf("expected panic") + } + pcs := make([]uintptr, 10) + n := Callers(0, pcs) + frames := CallersFrames(pcs[:n]) + for { + frame, more := frames.Next() + t.Log(frame.Function) + if frame.Function == expect { + return + } + if !more { + break + } + } + t.Fatalf("panicking wrapper %s missing from stack trace", expect) + }() + cb() + }) + t.Run("Stack", func(t *testing.T) { + defer func() { + err := recover() + if err == nil { + t.Fatalf("expected panic") + } + buf := make([]byte, 4<<10) + stk := string(buf[:Stack(buf, false)]) + if !strings.Contains(stk, "\n"+expect) { + t.Fatalf("panicking wrapper %s missing from stack trace:\n%s", expect, stk) + } + }() + cb() + }) +} + +func TestCallersFromWrapper(t *testing.T) { + // Test that invoking CallersFrames on a stack where the first + // PC is an autogenerated wrapper keeps the wrapper in the + // trace. Normally we elide these, assuming that the wrapper + // calls the thing you actually wanted to see, but in this + // case we need to keep it. + pc := reflect.ValueOf(I.M).Pointer() + frames := CallersFrames([]uintptr{pc}) + frame, more := frames.Next() + if frame.Function != "runtime_test.I.M" { + t.Fatalf("want function %s, got %s", "runtime_test.I.M", frame.Function) + } + if more { + t.Fatalf("want 1 frame, got > 1") + } +} + func TestTracebackSystemstack(t *testing.T) { if GOARCH == "ppc64" || GOARCH == "ppc64le" { t.Skip("systemstack tail call not implemented on ppc64x") diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index 135fc1a7ad..c6c2b89f69 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -19,6 +19,11 @@ type Frames struct { // stackExpander expands callers into a sequence of Frames, // tracking the necessary state across PCs. stackExpander stackExpander + + // elideWrapper indicates that, if the next frame is an + // autogenerated wrapper function, it should be elided from + // the stack. + elideWrapper bool } // Frame is the information returned by Frames for each call frame. @@ -112,11 +117,12 @@ func (se *stackExpander) init(callers []uintptr) []uintptr { // Next returns frame information for the next caller. // If more is false, there are no more callers (the Frame value is valid). func (ci *Frames) Next() (frame Frame, more bool) { - ci.callers, frame, more = ci.stackExpander.next(ci.callers) + ci.callers, frame, more = ci.stackExpander.next(ci.callers, ci.elideWrapper) + ci.elideWrapper = elideWrapperCalling(frame.Function) return } -func (se *stackExpander) next(callers []uintptr) (ncallers []uintptr, frame Frame, more bool) { +func (se *stackExpander) next(callers []uintptr, elideWrapper bool) (ncallers []uintptr, frame Frame, more bool) { ncallers = callers again: if !se.pcExpander.more { @@ -145,7 +151,7 @@ again: } frame = se.pcExpander.next() - if frame.File == "" { + if elideWrapper && frame.File == "" { // Ignore autogenerated functions such as pointer // method forwarding functions. These are an // implementation detail that doesn't reflect the diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 2282b2d5c0..501ecb0411 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -184,6 +184,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in cgoCtxt := gp.cgoCtxt printing := pcbuf == nil && callback == nil _defer := gp._defer + elideWrapper := false for _defer != nil && _defer.sp == _NoArgs { _defer = _defer.link @@ -386,8 +387,15 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } if printing { - // assume skip=0 for printing - if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp, nprint == 0) { + // assume skip=0 for printing. + // + // Never elide wrappers if we haven't printed + // any frames. And don't elide wrappers that + // called panic rather than the wrapped + // function. Otherwise, leave them out. + name := funcname(f) + nextElideWrapper := elideWrapperCalling(name) + if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp, nprint == 0, elideWrapper && nprint != 0) { // Print during crash. // main(0x1, 0x2, 0x3) // /home/rsc/go/src/runtime/x.go:23 +0xf @@ -411,7 +419,6 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in ix = inltree[ix].parent } } - name := funcname(f) if name == "runtime.gopanic" { name = "panic" } @@ -438,6 +445,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in print("\n") nprint++ } + elideWrapper = nextElideWrapper } n++ @@ -647,7 +655,7 @@ func printcreatedby(gp *g) { // Show what created goroutine, except main goroutine (goid 1). pc := gp.gopc f := findfunc(pc) - if f.valid() && showframe(f, gp, false) && gp.goid != 1 { + if f.valid() && showframe(f, gp, false, false) && gp.goid != 1 { print("created by ", funcname(f), "\n") tracepc := pc // back up to CALL instruction for funcline. if pc > f.entry { @@ -727,7 +735,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 funcInfo, gp *g, firstFrame bool) bool { +func showframe(f funcInfo, gp *g, firstFrame, elideWrapper bool) bool { g := getg() if g.m.throwing > 0 && gp != nil && (gp == g.m.curg || gp == g.m.caughtsig.ptr()) { return true @@ -738,8 +746,18 @@ func showframe(f funcInfo, gp *g, firstFrame bool) bool { return true } + if !f.valid() { + return false + } + + if elideWrapper { + file, _ := funcline(f, f.entry) + if file == "" { + return false + } + } + name := funcname(f) - file, _ := funcline(f, f.entry) // Special case: always show runtime.gopanic frame // in the middle of a stack trace, so that we can @@ -750,7 +768,7 @@ func showframe(f funcInfo, gp *g, firstFrame bool) bool { return true } - return f.valid() && contains(name, ".") && (!hasprefix(name, "runtime.") || isExportedRuntime(name)) && file != "" + return contains(name, ".") && (!hasprefix(name, "runtime.") || isExportedRuntime(name)) } // isExportedRuntime reports whether name is an exported runtime function. @@ -760,6 +778,14 @@ func isExportedRuntime(name string) bool { return len(name) > n && name[:n] == "runtime." && 'A' <= name[n] && name[n] <= 'Z' } +// elideWrapperCalling returns whether a wrapper function that called +// function "name" should be elided from stack traces. +func elideWrapperCalling(name string) bool { + // If the wrapper called a panic function instead of the + // wrapped function, we want to include it in stacks. + return !(name == "runtime.gopanic" || name == "runtime.sigpanic" || name == "runtime.panicwrap") +} + var gStatusStrings = [...]string{ _Gidle: "idle", _Grunnable: "runnable",