proc: skip autogenerated wrappers when stepping in and out

Under some circumstances (methods with non-pointer receivers or from
embedded fields called through an interface) the compiler will
autogenerate wrapper functions.

This commit changes next, step and stepout to skip all autogenerated
wrappers.

Fixes #1908

_fixtures/ifaceembcall.go

@@ -0,0 +1,31 @@
+package main
+
+import "fmt"
+
+type A struct {
+	a int
+}
+
+type B struct {
+	*A
+}
+
+type Iface interface {
+	PtrReceiver() string
+	NonPtrReceiver() string
+}
+
+func (*A) PtrReceiver() string {
+	return "blah"
+}
+
+func (A) NonPtrReceiver() string {
+	return "blah"
+}
+
+func main() {
+	var iface Iface = &B{&A{1}}
+	s := iface.PtrReceiver()
+	s = iface.NonPtrReceiver()
+	fmt.Printf("%s\n", s)
+}

pkg/proc/arm64_disasm.go

@@ -26,6 +26,8 @@ func arm64AsmDecode(asmInst *AsmInstruction, mem []byte, regs Registers, memrw M
 		asmInst.Kind = CallInstruction
 	case arm64asm.RET, arm64asm.ERET:
 		asmInst.Kind = RetInstruction
+	case arm64asm.B, arm64asm.BR:
+		asmInst.Kind = JmpInstruction
 	}
 
 	asmInst.DestLoc = resolveCallArgARM64(&inst, asmInst.Loc.PC, asmInst.AtPC, regs, memrw, bi)
@@ -34,7 +36,10 @@ func arm64AsmDecode(asmInst *AsmInstruction, mem []byte, regs Registers, memrw M
 }
 
 func resolveCallArgARM64(inst *arm64asm.Inst, instAddr uint64, currentGoroutine bool, regs Registers, mem MemoryReadWriter, bininfo *BinaryInfo) *Location {
-	if inst.Op != arm64asm.BL && inst.Op != arm64asm.BLR {
+	switch inst.Op {
+	case arm64asm.BL, arm64asm.BLR, arm64asm.B, arm64asm.BR:
+		//ok
+	default:
 		return nil
 	}
 

pkg/proc/bininfo.go

@@ -371,6 +371,22 @@ func (fn *Function) PrologueEndPC() uint64 {
 	return pc
 }
 
+// From $GOROOT/src/runtime/traceback.go:597
+// exportedRuntime reports whether the function is an exported runtime function.
+// It is only for runtime functions, so ASCII A-Z is fine.
+func (fn *Function) exportedRuntime() bool {
+	name := fn.Name
+	const n = len("runtime.")
+	return len(name) > n && name[:n] == "runtime." && 'A' <= name[n] && name[n] <= 'Z'
+}
+
+// unexportedRuntime reports whether the function is a private runtime function.
+func (fn *Function) privateRuntime() bool {
+	name := fn.Name
+	const n = len("runtime.")
+	return len(name) > n && name[:n] == "runtime." && !('A' <= name[n] && name[n] <= 'Z')
+}
+
 type constantsMap map[dwarfRef]*constantType
 
 type constantType struct {

pkg/proc/disasm.go

@@ -22,16 +22,24 @@ const (
 	OtherInstruction AsmInstructionKind = iota
 	CallInstruction
 	RetInstruction
+	JmpInstruction
 )
 
+// IsCall is true if instr is a call instruction.
 func (instr *AsmInstruction) IsCall() bool {
 	return instr.Kind == CallInstruction
 }
 
+// IsRet is true if instr is a return instruction.
 func (instr *AsmInstruction) IsRet() bool {
 	return instr.Kind == RetInstruction
 }
 
+// IsJmp is true if instr is an unconditional jump instruction.
+func (instr *AsmInstruction) IsJmp() bool {
+	return instr.Kind == JmpInstruction
+}
+
 type archInst interface {
 	Text(flavour AssemblyFlavour, pc uint64, symLookup func(uint64) (string, uint64)) string
 	OpcodeEquals(op uint64) bool

pkg/proc/proc_test.go

@@ -4758,3 +4758,49 @@ func TestIssue1925(t *testing.T) {
 		}
 	})
 }
+
+func TestStepIntoWrapperForEmbeddedPointer(t *testing.T) {
+	if runtime.GOOS == "linux" && runtime.GOARCH == "386" && buildMode == "pie" {
+		t.Skip("Skipping wrappers doesn't work on linux/386/PIE due to the use of get_pc_thunk")
+	}
+	// Under some circumstances (when using an interface to call a method on an
+	// embedded field, see _fixtures/ifaceembcall.go) the compiler will
+	// autogenerate a wrapper function that uses a tail call (i.e. it ends in
+	// an unconditional jump instruction to a different function).
+	// Delve should be able to step into this tail call.
+	testseq2(t, "ifaceembcall", "", []seqTest{
+		{contContinue, 28}, // main.main, the line calling iface.PtrReceiver()
+		{contStep, 18},     // main.(*A).PtrReceiver
+		{contStep, 19},
+		{contStepout, 28},
+		{contContinueToBreakpoint, 29}, // main.main, the line calling iface.NonPtrReceiver()
+		{contStep, 22},                 // main.(A).NonPtrReceiver
+		{contStep, 23},
+		{contStepout, 29}})
+
+	// same test but with next instead of stepout
+	if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) && runtime.GOARCH != "386" {
+		testseq2(t, "ifaceembcall", "", []seqTest{
+			{contContinue, 28}, // main.main, the line calling iface.PtrReceiver()
+			{contStep, 18},     // main.(*A).PtrReceiver
+			{contNext, 19},
+			{contNext, 19},
+			{contNext, 28},
+			{contContinueToBreakpoint, 29}, // main.main, the line calling iface.NonPtrReceiver()
+			{contStep, 22},
+			{contNext, 23},
+			{contNext, 23},
+			{contNext, 29}})
+	} else {
+		testseq2(t, "ifaceembcall", "", []seqTest{
+			{contContinue, 28}, // main.main, the line calling iface.PtrReceiver()
+			{contStep, 18},     // main.(*A).PtrReceiver
+			{contNext, 19},
+			{contNext, 28},
+			{contContinueToBreakpoint, 29}, // main.main, the line calling iface.NonPtrReceiver()
+			{contStep, 22},
+			{contNext, 23},
+			{contNext, 29}})
+
+	}
+}

pkg/proc/target_exec.go

@@ -8,11 +8,12 @@ import (
 	"go/token"
 	"path/filepath"
 	"strconv"
-	"strings"
 
 	"github.com/go-delve/delve/pkg/dwarf/reader"
 )
 
+const maxSkipAutogeneratedWrappers = 5 // maximum recursion depth for skipAutogeneratedWrappers
+
 // ErrNoSourceForPC is returned when the given address
 // does not correspond with a source file location.
 type ErrNoSourceForPC struct {
@@ -359,7 +360,6 @@ func (dbp *Target) StepOut() error {
 	}
 
 	sameGCond := sameGoroutineCondition(selg)
-	retFrameCond := andFrameoffCondition(sameGCond, retframe.FrameOffset())
 
 	if backward {
 		if err := stepOutReverse(dbp, topframe, retframe, sameGCond); err != nil {
@@ -383,12 +383,14 @@ func (dbp *Target) StepOut() error {
 	}
 
 	if topframe.Ret != 0 {
+		topframe, retframe := skipAutogeneratedWrappersOut(selg, curthread, &topframe, &retframe)
+		retFrameCond := andFrameoffCondition(sameGCond, retframe.FrameOffset())
 		bp, err := allowDuplicateBreakpoint(dbp.SetBreakpoint(topframe.Ret, NextBreakpoint, retFrameCond))
 		if err != nil {
 			return err
 		}
 		if bp != nil {
-			configureReturnBreakpoint(dbp.BinInfo(), bp, &topframe, retFrameCond)
+			configureReturnBreakpoint(dbp.BinInfo(), bp, topframe, retFrameCond)
 		}
 	}
 
@@ -535,24 +537,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
 		return err
 	}
 
-	retFrameCond := andFrameoffCondition(sameGCond, retframe.FrameOffset())
 	sameFrameCond := andFrameoffCondition(sameGCond, topframe.FrameOffset())
-	var sameOrRetFrameCond ast.Expr
-	if sameGCond != nil {
-		if topframe.Inlined {
-			sameOrRetFrameCond = sameFrameCond
-		} else {
-			sameOrRetFrameCond = &ast.BinaryExpr{
-				Op: token.LAND,
-				X:  sameGCond,
-				Y: &ast.BinaryExpr{
-					Op: token.LOR,
-					X:  frameoffCondition(topframe.FrameOffset()),
-					Y:  frameoffCondition(retframe.FrameOffset()),
-				},
-			}
-		}
-	}
 
 	if stepInto && !backward {
 		err := setStepIntoBreakpoints(dbp, text, topframe, sameGCond)
@@ -632,6 +617,21 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
 	}
 
 	if !topframe.Inlined {
+		topframe, retframe := skipAutogeneratedWrappersOut(selg, curthread, &topframe, &retframe)
+		retFrameCond := andFrameoffCondition(sameGCond, retframe.FrameOffset())
+		var sameOrRetFrameCond ast.Expr
+		if sameGCond != nil {
+			sameOrRetFrameCond = &ast.BinaryExpr{
+				Op: token.LAND,
+				X:  sameGCond,
+				Y: &ast.BinaryExpr{
+					Op: token.LOR,
+					X:  frameoffCondition(topframe.FrameOffset()),
+					Y:  frameoffCondition(retframe.FrameOffset()),
+				},
+			}
+		}
+
 		// Add a breakpoint on the return address for the current frame.
 		// For inlined functions there is no need to do this, the set of PCs
 		// returned by the AllPCsBetween call above already cover all instructions
@@ -649,7 +649,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
 		// Return address could be wrong, if we are unable to set a breakpoint
 		// there it's ok.
 		if bp != nil {
-			configureReturnBreakpoint(dbp.BinInfo(), bp, &topframe, retFrameCond)
+			configureReturnBreakpoint(dbp.BinInfo(), bp, topframe, retFrameCond)
 		}
 	}
 
@@ -687,7 +687,7 @@ func setStepIntoBreakpointsReverse(dbp Process, text []AsmInstruction, topframe
 			continue
 		}
 
-		if fn := instr.DestLoc.Fn; strings.HasPrefix(fn.Name, "runtime.") && !isExportedRuntime(fn.Name) {
+		if instr.DestLoc.Fn.privateRuntime() {
 			continue
 		}
 
@@ -759,7 +759,7 @@ func setStepIntoBreakpoint(dbp Process, text []AsmInstruction, cond ast.Expr) er
 	fn := instr.DestLoc.Fn
 
 	// Skip unexported runtime functions
-	if fn != nil && strings.HasPrefix(fn.Name, "runtime.") && !isExportedRuntime(fn.Name) {
+	if fn != nil && fn.privateRuntime() {
 		return nil
 	}
 
@@ -774,13 +774,15 @@ func setStepIntoBreakpoint(dbp Process, text []AsmInstruction, cond ast.Expr) er
 		return nil
 	}
 
+	fn, pc = skipAutogeneratedWrappersIn(dbp, fn, pc)
+
 	// We want to skip the function prologue but we should only do it if the
 	// destination address of the CALL instruction is the entry point of the
 	// function.
 	// Calls to runtime.duffzero and duffcopy inserted by the compiler can
 	// sometimes point inside the body of those functions, well after the
 	// prologue.
-	if fn != nil && fn.Entry == instr.DestLoc.PC {
+	if fn != nil && fn.Entry == pc {
 		pc, _ = FirstPCAfterPrologue(dbp, fn, false)
 	}
 
@@ -801,6 +803,109 @@ func allowDuplicateBreakpoint(bp *Breakpoint, err error) (*Breakpoint, error) {
 	return bp, err
 }
 
+func isAutogenerated(loc Location) bool {
+	return loc.File == "<autogenerated>" && loc.Line == 1
+}
+
+// skipAutogeneratedWrappers skips autogenerated wrappers when setting a
+// step-into breakpoint.
+// See genwrapper in: $GOROOT/src/cmd/compile/internal/gc/subr.go
+func skipAutogeneratedWrappersIn(p Process, startfn *Function, startpc uint64) (*Function, uint64) {
+	if startfn == nil {
+		return nil, startpc
+	}
+	fn := startfn
+	for count := 0; count < maxSkipAutogeneratedWrappers; count++ {
+		if !fn.cu.isgo {
+			// can't exit Go
+			return startfn, startpc
+		}
+		text, err := Disassemble(p.CurrentThread(), nil, p.Breakpoints(), p.BinInfo(), fn.Entry, fn.End)
+		if err != nil {
+			break
+		}
+		if len(text) == 0 {
+			break
+		}
+		if !isAutogenerated(text[0].Loc) {
+			return fn, fn.Entry
+		}
+		tgtfns := []*Function{}
+		// collect all functions called by the current destination function
+		for _, instr := range text {
+			switch {
+			case instr.IsCall():
+				if instr.DestLoc == nil || instr.DestLoc.Fn == nil {
+					return startfn, startpc
+				}
+				// calls to non private runtime functions
+				if !instr.DestLoc.Fn.privateRuntime() {
+					tgtfns = append(tgtfns, instr.DestLoc.Fn)
+				}
+			case instr.IsJmp():
+				// unconditional jumps to a different function that isn't a private runtime function
+				if instr.DestLoc != nil && instr.DestLoc.Fn != fn && !instr.DestLoc.Fn.privateRuntime() {
+					tgtfns = append(tgtfns, instr.DestLoc.Fn)
+				}
+			}
+		}
+		if len(tgtfns) != 1 {
+			// too many or not enough function calls
+			break
+		}
+
+		tgtfn := tgtfns[0]
+		if tgtfn.BaseName() != fn.BaseName() {
+			return startfn, startpc
+		}
+		fn = tgtfn
+	}
+	return startfn, startpc
+}
+
+// skipAutogeneratedWrappersOut skip autogenerated wrappers when setting a
+// step out breakpoint.
+// See genwrapper in: $GOROOT/src/cmd/compile/internal/gc/subr.go
+func skipAutogeneratedWrappersOut(g *G, thread Thread, startTopframe, startRetframe *Stackframe) (topframe, retframe *Stackframe) {
+	topframe, retframe = startTopframe, startRetframe
+	if startTopframe.Ret == 0 {
+		return
+	}
+	if !isAutogenerated(startRetframe.Current) {
+		return
+	}
+	retfn := thread.BinInfo().PCToFunc(startTopframe.Ret)
+	if retfn == nil {
+		return
+	}
+	if !retfn.cu.isgo {
+		return
+	}
+	var err error
+	var frames []Stackframe
+	if g == nil {
+		if thread.Blocked() {
+			return
+		}
+		frames, err = ThreadStacktrace(thread, maxSkipAutogeneratedWrappers)
+	} else {
+		frames, err = g.Stacktrace(maxSkipAutogeneratedWrappers, 0)
+	}
+	if err != nil {
+		return
+	}
+	for i := 1; i < len(frames); i++ {
+		frame := frames[i]
+		if frame.Current.Fn == nil {
+			return
+		}
+		if !isAutogenerated(frame.Current) {
+			return &frames[i-1], &frames[i]
+		}
+	}
+	return
+}
+
 // setDeferBreakpoint is a helper function used by next and StepOut to set a
 // breakpoint on the first deferred function.
 func setDeferBreakpoint(p Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr, stepInto bool) (uint64, error) {

pkg/proc/variables.go

@@ -489,7 +489,7 @@ func (g *G) UserCurrent() Location {
 		frame := it.Frame()
 		if frame.Call.Fn != nil {
 			name := frame.Call.Fn.Name
-			if strings.Contains(name, ".") && (!strings.HasPrefix(name, "runtime.") || isExportedRuntime(name)) {
+			if strings.Contains(name, ".") && (!strings.HasPrefix(name, "runtime.") || frame.Call.Fn.exportedRuntime()) {
 				return frame.Call
 			}
 		}
@@ -894,14 +894,6 @@ func (v *Variable) fieldVariable(name string) *Variable {
 	return nil
 }
 
-// From $GOROOT/src/runtime/traceback.go:597
-// isExportedRuntime reports whether name is an exported runtime function.
-// It is only for runtime functions, so ASCII A-Z is fine.
-func isExportedRuntime(name string) bool {
-	const n = len("runtime.")
-	return len(name) > n && name[:n] == "runtime." && 'A' <= name[n] && name[n] <= 'Z'
-}
-
 var errTracebackAncestorsDisabled = errors.New("tracebackancestors is disabled")
 
 // Ancestors returns the list of ancestors for g.

pkg/proc/x86_disasm.go

@@ -24,6 +24,8 @@ func x86AsmDecode(asmInst *AsmInstruction, mem []byte, regs Registers, memrw Mem
 	asmInst.Kind = OtherInstruction
 
 	switch inst.Op {
+	case x86asm.JMP, x86asm.LJMP:
+		asmInst.Kind = JmpInstruction
 	case x86asm.CALL, x86asm.LCALL:
 		asmInst.Kind = CallInstruction
 	case x86asm.RET, x86asm.LRET:
@@ -73,7 +75,10 @@ func (inst *x86Inst) OpcodeEquals(op uint64) bool {
 }
 
 func resolveCallArgX86(inst *x86asm.Inst, instAddr uint64, currentGoroutine bool, regs Registers, mem MemoryReadWriter, bininfo *BinaryInfo) *Location {
-	if inst.Op != x86asm.CALL && inst.Op != x86asm.LCALL {
+	switch inst.Op {
+	case x86asm.CALL, x86asm.LCALL, x86asm.JMP, x86asm.LJMP:
+		// ok
+	default:
 		return nil
 	}