proc,service: display return values when stepping out of a function

Displays the return values of the current function when we step out of
it after executing a step, next or stepout command.

Implementation of this feature is tricky: when the function has
returned the return variables are not in scope anymore. Implementing
this feature requires evaluating variables that are out of scope, using
a stack frame that doesn't exist anymore.

We can't calculate the address of these variables when the
next/step/stepout command is initiated either, because between that
point and the time where the stepout breakpoint is actually hit the
goroutine stack could grow and be moved to a different memory address.

_fixtures/stepoutret.go

@@ -0,0 +1,11 @@
+package main
+
+import "fmt"
+
+func stepout(n int) (str string, num int) {
+	return fmt.Sprintf("return %d", n), n + 1
+}
+
+func main() {
+	stepout(47)
+}

pkg/proc/breakpoints.go

@@ -1,6 +1,7 @@
 package proc
 
 import (
+	"debug/dwarf"
 	"errors"
 	"fmt"
 	"go/ast"
@@ -53,6 +54,10 @@ type Breakpoint struct {
 	Cond ast.Expr
 	// internalCond is the same as Cond but used for the condition of internal breakpoints
 	internalCond ast.Expr
+
+	// ReturnInfo describes how to collect return variables when this
+	// breakpoint is hit as a return breakpoint.
+	returnInfo *returnBreakpointInfo
 }
 
 // Breakpoint Kind determines the behavior of delve when the
@@ -102,6 +107,13 @@ func (iae InvalidAddressError) Error() string {
 	return fmt.Sprintf("Invalid address %#v\n", iae.Address)
 }
 
+type returnBreakpointInfo struct {
+	retFrameCond ast.Expr
+	fn           *Function
+	frameOffset  int64
+	spOffset     int64
+}
+
 // CheckCondition evaluates bp's condition on thread.
 func (bp *Breakpoint) CheckCondition(thread Thread) BreakpointState {
 	bpstate := BreakpointState{Breakpoint: bp, Active: false, Internal: false, CondError: nil}
@@ -302,6 +314,7 @@ func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakp
 	for addr, bp := range bpmap.M {
 		bp.Kind = bp.Kind & UserBreakpoint
 		bp.internalCond = nil
+		bp.returnInfo = nil
 		if bp.Kind != 0 {
 			continue
 		}
@@ -354,3 +367,82 @@ func (bpstate *BreakpointState) String() string {
 	}
 	return s
 }
+
+func configureReturnBreakpoint(bi *BinaryInfo, bp *Breakpoint, topframe *Stackframe, retFrameCond ast.Expr) {
+	if topframe.Current.Fn == nil {
+		return
+	}
+	bp.returnInfo = &returnBreakpointInfo{
+		retFrameCond: retFrameCond,
+		fn:           topframe.Current.Fn,
+		frameOffset:  topframe.FrameOffset(),
+		spOffset:     topframe.FrameOffset() - int64(bi.Arch.PtrSize()), // must be the value that SP had at the entry point of the function
+	}
+}
+
+func (rbpi *returnBreakpointInfo) Collect(thread Thread) []*Variable {
+	if rbpi == nil {
+		return nil
+	}
+
+	bi := thread.BinInfo()
+
+	g, err := GetG(thread)
+	if err != nil {
+		return returnInfoError("could not get g", err, thread)
+	}
+	scope, err := GoroutineScope(thread)
+	if err != nil {
+		return returnInfoError("could not get scope", err, thread)
+	}
+	v, err := scope.evalAST(rbpi.retFrameCond)
+	if err != nil || v.Unreadable != nil || v.Kind != reflect.Bool {
+		// This condition was evaluated as part of the breakpoint condition
+		// evaluation, if the errors happen they will be reported as part of the
+		// condition errors.
+		return nil
+	}
+	if !constant.BoolVal(v.Value) {
+		// Breakpoint not hit as a return breakpoint.
+		return nil
+	}
+
+	// Alter the eval scope so that it looks like we are at the entry point of
+	// the function we just returned from.
+	// This involves changing the location (PC, function...) as well as
+	// anything related to the stack pointer (SP, CFA, FrameBase).
+	scope.PC = rbpi.fn.Entry
+	scope.Fn = rbpi.fn
+	scope.File, scope.Line, _ = bi.PCToLine(rbpi.fn.Entry)
+	scope.Regs.CFA = rbpi.frameOffset + int64(g.stackhi)
+	scope.Regs.Regs[scope.Regs.SPRegNum].Uint64Val = uint64(rbpi.spOffset + int64(g.stackhi))
+
+	bi.dwarfReader.Seek(rbpi.fn.offset)
+	e, err := bi.dwarfReader.Next()
+	if err != nil {
+		return returnInfoError("could not read function entry", err, thread)
+	}
+	scope.Regs.FrameBase, _, _, _ = bi.Location(e, dwarf.AttrFrameBase, scope.PC, scope.Regs)
+
+	vars, err := scope.Locals()
+	if err != nil {
+		return returnInfoError("could not evaluate return variables", err, thread)
+	}
+	vars = filterVariables(vars, func(v *Variable) bool {
+		return (v.Flags & VariableReturnArgument) != 0
+	})
+
+	// Go saves the return variables in the opposite order that the user
+	// specifies them so here we reverse the slice to make it easier to
+	// understand.
+	for i := 0; i < len(vars)/2; i++ {
+		vars[i], vars[len(vars)-i-1] = vars[len(vars)-i-1], vars[i]
+	}
+	return vars
+}
+
+func returnInfoError(descr string, err error, mem MemoryReadWriter) []*Variable {
+	v := newConstant(constant.MakeString(fmt.Sprintf("%s: %v", descr, err.Error())), mem)
+	v.Name = "return value read error"
+	return []*Variable{v}
+}

pkg/proc/core/core.go

@@ -154,6 +154,7 @@ type Thread struct {
 	th     *LinuxPrStatus
 	fpregs []proc.Register
 	p      *Process
+	common proc.CommonThread
 }
 
 var ErrWriteCore = errors.New("can not to core process")
@@ -258,6 +259,10 @@ func (t *Thread) SetCurrentBreakpoint() error {
 	return nil
 }
 
+func (t *Thread) Common() *proc.CommonThread {
+	return &t.common
+}
+
 func (p *Process) Breakpoints() *proc.BreakpointMap {
 	return &p.breakpoints
 }

pkg/proc/core/linux_amd64_core.go

@@ -277,7 +277,7 @@ func readCore(corePath, exePath string) (*Core, error) {
 		switch note.Type {
 		case elf.NT_PRSTATUS:
 			t := note.Desc.(*LinuxPrStatus)
-			lastThread = &Thread{t, nil, nil}
+			lastThread = &Thread{t, nil, nil, proc.CommonThread{}}
 			core.Threads[int(t.Pid)] = lastThread
 		case NT_X86_XSTATE:
 			if lastThread != nil {

pkg/proc/gdbserial/gdbserver.go

@@ -132,6 +132,7 @@ type Thread struct {
 	CurrentBreakpoint proc.BreakpointState
 	p                 *Process
 	setbp             bool // thread was stopped because of a breakpoint
+	common            proc.CommonThread
 }
 
 // ErrBackendUnavailable is returned when the stub program can not be found.
@@ -1189,6 +1190,10 @@ func (t *Thread) BinInfo() *proc.BinaryInfo {
 	return &t.p.bi
 }
 
+func (t *Thread) Common() *proc.CommonThread {
+	return &t.common
+}
+
 func (t *Thread) stepInstruction(tu *threadUpdater) error {
 	pc := t.regs.PC()
 	if _, atbp := t.p.breakpoints.M[pc]; atbp {

pkg/proc/interface.go

@@ -103,10 +103,13 @@ type BreakpointManipulation interface {
 	ClearInternalBreakpoints() error
 }
 
+// CommonProcess contains fields used by this package, common to all
+// implementations of the Process interface.
 type CommonProcess struct {
 	allGCache []*G
 }
 
+// ClearAllGCache clears the cached contents of the cache for runtime.allgs.
 func (p *CommonProcess) ClearAllGCache() {
 	p.allGCache = nil
 }

pkg/proc/native/proc_darwin.go

@@ -103,7 +103,7 @@ func Launch(cmd []string, wd string, foreground bool) (*Process, error) {
 		return nil, err
 	}
 
-	dbp.allGCache = nil
+	dbp.common.ClearAllGCache()
 	for _, th := range dbp.threads {
 		th.CurrentBreakpoint.Clear()
 	}

pkg/proc/native/threads.go

@@ -19,6 +19,7 @@ type Thread struct {
 	dbp            *Process
 	singleStepping bool
 	os             *OSSpecificDetails
+	common         proc.CommonThread
 }
 
 // Continue the execution of this thread.
@@ -101,6 +102,10 @@ func (thread *Thread) BinInfo() *proc.BinaryInfo {
 	return &thread.dbp.bi
 }
 
+func (thread *Thread) Common() *proc.CommonThread {
+	return &thread.common
+}
+
 // SetPC sets the PC for this thread.
 func (thread *Thread) SetPC(pc uint64) error {
 	regs, err := thread.Registers(false)

pkg/proc/proc.go

@@ -95,6 +95,9 @@ func Continue(dbp Process) error {
 	if dbp.Exited() {
 		return &ProcessExitedError{Pid: dbp.Pid()}
 	}
+	for _, thread := range dbp.ThreadList() {
+		thread.Common().returnValues = nil
+	}
 	dbp.CheckAndClearManualStopRequest()
 	defer func() {
 		// Make sure we clear internal breakpoints if we simultaneously receive a
@@ -166,6 +169,7 @@ func Continue(dbp Process) error {
 					return err
 				}
 			} else {
+				curthread.Common().returnValues = curbp.Breakpoint.returnInfo.Collect(curthread)
 				if err := dbp.ClearInternalBreakpoints(); err != nil {
 					return err
 				}
@@ -357,12 +361,15 @@ func StepOut(dbp Process) error {
 	}
 
 	if topframe.Ret != 0 {
-		_, err := dbp.SetBreakpoint(topframe.Ret, NextBreakpoint, retFrameCond)
+		bp, err := dbp.SetBreakpoint(topframe.Ret, NextBreakpoint, retFrameCond)
 		if err != nil {
 			if _, isexists := err.(BreakpointExistsError); !isexists {
 				return err
 			}
 		}
+		if bp != nil {
+			configureReturnBreakpoint(dbp.BinInfo(), bp, &topframe, retFrameCond)
+		}
 	}
 
 	if bp := curthread.Breakpoint(); bp.Breakpoint == nil {

pkg/proc/proc_test.go

@@ -3807,3 +3807,40 @@ func TestMapLoadConfigWithReslice(t *testing.T) {
 		}
 	})
 }
+
+func TestStepOutReturn(t *testing.T) {
+	ver, _ := goversion.Parse(runtime.Version())
+	if ver.Major >= 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}) {
+		t.Skip("return variables aren't marked on 1.9 or earlier")
+	}
+	withTestProcess("stepoutret", t, func(p proc.Process, fixture protest.Fixture) {
+		_, err := setFunctionBreakpoint(p, "main.stepout")
+		assertNoError(err, t, "SetBreakpoint")
+		assertNoError(proc.Continue(p), t, "Continue")
+		assertNoError(proc.StepOut(p), t, "StepOut")
+		ret := p.CurrentThread().Common().ReturnValues(normalLoadConfig)
+		if len(ret) != 2 {
+			t.Fatalf("wrong number of return values %v", ret)
+		}
+
+		if ret[0].Name != "str" {
+			t.Fatalf("(str) bad return value name %s", ret[0].Name)
+		}
+		if ret[0].Kind != reflect.String {
+			t.Fatalf("(str) bad return value kind %v", ret[0].Kind)
+		}
+		if s := constant.StringVal(ret[0].Value); s != "return 47" {
+			t.Fatalf("(str) bad return value %q", s)
+		}
+
+		if ret[1].Name != "num" {
+			t.Fatalf("(num) bad return value name %s", ret[1].Name)
+		}
+		if ret[1].Kind != reflect.Int {
+			t.Fatalf("(num) bad return value kind %v", ret[1].Kind)
+		}
+		if n, _ := constant.Int64Val(ret[1].Value); n != 48 {
+			t.Fatalf("(num) bad return value %d", n)
+		}
+	})
+}

pkg/proc/threads.go

@@ -30,6 +30,8 @@ type Thread interface {
 	Blocked() bool
 	// SetCurrentBreakpoint updates the current breakpoint of this thread
 	SetCurrentBreakpoint() error
+	// Common returns the CommonThread structure for this thread
+	Common() *CommonThread
 }
 
 // Location represents the location of a thread.
@@ -50,6 +52,17 @@ func (tbe ThreadBlockedError) Error() string {
 	return ""
 }
 
+// CommonThread contains fields used by this package, common to all
+// implementations of the Thread interface.
+type CommonThread struct {
+	returnValues []*Variable
+}
+
+func (t *CommonThread) ReturnValues(cfg LoadConfig) []*Variable {
+	loadValues(t.returnValues, cfg)
+	return t.returnValues
+}
+
 // topframe returns the two topmost frames of g, or thread if g is nil.
 func topframe(g *G, thread Thread) (Stackframe, Stackframe, error) {
 	var frames []Stackframe
@@ -279,7 +292,8 @@ func next(dbp Process, stepInto, inlinedStepOut bool) error {
 		// For inlined functions there is no need to do this, the set of PCs
 		// returned by the AllPCsBetween call above already cover all instructions
 		// of the containing function.
-		if bp, err := dbp.SetBreakpoint(topframe.Ret, NextBreakpoint, retFrameCond); err != nil {
+		bp, err := dbp.SetBreakpoint(topframe.Ret, NextBreakpoint, retFrameCond)
+		if err != nil {
 			if _, isexists := err.(BreakpointExistsError); isexists {
 				if bp.Kind == NextBreakpoint {
 					// If the return address shares the same address with one of the lines
@@ -292,6 +306,9 @@ func next(dbp Process, 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)
+		}
 	}
 
 	if bp := curthread.Breakpoint(); bp.Breakpoint == nil {

pkg/terminal/command.go

@@ -1518,11 +1518,23 @@ func printcontextLocation(loc api.Location) {
 	return
 }
 
+func printReturnValues(th *api.Thread) {
+	if th.ReturnValues == nil {
+		return
+	}
+	fmt.Println("Values returned:")
+	for _, v := range th.ReturnValues {
+		fmt.Printf("\t%s: %s\n", v.Name, v.MultilineString("\t"))
+	}
+	fmt.Println()
+}
+
 func printcontextThread(t *Term, th *api.Thread) {
 	fn := th.Function
 
 	if th.Breakpoint == nil {
 		printcontextLocation(api.Location{PC: th.PC, File: th.File, Line: th.Line, Function: th.Function})
+		printReturnValues(th)
 		return
 	}
 
@@ -1565,6 +1577,8 @@ func printcontextThread(t *Term, th *api.Thread) {
 		fmt.Println(optimizedFunctionWarning)
 	}
 
+	printReturnValues(th)
+
 	if th.BreakpointInfo != nil {
 		bp := th.Breakpoint
 		bpi := th.BreakpointInfo

pkg/terminal/command_test.go

@@ -9,12 +9,14 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strconv"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/derekparker/delve/pkg/config"
+	"github.com/derekparker/delve/pkg/goversion"
 	"github.com/derekparker/delve/pkg/proc/test"
 	"github.com/derekparker/delve/service"
 	"github.com/derekparker/delve/service/api"
@@ -783,3 +785,19 @@ func TestPrintContextParkedGoroutine(t *testing.T) {
 		}
 	})
 }
+
+func TestStepOutReturn(t *testing.T) {
+	ver, _ := goversion.Parse(runtime.Version())
+	if ver.Major >= 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}) {
+		t.Skip("return variables aren't marked on 1.9 or earlier")
+	}
+	withTestTerminal("stepoutret", t, func(term *FakeTerminal) {
+		term.MustExec("break main.stepout")
+		term.MustExec("continue")
+		out := term.MustExec("stepout")
+		t.Logf("output: %q", out)
+		if !strings.Contains(out, "num: ") || !strings.Contains(out, "str: ") {
+			t.Fatal("could not find parameter")
+		}
+	})
+}

pkg/terminal/terminal.go

@@ -55,6 +55,10 @@ func New(client service.Client, conf *config.Config) *Term {
 		w = getColorableWriter()
 	}
 
+	if client != nil {
+		client.SetReturnValuesLoadConfig(&LongLoadConfig)
+	}
+
 	return &Term{
 		client: client,
 		conf:   conf,

service/api/types.go

@@ -107,6 +107,9 @@ type Thread struct {
 	Breakpoint *Breakpoint `json:"breakPoint,omitempty"`
 	// Informations requested by the current breakpoint
 	BreakpointInfo *BreakpointInfo `json:"breakPointInfo,omitempty"`
+
+	// ReturnValues contains the return values of the function we just stepped out of
+	ReturnValues []Variable
 }
 
 type Location struct {
@@ -263,6 +266,9 @@ type DebuggerCommand struct {
 	// GoroutineID is used to specify which thread to use with the SwitchGoroutine
 	// command.
 	GoroutineID int `json:"goroutineID,omitempty"`
+	// When ReturnInfoLoadConfig is not nil it will be used to load the value
+	// of any return variables.
+	ReturnInfoLoadConfig *LoadConfig
 }
 
 // Informations about the current breakpoint

service/client.go

@@ -127,4 +127,7 @@ type Client interface {
 	ListCheckpoints() ([]api.Checkpoint, error)
 	// ClearCheckpoint removes a checkpoint
 	ClearCheckpoint(id int) error
+
+	// SetReturnValuesLoadConfig sets the load configuration for return values.
+	SetReturnValuesLoadConfig(*api.LoadConfig)
 }

service/debugger/debugger.go

@@ -255,10 +255,10 @@ func (d *Debugger) Restart(pos string, resetArgs bool, newArgs []string) ([]api.
 func (d *Debugger) State() (*api.DebuggerState, error) {
 	d.processMutex.Lock()
 	defer d.processMutex.Unlock()
-	return d.state()
+	return d.state(nil)
 }
 
-func (d *Debugger) state() (*api.DebuggerState, error) {
+func (d *Debugger) state(retLoadCfg *proc.LoadConfig) (*api.DebuggerState, error) {
 	if d.target.Exited() {
 		return nil, proc.ProcessExitedError{Pid: d.ProcessPid()}
 	}
@@ -279,6 +279,11 @@ func (d *Debugger) state() (*api.DebuggerState, error) {
 
 	for _, thread := range d.target.ThreadList() {
 		th := api.ConvertThread(thread)
+
+		if retLoadCfg != nil {
+			th.ReturnValues = convertVars(thread.Common().ReturnValues(*retLoadCfg))
+		}
+
 		state.Threads = append(state.Threads, th)
 		if thread.ThreadID() == d.target.CurrentThread().ThreadID() {
 			state.CurrentThread = th
@@ -556,7 +561,7 @@ func (d *Debugger) Command(command *api.DebuggerCommand) (*api.DebuggerState, er
 		}
 		return nil, err
 	}
-	state, stateErr := d.state()
+	state, stateErr := d.state(api.LoadConfigToProc(command.ReturnInfoLoadConfig))
 	if stateErr != nil {
 		return state, stateErr
 	}
@@ -755,6 +760,9 @@ func (d *Debugger) Registers(threadID int, floatingPoint bool) (api.Registers, e
 }
 
 func convertVars(pv []*proc.Variable) []api.Variable {
+	if pv == nil {
+		return nil
+	}
 	vars := make([]api.Variable, 0, len(pv))
 	for _, v := range pv {
 		vars = append(vars, *api.ConvertVar(v))

service/rpc2/client.go

@@ -15,6 +15,8 @@ import (
 type RPCClient struct {
 	addr   string
 	client *rpc.Client
+
+	retValLoadCfg *api.LoadConfig
 }
 
 // Ensure the implementation satisfies the interface.
@@ -80,7 +82,7 @@ func (c *RPCClient) continueDir(cmd string) <-chan *api.DebuggerState {
 	go func() {
 		for {
 			out := new(CommandOut)
-			err := c.call("Command", &api.DebuggerCommand{Name: cmd}, &out)
+			err := c.call("Command", &api.DebuggerCommand{Name: cmd, ReturnInfoLoadConfig: c.retValLoadCfg}, &out)
 			state := out.State
 			if err != nil {
 				state.Err = err
@@ -115,19 +117,19 @@ func (c *RPCClient) continueDir(cmd string) <-chan *api.DebuggerState {
 
 func (c *RPCClient) Next() (*api.DebuggerState, error) {
 	var out CommandOut
-	err := c.call("Command", api.DebuggerCommand{Name: api.Next}, &out)
+	err := c.call("Command", api.DebuggerCommand{Name: api.Next, ReturnInfoLoadConfig: c.retValLoadCfg}, &out)
 	return &out.State, err
 }
 
 func (c *RPCClient) Step() (*api.DebuggerState, error) {
 	var out CommandOut
-	err := c.call("Command", api.DebuggerCommand{Name: api.Step}, &out)
+	err := c.call("Command", api.DebuggerCommand{Name: api.Step, ReturnInfoLoadConfig: c.retValLoadCfg}, &out)
 	return &out.State, err
 }
 
 func (c *RPCClient) StepOut() (*api.DebuggerState, error) {
 	var out CommandOut
-	err := c.call("Command", &api.DebuggerCommand{Name: api.StepOut}, &out)
+	err := c.call("Command", &api.DebuggerCommand{Name: api.StepOut, ReturnInfoLoadConfig: c.retValLoadCfg}, &out)
 	return &out.State, err
 }
 
@@ -348,6 +350,10 @@ func (c *RPCClient) ClearCheckpoint(id int) error {
 	return err
 }
 
+func (c *RPCClient) SetReturnValuesLoadConfig(cfg *api.LoadConfig) {
+	c.retValLoadCfg = cfg
+}
+
 func (c *RPCClient) call(method string, args, reply interface{}) error {
 	return c.client.Call("RPCServer."+method, args, reply)
 }

service/test/integration2_test.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
@@ -1415,3 +1416,44 @@ func TestClientServerConsistentExit(t *testing.T) {
 		}
 	})
 }
+
+func TestClientServer_StepOutReturn(t *testing.T) {
+	ver, _ := goversion.Parse(runtime.Version())
+	if ver.Major >= 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}) {
+		t.Skip("return variables aren't marked on 1.9 or earlier")
+	}
+	withTestClient2("stepoutret", t, func(c service.Client) {
+		c.SetReturnValuesLoadConfig(&normalLoadConfig)
+		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.stepout", Line: -1})
+		assertNoError(err, t, "CreateBreakpoint()")
+		stateBefore := <-c.Continue()
+		assertNoError(stateBefore.Err, t, "Continue()")
+		stateAfter, err := c.StepOut()
+		assertNoError(err, t, "StepOut")
+		ret := stateAfter.CurrentThread.ReturnValues
+
+		if len(ret) != 2 {
+			t.Fatalf("wrong number of return values %v", ret)
+		}
+
+		if ret[0].Name != "str" {
+			t.Fatalf("(str) bad return value name %s", ret[0].Name)
+		}
+		if ret[0].Kind != reflect.String {
+			t.Fatalf("(str) bad return value kind %v", ret[0].Kind)
+		}
+		if ret[0].Value != "return 47" {
+			t.Fatalf("(str) bad return value %q", ret[0].Value)
+		}
+
+		if ret[1].Name != "num" {
+			t.Fatalf("(num) bad return value name %s", ret[1].Name)
+		}
+		if ret[1].Kind != reflect.Int {
+			t.Fatalf("(num) bad return value kind %v", ret[1].Kind)
+		}
+		if ret[1].Value != "48" {
+			t.Fatalf("(num) bad return value %s", ret[1].Value)
+		}
+	})
+}