*: Show return values on CLI trace

This patch allows the `trace` CLI subcommand to display return values of
a function. Additionally, it will also display information on where the
function exited, which could also be helpful in determining the path
taken during function execution.

Fixes #388

cmd/dlv/cmds/commands.go

@@ -397,11 +397,34 @@ func traceCmd(cmd *cobra.Command, args []string) {
 			return 1
 		}
 		for i := range funcs {
-			_, err = client.CreateBreakpoint(&api.Breakpoint{FunctionName: funcs[i], Tracepoint: true, Line: -1, Stacktrace: traceStackDepth, LoadArgs: &terminal.ShortLoadConfig})
+			_, err = client.CreateBreakpoint(&api.Breakpoint{
+				FunctionName: funcs[i],
+				Tracepoint:   true,
+				Line:         -1,
+				Stacktrace:   traceStackDepth,
+				LoadArgs:     &terminal.ShortLoadConfig,
+			})
 			if err != nil {
 				fmt.Fprintln(os.Stderr, err)
 				return 1
 			}
+			addrs, err := client.FunctionReturnLocations(funcs[i])
+			if err != nil {
+				fmt.Fprintln(os.Stderr, err)
+				return 1
+			}
+			for i := range addrs {
+				_, err = client.CreateBreakpoint(&api.Breakpoint{
+					Addr:        addrs[i],
+					TraceReturn: true,
+					Line:        -1,
+					LoadArgs:    &terminal.ShortLoadConfig,
+				})
+				if err != nil {
+					fmt.Fprintln(os.Stderr, err)
+					return 1
+				}
+			}
 		}
 		cmds := terminal.DebugCommands(client)
 		t := terminal.New(client, nil)

pkg/proc/breakpoints.go

@@ -30,7 +30,8 @@ type Breakpoint struct {
 	Kind BreakpointKind
 
 	// Breakpoint information
-	Tracepoint    bool     // Tracepoint flag
+	Tracepoint    bool // Tracepoint flag
+	TraceReturn   bool
 	Goroutine     bool     // Retrieve goroutine information
 	Stacktrace    int      // Number of stack frames to retrieve
 	Variables     []string // Variables to evaluate
@@ -45,7 +46,7 @@ type Breakpoint struct {
 	// Next uses NextDeferBreakpoints for the breakpoint it sets on the
 	// deferred function, DeferReturns is populated with the
 	// addresses of calls to runtime.deferreturn in the current
-	// function. This insures that the breakpoint on the deferred
+	// function. This ensures that the breakpoint on the deferred
 	// function only triggers on panic or on the defer call to
 	// the function, not when the function is called directly
 	DeferReturns []uint64

pkg/proc/disasm_amd64.go

@@ -65,6 +65,14 @@ func (inst *AsmInstruction) IsCall() bool {
 	return inst.Inst.Op == x86asm.CALL || inst.Inst.Op == x86asm.LCALL
 }
 
+// IsRet returns true if the instruction is a RET or LRET instruction.
+func (inst *AsmInstruction) IsRet() bool {
+	if inst.Inst == nil {
+		return false
+	}
+	return inst.Inst.Op == x86asm.RET || inst.Inst.Op == x86asm.LRET
+}
+
 func resolveCallArg(inst *archInst, currentGoroutine bool, regs Registers, mem MemoryReadWriter, bininfo *BinaryInfo) *Location {
 	if inst.Op != x86asm.CALL && inst.Op != x86asm.LCALL {
 		return nil

pkg/proc/proc.go

@@ -88,6 +88,33 @@ func FindFunctionLocation(p Process, funcName string, firstLine bool, lineOffset
 	return origfn.Entry, nil
 }
 
+// FunctionReturnLocations will return a list of addresses corresponding
+// to 'ret' or 'call runtime.deferreturn'.
+func FunctionReturnLocations(p Process, funcName string) ([]uint64, error) {
+	const deferReturn = "runtime.deferreturn"
+
+	g := p.SelectedGoroutine()
+	fn, ok := p.BinInfo().LookupFunc[funcName]
+	if !ok {
+		return nil, fmt.Errorf("unable to find function %s", funcName)
+	}
+
+	instructions, err := Disassemble(p, g, fn.Entry, fn.End)
+	if err != nil {
+		return nil, err
+	}
+
+	var addrs []uint64
+	for _, instruction := range instructions {
+		if instruction.IsRet() {
+			addrs = append(addrs, instruction.Loc.PC)
+		}
+	}
+	addrs = append(addrs, findDeferReturnCalls(instructions)...)
+
+	return addrs, nil
+}
+
 // Next continues execution until the next source line.
 func Next(dbp Process) (err error) {
 	if _, err := dbp.Valid(); err != nil {
@@ -184,7 +211,8 @@ func Continue(dbp Process) error {
 				return conditionErrors(threads)
 			}
 		case curbp.Active && curbp.Internal:
-			if curbp.Kind == StepBreakpoint {
+			switch curbp.Kind {
+			case StepBreakpoint:
 				// See description of proc.(*Process).next for the meaning of StepBreakpoints
 				if err := conditionErrors(threads); err != nil {
 					return err
@@ -204,7 +232,7 @@ func Continue(dbp Process) error {
 				if err = setStepIntoBreakpoint(dbp, text, SameGoroutineCondition(dbp.SelectedGoroutine())); err != nil {
 					return err
 				}
-			} else {
+			default:
 				curthread.Common().returnValues = curbp.Breakpoint.returnInfo.Collect(curthread)
 				if err := dbp.ClearInternalBreakpoints(); err != nil {
 					return err

pkg/proc/threads.go

@@ -225,15 +225,7 @@ func next(dbp Process, stepInto, inlinedStepOut bool) error {
 	}
 
 	if !csource {
-		deferreturns := []uint64{}
+		deferreturns := findDeferReturnCalls(text)
-
-		// Find all runtime.deferreturn locations in the function
-		// See documentation of Breakpoint.DeferCond for why this is necessary
-		for _, instr := range text {
-			if instr.IsCall() && instr.DestLoc != nil && instr.DestLoc.Fn != nil && instr.DestLoc.Fn.Name == "runtime.deferreturn" {
-				deferreturns = append(deferreturns, instr.Loc.PC)
-			}
-		}
 
 		// Set breakpoint on the most recently deferred function (if any)
 		var deferpc uint64
@@ -333,6 +325,20 @@ func next(dbp Process, stepInto, inlinedStepOut bool) error {
 	return nil
 }
 
+func findDeferReturnCalls(text []AsmInstruction) []uint64 {
+	const deferreturn = "runtime.deferreturn"
+	deferreturns := []uint64{}
+
+	// Find all runtime.deferreturn locations in the function
+	// See documentation of Breakpoint.DeferCond for why this is necessary
+	for _, instr := range text {
+		if instr.IsCall() && instr.DestLoc != nil && instr.DestLoc.Fn != nil && instr.DestLoc.Fn.Name == deferreturn {
+			deferreturns = append(deferreturns, instr.Loc.PC)
+		}
+	}
+	return deferreturns
+}
+
 // Removes instructions belonging to inlined calls of topframe from pcs.
 // If includeCurrentFn is true it will also remove all instructions
 // belonging to the current function.

pkg/proc/variables.go

@@ -405,13 +405,13 @@ func (scope *EvalScope) PtrSize() int {
 	return scope.BinInfo.Arch.PtrSize()
 }
 
-// NoGError returned when a G could not be found
+// ErrNoGoroutine returned when a G could not be found
 // for a specific thread.
-type NoGError struct {
+type ErrNoGoroutine struct {
 	tid int
 }
 
-func (ng NoGError) Error() string {
+func (ng ErrNoGoroutine) Error() string {
 	return fmt.Sprintf("no G executing on thread %d", ng.tid)
 }
 
@@ -433,7 +433,7 @@ func (v *Variable) parseG() (*G, error) {
 		if thread, ok := mem.(Thread); ok {
 			id = thread.ThreadID()
 		}
-		return nil, NoGError{tid: id}
+		return nil, ErrNoGoroutine{tid: id}
 	}
 	for {
 		if _, isptr := v.RealType.(*godwarf.PtrType); !isptr {

pkg/terminal/command.go

@@ -1702,7 +1702,14 @@ func printcontextThread(t *Term, th *api.Thread) {
 	if th.BreakpointInfo != nil && th.Breakpoint.LoadArgs != nil && *th.Breakpoint.LoadArgs == ShortLoadConfig {
 		var arg []string
 		for _, ar := range th.BreakpointInfo.Arguments {
-			arg = append(arg, ar.SinglelineString())
+			// For AI compatibility return values are included in the
+			// argument list. This is a relic of the dark ages when the
+			// Go debug information did not distinguish between the two.
+			// Filter them out here instead, so during trace operations
+			// they are not printed as an argument.
+			if (ar.Flags & api.VariableArgument) != 0 {
+				arg = append(arg, ar.SinglelineString())
+			}
 		}
 		args = strings.Join(arg, ", ")
 	}

service/api/conversions.go

@@ -23,6 +23,7 @@ func ConvertBreakpoint(bp *proc.Breakpoint) *Breakpoint {
 		Line:          bp.Line,
 		Addr:          bp.Addr,
 		Tracepoint:    bp.Tracepoint,
+		TraceReturn:   bp.TraceReturn,
 		Stacktrace:    bp.Stacktrace,
 		Goroutine:     bp.Goroutine,
 		Variables:     bp.Variables,

service/api/types.go

@@ -59,8 +59,11 @@ type Breakpoint struct {
 	// Breakpoint condition
 	Cond string
 
-	// tracepoint flag
+	// Tracepoint flag, signifying this is a tracepoint.
 	Tracepoint bool `json:"continue"`
+	// TraceReturn flag signifying this is a breakpoint set at a return
+	// statement in a traced function.
+	TraceReturn bool `json:"traceReturn"`
 	// retrieve goroutine information
 	Goroutine bool `json:"goroutine"`
 	// number of stack frames to retrieve
@@ -197,11 +200,11 @@ const (
 	// that may outlive the stack frame are allocated on the heap instead and
 	// only the address is recorded on the stack. These variables will be
 	// marked with this flag.
-	VariableEscaped = VariableFlags(proc.VariableEscaped)
+	VariableEscaped = (1 << iota)
 
 	// VariableShadowed is set for local variables that are shadowed by a
 	// variable with the same name in another scope
-	VariableShadowed = VariableFlags(proc.VariableShadowed)
+	VariableShadowed
 
 	// VariableConstant means this variable is a constant value
 	VariableConstant

service/debugger/debugger.go

@@ -195,6 +195,14 @@ func (d *Debugger) LastModified() time.Time {
 	return d.target.BinInfo().LastModified()
 }
 
+// FunctionReturnLocations returns all return locations
+// for the given function. See the documentation for the
+// function of the same name within the `proc` package for
+// more information.
+func (d *Debugger) FunctionReturnLocations(fnName string) ([]uint64, error) {
+	return proc.FunctionReturnLocations(d.target, fnName)
+}
+
 // Detach detaches from the target process.
 // If `kill` is true we will kill the process after
 // detaching.
@@ -348,6 +356,8 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 	}
 
 	switch {
+	case requestedBp.TraceReturn:
+		addr = requestedBp.Addr
 	case len(requestedBp.File) > 0:
 		fileName := requestedBp.File
 		if runtime.GOOS == "windows" {
@@ -414,6 +424,7 @@ func (d *Debugger) CancelNext() error {
 func copyBreakpointInfo(bp *proc.Breakpoint, requested *api.Breakpoint) (err error) {
 	bp.Name = requested.Name
 	bp.Tracepoint = requested.Tracepoint
+	bp.TraceReturn = requested.TraceReturn
 	bp.Goroutine = requested.Goroutine
 	bp.Stacktrace = requested.Stacktrace
 	bp.Variables = requested.Variables
@@ -617,6 +628,15 @@ func (d *Debugger) Command(command *api.DebuggerCommand) (*api.DebuggerState, er
 	if withBreakpointInfo {
 		err = d.collectBreakpointInformation(state)
 	}
+	for _, th := range state.Threads {
+		if th.Breakpoint != nil && th.Breakpoint.TraceReturn {
+			for _, v := range th.BreakpointInfo.Arguments {
+				if (v.Flags & api.VariableReturnArgument) != 0 {
+					th.ReturnValues = append(th.ReturnValues, v)
+				}
+			}
+		}
+	}
 	return state, err
 }
 

service/rpc2/client.go

@@ -117,7 +117,7 @@ func (c *RPCClient) continueDir(cmd string) <-chan *api.DebuggerState {
 			for i := range state.Threads {
 				if state.Threads[i].Breakpoint != nil {
 					isbreakpoint = true
-					istracepoint = istracepoint && state.Threads[i].Breakpoint.Tracepoint
+					istracepoint = istracepoint && (state.Threads[i].Breakpoint.Tracepoint || state.Threads[i].Breakpoint.TraceReturn)
 				}
 			}
 
@@ -375,6 +375,12 @@ func (c *RPCClient) SetReturnValuesLoadConfig(cfg *api.LoadConfig) {
 	c.retValLoadCfg = cfg
 }
 
+func (c *RPCClient) FunctionReturnLocations(fnName string) ([]uint64, error) {
+	var out FunctionReturnLocationsOut
+	err := c.call("FunctionReturnLocations", FunctionReturnLocationsIn{fnName}, &out)
+	return out.Addrs, err
+}
+
 func (c *RPCClient) IsMulticlient() bool {
 	var out IsMulticlientOut
 	c.call("IsMulticlient", IsMulticlientIn{}, &out)

service/rpc2/server.go

@@ -655,3 +655,33 @@ func (s *RPCServer) IsMulticlient(arg IsMulticlientIn, out *IsMulticlientOut) er
 	}
 	return nil
 }
+
+// FunctionReturnLocationsIn holds arguments for the
+// FunctionReturnLocationsRPC call. It holds the name of
+// the function for which all return locations should be
+// given.
+type FunctionReturnLocationsIn struct {
+	// FnName is the name of the function for which all
+	// return locations should be given.
+	FnName string
+}
+
+// FunctionReturnLocationsOut holds the result of the FunctionReturnLocations
+// RPC call. It provides the list of addresses that the given function returns,
+// for example with a `RET` instruction or `CALL runtime.deferreturn`.
+type FunctionReturnLocationsOut struct {
+	// Addrs is the list of all locations where the given function returns.
+	Addrs []uint64
+}
+
+// FunctionReturnLocations is the implements the client call of the same name. Look at client documentation for more information.
+func (s *RPCServer) FunctionReturnLocations(in FunctionReturnLocationsIn, out *FunctionReturnLocationsOut) error {
+	addrs, err := s.debugger.FunctionReturnLocations(in.FnName)
+	if err != nil {
+		return err
+	}
+	*out = FunctionReturnLocationsOut{
+		Addrs: addrs,
+	}
+	return nil
+}