proc: allow function calls to appear inside an expression (#1503)

The initial implementation of the 'call' command required the
function call to be the root expression, i.e. something like:

	double(3) + 1

was not allowed, because the root expression was the binary operator
'+', not the function call.

With this change expressions like the one above and others are
allowed.

This is the first step necessary to implement nested function calls
(where the result of a function call is used as argument to another
function call).

This is implemented by replacing proc.CallFunction with
proc.EvalExpressionWithCalls. EvalExpressionWithCalls will run
proc.(*EvalScope).EvalExpression in a different goroutine. This
goroutine, the 'eval' goroutine, will communicate with the main
goroutine of the debugger by means of two channels: continueRequest
and continueCompleted.

The eval goroutine evaluates the expression recursively, when
a function call is encountered it takes care of setting up the
function call on the target program and writes a request to the
continueRequest channel, this causes the 'main' goroutine to restart
the target program by calling proc.Continue.

Whenever Continue encounters a breakpoint that belongs to the
function call injection protocol (runtime.debugCallV1 and associated
functions) it writes to continueCompleted which resumes the 'eval'
goroutine.

The 'eval' goroutine takes care of implementing the function call
injection protocol.

When the expression is fully evaluated the 'eval' goroutine will
write a special message to 'continueRequest' signaling that the
expression evaluation is terminated which will cause Continue to
return to the user.

Updates #119

_fixtures/fncall.go

@@ -76,6 +76,21 @@ func escapeArg(pa2 *a2struct) {
 	globalPA2 = pa2
 }
 
+func square(x int) int {
+	return x * x
+}
+
+func intcallpanic(a int) int {
+	if a == 0 {
+		panic("panic requested")
+	}
+	return a
+}
+
+func onetwothree(n int) []int {
+	return []int{n + 1, n + 2, n + 3}
+}
+
 func main() {
 	one, two := 1, 2
 	intslice := []int{1, 2, 3}
@@ -98,5 +113,5 @@ func main() {
 	runtime.Breakpoint()
 	call1(one, two)
 	fn2clos(2)
-	fmt.Println(one, two, zero, callpanic, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2)
+	fmt.Println(one, two, zero, callpanic, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2, square, intcallpanic, onetwothree)
 }

pkg/proc/eval.go

@@ -23,8 +23,13 @@ var errOperationOnSpecialFloat = errors.New("operations on non-finite floats not
 
 // EvalExpression returns the value of the given expression.
 func (scope *EvalScope) EvalExpression(expr string, cfg LoadConfig) (*Variable, error) {
+	if scope.callCtx != nil {
+		// makes sure that the other goroutine won't wait forever if we make a mistake
+		defer close(scope.callCtx.continueRequest)
+	}
 	t, err := parser.ParseExpr(expr)
 	if err != nil {
+		scope.callCtx.doReturn(nil, err)
 		return nil, err
 	}
 
@@ -33,12 +38,14 @@ func (scope *EvalScope) EvalExpression(expr string, cfg LoadConfig) (*Variable,
 		ev, err = scope.evalAST(t)
 	}
 	if err != nil {
+		scope.callCtx.doReturn(nil, err)
 		return nil, err
 	}
 	ev.loadValue(cfg)
 	if ev.Name == "" {
 		ev.Name = expr
 	}
+	scope.callCtx.doReturn(ev, nil)
 	return ev, nil
 }
 
@@ -174,20 +181,11 @@ func (scope *EvalScope) evalAST(t ast.Expr) (*Variable, error) {
 	case *ast.CallExpr:
 		if len(node.Args) == 1 {
 			v, err := scope.evalTypeCast(node)
-			if err == nil {
-				return v, nil
-			}
-			_, isident := node.Fun.(*ast.Ident)
-			// we don't support function calls at the moment except for a few
-			// builtin functions so just return the type error here if the function
-			// isn't an identifier.
-			// More sophisticated logic will be required when function calls
-			// are implemented.
-			if err != reader.TypeNotFoundErr || !isident {
+			if err == nil || err != reader.TypeNotFoundErr {
 				return v, err
 			}
 		}
-		return scope.evalBuiltinCall(node)
+		return scope.evalFunctionCall(node)
 
 	case *ast.Ident:
 		return scope.evalIdent(node)
@@ -395,7 +393,7 @@ func convertInt(n uint64, signed bool, size int64) uint64 {
 func (scope *EvalScope) evalBuiltinCall(node *ast.CallExpr) (*Variable, error) {
 	fnnode, ok := node.Fun.(*ast.Ident)
 	if !ok {
-		return nil, fmt.Errorf("function calls are not supported")
+		return nil, nil
 	}
 
 	args := make([]*Variable, len(node.Args))
@@ -421,7 +419,7 @@ func (scope *EvalScope) evalBuiltinCall(node *ast.CallExpr) (*Variable, error) {
 		return realBuiltin(args, node.Args)
 	}
 
-	return nil, fmt.Errorf("function calls are not supported")
+	return nil, nil
 }
 
 func capBuiltin(args []*Variable, nodeargs []ast.Expr) (*Variable, error) {

pkg/proc/fncall.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/constant"
-	"go/parser"
 	"reflect"
 	"sort"
 
@@ -23,13 +22,18 @@ import (
 // The protocol is described in $GOROOT/src/runtime/asm_amd64.s in the
 // comments for function runtime·debugCallV1.
 //
-// There are two main entry points here. The first one is CallFunction which
-// evaluates a function call expression, sets up the function call on the
-// selected goroutine and resumes execution of the process.
+// The main entry point is EvalExpressionWithCalls which will start a goroutine to
+// evaluate the provided expression.
+// This goroutine can either return immediately, if no function calls were
+// needed, or write a continue request to the scope.callCtx.continueRequest
+// channel. When this happens EvalExpressionWithCalls will call Continue and
+// return.
 //
-// The second one is (*FunctionCallState).step() which is called every time
-// the process stops at a breakpoint inside one of the debug injcetion
-// functions.
+// The Continue loop will write to scope.callCtx.continueCompleted when it
+// hits a breakpoint in the call injection protocol.
+//
+// The work of setting up the function call and executing the protocol is
+// done by evalFunctionCall and funcCallStep.
 
 const (
 	debugCallFunctionNamePrefix1 = "debugCall"
@@ -49,17 +53,12 @@ var (
 	errNotEnoughArguments         = errors.New("not enough arguments")
 	errNoAddrUnsupported          = errors.New("arguments to a function call must have an address")
 	errNotAGoFunction             = errors.New("not a Go function")
+	errFuncCallNotAllowed         = errors.New("function calls not allowed without using 'call'")
 )
 
 type functionCallState struct {
-	// inProgress is true if a function call is in progress
-	inProgress bool
-	// finished is true if the function call terminated
-	finished bool
 	// savedRegs contains the saved registers
 	savedRegs Registers
-	// expr contains an expression describing the current function call
-	expr string
 	// err contains a saved error
 	err error
 	// fn is the function that is being called
@@ -77,21 +76,56 @@ type functionCallState struct {
 	panicvar *Variable
 }
 
-// CallFunction starts a debugger injected function call on the current thread of p.
-// See runtime.debugCallV1 in $GOROOT/src/runtime/asm_amd64.s for a
-// description of the protocol.
-func CallFunction(p Process, expr string, retLoadCfg *LoadConfig, checkEscape bool) error {
+type callContext struct {
+	p Process
+
+	// checkEscape is true if the escape check should be performed.
+	// See service/api.DebuggerCommand.UnsafeCall in service/api/types.go.
+	checkEscape bool
+
+	// retLoadCfg is the load configuration used to load return values
+	retLoadCfg LoadConfig
+
+	// Write to continueRequest to request a call to Continue from the
+	// debugger's main goroutine.
+	// Read from continueCompleted to wait for the target process to stop at
+	// one of the interaction point of the function call protocol.
+	// To signal that evaluation is completed a value will be written to
+	// continueRequest having cont == false and the return values in ret.
+	continueRequest   chan<- continueRequest
+	continueCompleted <-chan struct{}
+}
+
+type continueRequest struct {
+	cont bool
+	err  error
+	ret  *Variable
+}
+
+func (callCtx *callContext) doContinue() {
+	callCtx.continueRequest <- continueRequest{cont: true}
+	<-callCtx.continueCompleted
+}
+
+func (callCtx *callContext) doReturn(ret *Variable, err error) {
+	if callCtx == nil {
+		return
+	}
+	callCtx.continueRequest <- continueRequest{cont: false, ret: ret, err: err}
+}
+
+// EvalExpressionWithCalls is like EvalExpression but allows function calls in 'expr'.
+// Because this can only be done in the current goroutine, unlike
+// EvalExpression, EvalExpressionWithCalls is not a method of EvalScope.
+func EvalExpressionWithCalls(p Process, expr string, retLoadCfg LoadConfig, checkEscape bool) error {
 	bi := p.BinInfo()
 	if !p.Common().fncallEnabled {
 		return errFuncCallUnsupportedBackend
 	}
-	fncall := &p.Common().fncallState
-	if fncall.inProgress {
+	if p.Common().continueCompleted != nil {
 		return errFuncCallInProgress
 	}
 
-	*fncall = functionCallState{}
-
 	dbgcallfn := bi.LookupFunc[debugCallFunctionName]
 	if dbgcallfn == nil {
 		return errFuncCallUnsupported
@@ -106,49 +140,217 @@ func CallFunction(p Process, expr string, retLoadCfg *LoadConfig, checkEscape bo
 		return errGoroutineNotRunning
 	}
 
+	scope, err := GoroutineScope(p.CurrentThread())
+	if err != nil {
+		return err
+	}
+
+	continueRequest := make(chan continueRequest)
+	continueCompleted := make(chan struct{})
+
+	scope.callCtx = &callContext{
+		p:                 p,
+		checkEscape:       checkEscape,
+		retLoadCfg:        retLoadCfg,
+		continueRequest:   continueRequest,
+		continueCompleted: continueCompleted,
+	}
+
+	p.Common().continueRequest = continueRequest
+	p.Common().continueCompleted = continueCompleted
+
+	go scope.EvalExpression(expr, retLoadCfg)
+
+	contReq, ok := <-continueRequest
+	if contReq.cont {
+		return Continue(p)
+	}
+
+	return finishEvalExpressionWithCalls(p, contReq, ok)
+}
+
+func finishEvalExpressionWithCalls(p Process, contReq continueRequest, ok bool) error {
+	var err error
+	if !ok {
+		err = errors.New("internal error EvalExpressionWithCalls didn't return anything")
+	} else if contReq.err != nil {
+		if fpe, ispanic := contReq.err.(fncallPanicErr); ispanic {
+			p.CurrentThread().Common().returnValues = []*Variable{fpe.panicVar}
+		} else {
+			err = contReq.err
+		}
+	} else if contReq.ret.Addr == 0 && contReq.ret.DwarfType == nil {
+		// this is a variable returned by a function call with multiple return values
+		r := make([]*Variable, len(contReq.ret.Children))
+		for i := range contReq.ret.Children {
+			r[i] = &contReq.ret.Children[i]
+		}
+		p.CurrentThread().Common().returnValues = r
+	} else {
+		p.CurrentThread().Common().returnValues = []*Variable{contReq.ret}
+	}
+
+	p.Common().continueRequest = nil
+	close(p.Common().continueCompleted)
+	p.Common().continueCompleted = nil
+	return err
+}
+
+// evalFunctionCall evaluates a function call.
+// If this is a built-in function it's evaluated directly.
+// Otherwise this will start the function call injection protocol and
+// request that the target process resumes.
+// See the comment describing the field EvalScope.callCtx for a description
+// of the preconditions that make starting the function call protocol
+// possible.
+// See runtime.debugCallV1 in $GOROOT/src/runtime/asm_amd64.s for a
+// description of the protocol.
+func (scope *EvalScope) evalFunctionCall(node *ast.CallExpr) (*Variable, error) {
+	r, err := scope.evalBuiltinCall(node)
+	if r != nil || err != nil {
+		// it was a builtin call
+		return r, err
+	}
+	if scope.callCtx == nil {
+		return nil, errFuncCallNotAllowed
+	}
+
+	p := scope.callCtx.p
+	bi := scope.BinInfo
+	if !p.Common().fncallEnabled {
+		return nil, errFuncCallUnsupportedBackend
+	}
+	if p.Common().callInProgress {
+		return nil, errFuncCallInProgress
+	}
+
+	p.Common().callInProgress = true
+	defer func() {
+		p.Common().callInProgress = false
+	}()
+
+	dbgcallfn := bi.LookupFunc[debugCallFunctionName]
+	if dbgcallfn == nil {
+		return nil, errFuncCallUnsupported
+	}
+
+	// check that the selected goroutine is running
+	g := p.SelectedGoroutine()
+	if g == nil {
+		return nil, errNoGoroutine
+	}
+	if g.Status != Grunning || g.Thread == nil {
+		return nil, errGoroutineNotRunning
+	}
+
 	// check that there are at least 256 bytes free on the stack
 	regs, err := g.Thread.Registers(true)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	regs = regs.Copy()
 	if regs.SP()-256 <= g.stacklo {
-		return errNotEnoughStack
+		return nil, errNotEnoughStack
 	}
 	_, err = regs.Get(int(x86asm.RAX))
 	if err != nil {
-		return errFuncCallUnsupportedBackend
+		return nil, errFuncCallUnsupportedBackend
 	}
 
-	fn, closureAddr, argvars, err := funcCallEvalExpr(p, expr)
+	fn, closureAddr, argvars, err := scope.funcCallEvalExpr(node)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	argmem, err := funcCallArgFrame(fn, argvars, g, bi, checkEscape)
+	argmem, err := funcCallArgFrame(fn, argvars, g, bi, scope.callCtx.checkEscape)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	if err := callOP(bi, g.Thread, regs, dbgcallfn.Entry); err != nil {
-		return err
+		return nil, err
 	}
 	// write the desired argument frame size at SP-(2*pointer_size) (the extra pointer is the saved PC)
 	if err := writePointer(bi, g.Thread, regs.SP()-3*uint64(bi.Arch.PtrSize()), uint64(len(argmem))); err != nil {
-		return err
+		return nil, err
 	}
 
-	fncall.inProgress = true
-	fncall.savedRegs = regs
-	fncall.expr = expr
-	fncall.fn = fn
-	fncall.closureAddr = closureAddr
-	fncall.argmem = argmem
-	fncall.retLoadCfg = retLoadCfg
+	fncall := functionCallState{
+		savedRegs:   regs,
+		fn:          fn,
+		closureAddr: closureAddr,
+		argmem:      argmem,
+		retLoadCfg:  &scope.callCtx.retLoadCfg,
+	}
 
 	fncallLog("function call initiated %v frame size %d\n", fn, len(argmem))
 
-	return Continue(p)
+	spoff := int64(scope.Regs.Uint64Val(scope.Regs.SPRegNum)) - int64(g.stackhi)
+	bpoff := int64(scope.Regs.Uint64Val(scope.Regs.BPRegNum)) - int64(g.stackhi)
+	fboff := scope.Regs.FrameBase - int64(g.stackhi)
+
+	for {
+		scope.callCtx.doContinue()
+
+		g = p.SelectedGoroutine()
+		if g != nil {
+			// adjust the value of registers inside scope
+			for regnum := range scope.Regs.Regs {
+				switch uint64(regnum) {
+				case scope.Regs.PCRegNum, scope.Regs.SPRegNum, scope.Regs.BPRegNum:
+					// leave these alone
+				default:
+					// every other register is dirty and unrecoverable
+					scope.Regs.Regs[regnum] = nil
+				}
+			}
+
+			scope.Regs.Regs[scope.Regs.SPRegNum].Uint64Val = uint64(spoff + int64(g.stackhi))
+			scope.Regs.Regs[scope.Regs.BPRegNum].Uint64Val = uint64(bpoff + int64(g.stackhi))
+			scope.Regs.FrameBase = fboff + int64(g.stackhi)
+			scope.Regs.CFA = scope.frameOffset + int64(g.stackhi)
+		}
+
+		finished := funcCallStep(scope, &fncall)
+		if finished {
+			break
+		}
+	}
+
+	if fncall.err != nil {
+		return nil, fncall.err
+	}
+
+	if fncall.panicvar != nil {
+		return nil, fncallPanicErr{fncall.panicvar}
+	}
+	switch len(fncall.retvars) {
+	case 0:
+		r := scope.newVariable("", 0, nil, nil)
+		r.loaded = true
+		r.Unreadable = errors.New("no return values")
+		return r, nil
+	case 1:
+		return fncall.retvars[0], nil
+	default:
+		// create a fake variable without address or type to return multiple values
+		r := scope.newVariable("", 0, nil, nil)
+		r.loaded = true
+		r.Children = make([]Variable, len(fncall.retvars))
+		for i := range fncall.retvars {
+			r.Children[i] = *fncall.retvars[i]
+		}
+		return r, nil
+	}
+}
+
+// fncallPanicErr is the error returned if a called function panics
+type fncallPanicErr struct {
+	panicVar *Variable
+}
+
+func (err fncallPanicErr) Error() string {
+	return fmt.Sprintf("panic calling a function")
 }
 
 func fncallLog(fmtstr string, args ...interface{}) {
@@ -191,21 +393,8 @@ func callOP(bi *BinaryInfo, thread Thread, regs Registers, callAddr uint64) erro
 
 // funcCallEvalExpr evaluates expr, which must be a function call, returns
 // the function being called and its arguments.
-func funcCallEvalExpr(p Process, expr string) (fn *Function, closureAddr uint64, argvars []*Variable, err error) {
-	bi := p.BinInfo()
-	scope, err := GoroutineScope(p.CurrentThread())
-	if err != nil {
-		return nil, 0, nil, err
-	}
-
-	t, err := parser.ParseExpr(expr)
-	if err != nil {
-		return nil, 0, nil, err
-	}
-	callexpr, iscall := t.(*ast.CallExpr)
-	if !iscall {
-		return nil, 0, nil, errNotACallExpr
-	}
+func (scope *EvalScope) funcCallEvalExpr(callexpr *ast.CallExpr) (fn *Function, closureAddr uint64, argvars []*Variable, err error) {
+	bi := scope.BinInfo
 
 	fnvar, err := scope.evalAST(callexpr.Fun)
 	if err != nil {
@@ -395,16 +584,16 @@ const (
 	debugCallAXRestoreRegisters = 16
 )
 
-func (fncall *functionCallState) step(p Process) {
+// funcCallStep executes one step of the function call injection protocol.
+func funcCallStep(scope *EvalScope, fncall *functionCallState) bool {
+	p := scope.callCtx.p
 	bi := p.BinInfo()
 
 	thread := p.CurrentThread()
 	regs, err := thread.Registers(false)
 	if err != nil {
 		fncall.err = err
-		fncall.finished = true
-		fncall.inProgress = false
-		return
+		return true
 	}
 	regs = regs.Copy()
 
@@ -453,7 +642,6 @@ func (fncall *functionCallState) step(p Process) {
 	case debugCallAXRestoreRegisters:
 		// runtime requests that we restore the registers (all except pc and sp),
 		// this is also the last step of the function call protocol.
-		fncall.finished = true
 		pc, sp := regs.PC(), regs.SP()
 		if err := thread.RestoreRegisters(fncall.savedRegs); err != nil {
 			fncall.err = fmt.Errorf("could not restore registers: %v", err)
@@ -467,6 +655,7 @@ func (fncall *functionCallState) step(p Process) {
 		if err := stepInstructionOut(p, thread, debugCallFunctionName, debugCallFunctionName); err != nil {
 			fncall.err = fmt.Errorf("could not step out of %s: %v", debugCallFunctionName, err)
 		}
+		return true
 
 	case debugCallAXReadReturn:
 		// read return arguments from stack
@@ -496,7 +685,7 @@ func (fncall *functionCallState) step(p Process) {
 	case debugCallAXReadPanic:
 		// read panic value from stack
 		if fncall.retLoadCfg == nil {
-			return
+			return false
 		}
 		fncall.panicvar, err = readTopstackVariable(thread, regs, "interface {}", *fncall.retLoadCfg)
 		if err != nil {
@@ -515,6 +704,8 @@ func (fncall *functionCallState) step(p Process) {
 		// possible is to ignore it and hope it didn't matter.
 		fncallLog("unknown value of AX %#x", rax)
 	}
+
+	return false
 }
 
 func readTopstackVariable(thread Thread, regs Registers, typename string, loadCfg LoadConfig) (*Variable, error) {

pkg/proc/interface.go

@@ -115,8 +115,19 @@ type BreakpointManipulation interface {
 // implementations of the Process interface.
 type CommonProcess struct {
 	allGCache     []*G
-	fncallState   functionCallState
 	fncallEnabled bool
+
+	// if continueCompleted is not nil it means we are in the process of
+	// executing an injected function call, see comments throughout
+	// pkg/proc/fncall.go for a description of how this works.
+	continueCompleted chan<- struct{}
+	continueRequest   <-chan continueRequest
+
+	// callInProgress is true when a function call is being injected in the
+	// target process.
+	// This is only used to prevent nested function calls, it should be removed
+	// when we add support for them.
+	callInProgress bool
 }
 
 // NewCommonProcess returns a struct with fields common across

pkg/proc/proc.go

@@ -228,18 +228,18 @@ func Continue(dbp Process) error {
 				}
 				return conditionErrors(threads)
 			case strings.HasPrefix(loc.Fn.Name, debugCallFunctionNamePrefix1) || strings.HasPrefix(loc.Fn.Name, debugCallFunctionNamePrefix2):
-				fncall := &dbp.Common().fncallState
-				if !fncall.inProgress {
+				continueCompleted := dbp.Common().continueCompleted
+				if continueCompleted == nil {
 					return conditionErrors(threads)
 				}
-				fncall.step(dbp)
-				// only stop execution if the function call finished
-				if fncall.finished {
-					fncall.inProgress = false
-					if fncall.err != nil {
-						return fncall.err
+				continueCompleted <- struct{}{}
+				contReq, ok := <-dbp.Common().continueRequest
+				if !contReq.cont {
+					// only stop execution if the expression evaluation with calls finished
+					err := finishEvalExpressionWithCalls(dbp, contReq, ok)
+					if err != nil {
+						return err
 					}
-					curthread.Common().returnValues = fncall.returnValues()
 					return conditionErrors(threads)
 				}
 			default:

pkg/proc/proc_test.go

@@ -4123,7 +4123,7 @@ func TestIssue1374(t *testing.T) {
 		setFileBreakpoint(p, t, fixture, 7)
 		assertNoError(proc.Continue(p), t, "First Continue")
 		assertLineNumber(p, t, 7, "Did not continue to correct location (first continue),")
-		assertNoError(proc.CallFunction(p, "getNum()", &normalLoadConfig, true), t, "Call")
+		assertNoError(proc.EvalExpressionWithCalls(p, "getNum()", normalLoadConfig, true), t, "Call")
 		err := proc.Continue(p)
 		if _, isexited := err.(proc.ErrProcessExited); !isexited {
 			regs, _ := p.CurrentThread().Registers(false)
@@ -4328,7 +4328,7 @@ func TestCallConcurrent(t *testing.T) {
 
 		gid1 := p.SelectedGoroutine().ID
 		t.Logf("starting injection in %d / %d", p.SelectedGoroutine().ID, p.CurrentThread().ThreadID())
-		assertNoError(proc.CallFunction(p, "Foo(10, 1)", &normalLoadConfig, false), t, "EvalExpressionWithCalls()")
+		assertNoError(proc.EvalExpressionWithCalls(p, "Foo(10, 1)", normalLoadConfig, false), t, "EvalExpressionWithCalls()")
 
 		returned := testCallConcurrentCheckReturns(p, t, gid1)
 

pkg/proc/variables.go

@@ -221,6 +221,20 @@ type EvalScope struct {
 	frameOffset int64
 
 	aordr *dwarf.Reader // extra reader to load DW_AT_abstract_origin entries, do not initialize
+
+	// When the following pointer is not nil this EvalScope was created
+	// by CallFunction and the expression evaluation is executing on a
+	// different goroutine from the debugger's main goroutine.
+	// Under this circumstance the expression evaluator can make function
+	// calls by setting up the runtime.debugCallV1 call and then writing a
+	// value to the continueRequest channel.
+	// When a value is written to continueRequest the debugger's main goroutine
+	// will call Continue, when the runtime in the target process sends us a
+	// request in the function call protocol the debugger's main goroutine will
+	// write a value to the continueCompleted channel.
+	// The goroutine executing the expression evaluation shall signal that the
+	// evaluation is complete by closing the continueRequest channel.
+	callCtx *callContext
 }
 
 // IsNilErr is returned when a variable is nil.

service/api/types.go

@@ -311,7 +311,20 @@ type DebuggerCommand struct {
 	ReturnInfoLoadConfig *LoadConfig
 	// Expr is the expression argument for a Call command
 	Expr string `json:"expr,omitempty"`
-	// UnsafeCall disabled parameter escape checking for function calls
+
+	// UnsafeCall disables parameter escape checking for function calls.
+	// Go objects can be allocated on the stack or on the heap. Heap objects
+	// can be used by any goroutine; stack objects can only be used by the
+	// goroutine that owns the stack they are allocated on and can not surivive
+	// the stack frame of allocation.
+	// The Go compiler will use escape analysis to determine whether to
+	// allocate an object on the stack or the heap.
+	// When injecting a function call Delve will check that no address of a
+	// stack allocated object is passed to the called function: this ensures
+	// the rules for stack objects will not be violated.
+	// If you are absolutely sure that the function you are calling will not
+	// violate the rules about stack objects you can disable this safety check
+	// by setting UnsafeCall to true.
 	UnsafeCall bool `json:"unsafeCall,omitempty"`
 }
 

service/debugger/debugger.go

@@ -599,7 +599,10 @@ func (d *Debugger) Command(command *api.DebuggerCommand) (*api.DebuggerState, er
 		err = proc.Continue(d.target)
 	case api.Call:
 		d.log.Debugf("function call %s", command.Expr)
-		err = proc.CallFunction(d.target, command.Expr, api.LoadConfigToProc(command.ReturnInfoLoadConfig), !command.UnsafeCall)
+		if command.ReturnInfoLoadConfig == nil {
+			return nil, errors.New("can not call function with nil ReturnInfoLoadConfig")
+		}
+		err = proc.EvalExpressionWithCalls(d.target, command.Expr, *api.LoadConfigToProc(command.ReturnInfoLoadConfig), !command.UnsafeCall)
 	case api.Rewind:
 		d.log.Debug("rewinding")
 		if err := d.target.Direction(proc.Backward); err != nil {

service/test/integration2_test.go

@@ -1584,6 +1584,7 @@ func TestClientServerFunctionCallBadPos(t *testing.T) {
 		state = <-c.Continue()
 		assertNoError(state.Err, t, "Continue()")
 
+		c.SetReturnValuesLoadConfig(&normalLoadConfig)
 		state, err = c.Call("main.call1(main.zero, main.zero)", false)
 		if err == nil || err.Error() != "call not at safe point" {
 			t.Fatalf("wrong error or no error: %v", err)

service/test/variables_test.go

@@ -737,7 +737,7 @@ func TestEvalExpression(t *testing.T) {
 		{"i2 << i3", false, "", "", "int", fmt.Errorf("shift count type int, must be unsigned integer")},
 		{"*(i2 + i3)", false, "", "", "", fmt.Errorf("expression \"(i2 + i3)\" (int) can not be dereferenced")},
 		{"i2.member", false, "", "", "", fmt.Errorf("i2 (type int) is not a struct")},
-		{"fmt.Println(\"hello\")", false, "", "", "", fmt.Errorf("no type entry found, use 'types' for a list of valid types")},
+		{"fmt.Println(\"hello\")", false, "", "", "", fmt.Errorf("function calls not allowed without using 'call'")},
 		{"*nil", false, "", "", "", fmt.Errorf("nil can not be dereferenced")},
 		{"!nil", false, "", "", "", fmt.Errorf("operator ! can not be applied to \"nil\"")},
 		{"&nil", false, "", "", "", fmt.Errorf("can not take address of \"nil\"")},
@@ -1093,6 +1093,8 @@ func TestCallFunction(t *testing.T) {
 		outs []string // list of return parameters in this format: <param name>:<param type>:<param value>
 		err  error    // if not nil should return an error
 	}{
+		// Basic function call injection tests
+
 		{"call1(one, two)", []string{":int:3"}, nil},
 		{"call1(one+two, 4)", []string{":int:7"}, nil},
 		{"callpanic()", []string{`~panic:interface {}:interface {}(string) "callpanic panicked"`}, nil},
@@ -1102,6 +1104,13 @@ func TestCallFunction(t *testing.T) {
 		{`stringsJoin(s1, comma)`, nil, errors.New("could not find symbol value for s1")},
 		{`stringsJoin(intslice, comma)`, nil, errors.New("can not convert value of type []int to []string")},
 
+		// Expression tests
+		{`square(2) + 1`, []string{":int:5"}, nil},
+		{`intcallpanic(1) + 1`, []string{":int:2"}, nil},
+		{`intcallpanic(0) + 1`, []string{`~panic:interface {}:interface {}(string) "panic requested"`}, nil},
+		{`onetwothree(5)[1] + 2`, []string{":int:9"}, nil},
+
+		// Call types tests (methods, function pointers, etc.)
 		// The following set of calls was constructed using https://docs.google.com/document/d/1bMwCey-gmqZVTpRax-ESeVuZGmjwbocYs1iHplK-cjo/pub as a reference
 
 		{`a.VRcvr(1)`, []string{`:string:"1 + 3 = 4"`}, nil}, // direct call of a method with value receiver / on a value
@@ -1130,6 +1139,8 @@ func TestCallFunction(t *testing.T) {
 
 		{"ga.PRcvr(2)", []string{`:string:"2 - 0 = 2"`}, nil},
 
+		// Escape tests
+
 		{"escapeArg(&a2)", nil, errors.New("cannot use &a2 as argument pa2 in function main.escapeArg: stack object passed to escaping pointer: pa2")},
 
 		{"-unsafe escapeArg(&a2)", nil, nil}, // LEAVE THIS AS THE LAST ITEM, IT BREAKS THE TARGET PROCESS!!!
@@ -1151,9 +1162,9 @@ func TestCallFunction(t *testing.T) {
 				checkEscape = false
 			}
 			t.Logf("call %q", tc.expr)
-			err := proc.CallFunction(p, expr, &pnormalLoadConfig, checkEscape)
+			err := proc.EvalExpressionWithCalls(p, expr, pnormalLoadConfig, checkEscape)
 			if tc.err != nil {
-
+				t.Logf("\terr = %v\n", err)
 				if err == nil {
 					t.Fatalf("call %q: expected error %q, got no error", tc.expr, tc.err.Error())
 				}