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>

3 files changed, 89 insertions, 54 deletions

diff --git a/src/cmd/internal/objabi/funcid.go b/src/cmd/internal/objabi/funcid.go
index fc9e421836..1792df7cc1 100644
--- 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 da15ed0680..9b7fafcad7 100644
--- 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
index 0000000000..b3deab4822
--- /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()
+}