runtime: proper panic tracebacks with mid-stack inlining

As a followon to CL 152537, modify the panic-printing traceback
to also handle mid-stack inlining correctly.

Also declare -fm functions (aka method functions) as wrappers, so that
they get elided during traceback. This fixes part 2 of #26839.

Fixes #28640
Fixes #24488
Update #26839

Change-Id: I1c535a9b87a9a1ea699621be1e6526877b696c21
Reviewed-on: https://go-review.googlesource.com/c/153477
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/cmd/internal/objabi/funcid.go b/src/cmd/internal/objabi/funcid.go
index fc9e4218369261bfa1e6f9d33da675090666ac77..1792df7cc1d0fc697d16d449d4b7aec2199d9917 100644 (file)
--- a/src/cmd/internal/objabi/funcid.go
+++ b/src/cmd/internal/objabi/funcid.go
@@ -92,5 +92,8 @@ func GetFuncID(name, file string) FuncID {
                        return FuncID_wrapper
                }
        }
+       if strings.HasSuffix(name, "-fm") {
+               return FuncID_wrapper
+       }
        return FuncID_normal
 }

diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index da15ed0680d8666cfa0bc9dfdf5ba9ad3d7fdbbb..9b7fafcad70b37543ea212d16a6f6617de7dafc1 100644 (file)
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -146,7 +146,6 @@ 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
@@ -392,32 +391,39 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                        // any frames. And don't elide wrappers that
                        // called panic rather than the wrapped
                        // function. Otherwise, leave them out.
-                       name := funcname(f)
-                       nextElideWrapper := elideWrapperCalling(f.funcID)
-                       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
-                               //
-                               tracepc := frame.pc // back up to CALL instruction for funcline.
-                               if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic {
-                                       tracepc--
-                               }
-                               file, line := funcline(f, tracepc)
-                               inldata := funcdata(f, _FUNCDATA_InlTree)
-                               if inldata != nil {
-                                       inltree := (*[1 << 20]inlinedCall)(inldata)
+
+                       // backup to CALL instruction to read inlining info (same logic as below)
+                       tracepc := frame.pc
+                       if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic {
+                               tracepc--
+                       }
+                       // If there is inlining info, print the inner frames.
+                       if inldata := funcdata(f, _FUNCDATA_InlTree); inldata != nil {
+                               inltree := (*[1 << 20]inlinedCall)(inldata)
+                               for {
                                        ix := pcdatavalue(f, _PCDATA_InlTreeIndex, tracepc, nil)
-                                       for ix != -1 {
+                                       if ix < 0 {
+                                               break
+                                       }
+                                       if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp, nprint == 0, inltree[ix].funcID, lastFuncID) {
                                                name := funcnameFromNameoff(f, inltree[ix].func_)
+                                               file, line := funcline(f, tracepc)
                                                print(name, "(...)\n")
                                                print("\t", file, ":", line, "\n")
-
-                                               file = funcfile(f, inltree[ix].file)
-                                               line = inltree[ix].line
-                                               ix = int32(inltree[ix].parent)
+                                               nprint++
                                        }
+                                       lastFuncID = inltree[ix].funcID
+                                       // Back up to an instruction in the "caller".
+                                       tracepc = frame.fn.entry + uintptr(inltree[ix].parentPc)
                                }
+                       }
+                       if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp, nprint == 0, f.funcID, lastFuncID) {
+                               // Print during crash.
+                               //      main(0x1, 0x2, 0x3)
+                               //              /home/rsc/go/src/runtime/x.go:23 +0xf
+                               //
+                               name := funcname(f)
+                               file, line := funcline(f, tracepc)
                                if name == "runtime.gopanic" {
                                        name = "panic"
                                }
@@ -444,7 +450,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
                                print("\n")
                                nprint++
                        }
-                       elideWrapper = nextElideWrapper
+                       lastFuncID = f.funcID
                }
                n++
 
@@ -669,7 +675,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, false) && gp.goid != 1 {
+       if f.valid() && showframe(f, gp, false, funcID_normal, funcID_normal) && gp.goid != 1 {
                printcreatedby1(f, pc)
        }
 }
@@ -756,11 +762,10 @@ func traceback1(pc, sp, lr uintptr, gp *g, flags uint) {
 // TODO: Unify this with gentraceback and CallersFrames.
 func printAncestorTraceback(ancestor ancestorInfo) {
        print("[originating from goroutine ", ancestor.goid, "]:\n")
-       elideWrapper := false
        for fidx, pc := range ancestor.pcs {
                f := findfunc(pc) // f previously validated
-               if showfuncinfo(f, fidx == 0, elideWrapper && fidx != 0) {
-                       elideWrapper = printAncestorTracebackFuncInfo(f, pc)
+               if showfuncinfo(f, fidx == 0, funcID_normal, funcID_normal) {
+                       printAncestorTracebackFuncInfo(f, pc)
                }
        }
        if len(ancestor.pcs) == _TracebackMaxFrames {
@@ -768,7 +773,7 @@ func printAncestorTraceback(ancestor ancestorInfo) {
        }
        // Show what created goroutine, except main goroutine (goid 1).
        f := findfunc(ancestor.gopc)
-       if f.valid() && showfuncinfo(f, false, false) && ancestor.goid != 1 {
+       if f.valid() && showfuncinfo(f, false, funcID_normal, funcID_normal) && ancestor.goid != 1 {
                printcreatedby1(f, ancestor.gopc)
        }
 }
@@ -777,27 +782,16 @@ func printAncestorTraceback(ancestor ancestorInfo) {
 // within an ancestor traceback. The precision of this info is reduced
 // due to only have access to the pcs at the time of the caller
 // goroutine being created.
-func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) bool {
-       tracepc := pc // back up to CALL instruction for funcline.
-       if pc > f.entry {
-               tracepc -= sys.PCQuantum
-       }
-       file, line := funcline(f, tracepc)
-       inldata := funcdata(f, _FUNCDATA_InlTree)
-       if inldata != nil {
+func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) {
+       name := funcname(f)
+       if inldata := funcdata(f, _FUNCDATA_InlTree); inldata != nil {
                inltree := (*[1 << 20]inlinedCall)(inldata)
-               ix := pcdatavalue(f, _PCDATA_InlTreeIndex, tracepc, nil)
-               for ix != -1 {
-                       name := funcnameFromNameoff(f, inltree[ix].func_)
-                       print(name, "(...)\n")
-                       print("\t", file, ":", line, "\n")
-
-                       file = funcfile(f, inltree[ix].file)
-                       line = inltree[ix].line
-                       ix = int32(inltree[ix].parent)
+               ix := pcdatavalue(f, _PCDATA_InlTreeIndex, pc, nil)
+               if ix >= 0 {
+                       name = funcnameFromNameoff(f, inltree[ix].func_)
                }
        }
-       name := funcname(f)
+       file, line := funcline(f, pc)
        if name == "runtime.gopanic" {
                name = "panic"
        }
@@ -807,7 +801,6 @@ func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) bool {
                print(" +", hex(pc-f.entry))
        }
        print("\n")
-       return elideWrapperCalling(f.funcID)
 }
 
 func callers(skip int, pcbuf []uintptr) int {
@@ -825,15 +818,19 @@ 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, elideWrapper bool) bool {
+// showframe reports whether the frame with the given characteristics should
+// be printed during a traceback.
+func showframe(f funcInfo, gp *g, firstFrame bool, funcID, childID funcID) bool {
        g := getg()
        if g.m.throwing > 0 && gp != nil && (gp == g.m.curg || gp == g.m.caughtsig.ptr()) {
                return true
        }
-       return showfuncinfo(f, firstFrame, elideWrapper)
+       return showfuncinfo(f, firstFrame, funcID, childID)
 }
 
-func showfuncinfo(f funcInfo, firstFrame, elideWrapper bool) bool {
+// showfuncinfo reports whether a function with the given characteristics should
+// be printed during a traceback.
+func showfuncinfo(f funcInfo, firstFrame bool, funcID, childID funcID) bool {
        level, _, _ := gotraceback()
        if level > 1 {
                // Show all frames.
@@ -844,11 +841,8 @@ func showfuncinfo(f funcInfo, firstFrame, elideWrapper bool) bool {
                return false
        }
 
-       if elideWrapper {
-               file, _ := funcline(f, f.entry)
-               if file == "<autogenerated>" {
-                       return false
-               }
+       if funcID == funcID_wrapper && elideWrapperCalling(childID) {
+               return false
        }
 
        name := funcname(f)

diff --git a/test/fixedbugs/issue24488.go b/test/fixedbugs/issue24488.go
new file mode 100644 (file)
index 0000000..b3deab4
--- /dev/null
+++ b/test/fixedbugs/issue24488.go
@@ -0,0 +1,38 @@
+// run
+
+// Copyright 2018 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
+
+import (
+       "runtime"
+       "strings"
+)
+
+type Func func()
+
+func (f Func) Foo() {
+       if f != nil {
+               f()
+       }
+}
+
+func (f Func) Bar() {
+       if f != nil {
+               f()
+       }
+       buf := make([]byte, 4000)
+       n := runtime.Stack(buf, true)
+       s := string(buf[:n])
+       if strings.Contains(s, "-fm") {
+               panic("wrapper present in stack trace:\n" + s)
+       }
+}
+
+func main() {
+       foo := Func(func() {})
+       foo = foo.Bar
+       foo.Foo()
+}