proc: support setting string values when it requires an allocation (#1548)

Allow changing the value of a string variable to a new literal string,
which requires calling runtime.mallocgc to allocate the string into the
target process.

This means that a command like:

    call f("some string")

is now supported.

Additionally the command:

    call s = "some string"

is also supported.

Fixes #826

_fixtures/fncall.go

@@ -117,6 +117,10 @@ func getVRcvrableFromAStructPtr(n int) VRcvrable {
 	return &astruct{X: n}
 }
 
+func noreturncall(n int) {
+	return
+}
+
 func main() {
 	one, two := 1, 2
 	intslice := []int{1, 2, 3}
@@ -125,6 +129,8 @@ func main() {
 	a := astruct{X: 3}
 	pa := &astruct{X: 6}
 	a2 := a2struct{Y: 7}
+	var pa2 *astruct
+	var str string = "old string value"
 
 	var vable_a VRcvrable = a
 	var vable_pa VRcvrable = pa
@@ -139,5 +145,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, square, intcallpanic, onetwothree, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr)
+	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, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr, pa2, noreturncall, str)
 }

pkg/dwarf/reader/variables.go

@@ -17,23 +17,25 @@ func ToRelAddr(addr uint64, staticBase uint64) RelAddr {
 // VariableReader provides a way of reading the local variables and formal
 // parameters of a function that are visible at the specified PC address.
 type VariableReader struct {
-	dwarf       *dwarf.Data
+	dwarf  *dwarf.Data
-	reader      *dwarf.Reader
+	reader *dwarf.Reader
-	entry       *dwarf.Entry
+	entry  *dwarf.Entry
-	depth       int
+	depth  int
-	onlyVisible bool
+	pc     uint64
-	pc          uint64
+	line   int
-	line        int
+	err    error
-	err         error
+
+	onlyVisible            bool
+	skipInlinedSubroutines bool
 }
 
 // Variables returns a VariableReader for the function or lexical block at off.
 // If onlyVisible is true only variables visible at pc will be returned by
 // the VariableReader.
-func Variables(dwarf *dwarf.Data, off dwarf.Offset, pc RelAddr, line int, onlyVisible bool) *VariableReader {
+func Variables(dwarf *dwarf.Data, off dwarf.Offset, pc RelAddr, line int, onlyVisible, skipInlinedSubroutines bool) *VariableReader {
 	reader := dwarf.Reader()
 	reader.Seek(off)
-	return &VariableReader{dwarf: dwarf, reader: reader, entry: nil, depth: 0, onlyVisible: onlyVisible, pc: uint64(pc), line: line, err: nil}
+	return &VariableReader{dwarf: dwarf, reader: reader, entry: nil, depth: 0, onlyVisible: onlyVisible, skipInlinedSubroutines: skipInlinedSubroutines, pc: uint64(pc), line: line, err: nil}
 }
 
 // Next reads the next variable entry, returns false if there aren't any.
@@ -55,7 +57,14 @@ func (vrdr *VariableReader) Next() bool {
 				return false
 			}
 
-		case dwarf.TagLexDwarfBlock, dwarf.TagSubprogram, dwarf.TagInlinedSubroutine:
+		case dwarf.TagInlinedSubroutine:
+			if vrdr.skipInlinedSubroutines {
+				vrdr.reader.SkipChildren()
+				continue
+			}
+			fallthrough
+		case dwarf.TagLexDwarfBlock, dwarf.TagSubprogram:
+
 			recur := true
 			if vrdr.onlyVisible {
 				recur, vrdr.err = entryRangesContains(vrdr.dwarf, vrdr.entry, vrdr.pc)

pkg/proc/eval.go

@@ -9,6 +9,7 @@ import (
 	"go/constant"
 	"go/parser"
 	"go/printer"
+	"go/scanner"
 	"go/token"
 	"reflect"
 	"strconv"
@@ -28,6 +29,13 @@ func (scope *EvalScope) EvalExpression(expr string, cfg LoadConfig) (*Variable,
 		defer close(scope.callCtx.continueRequest)
 	}
 	t, err := parser.ParseExpr(expr)
+	if eqOff, isAs := isAssignment(err); scope.callCtx != nil && isAs {
+		lexpr := expr[:eqOff]
+		rexpr := expr[eqOff+1:]
+		err := scope.SetVariable(lexpr, rexpr)
+		scope.callCtx.doReturn(nil, err)
+		return nil, err
+	}
 	if err != nil {
 		scope.callCtx.doReturn(nil, err)
 		return nil, err
@@ -49,6 +57,14 @@ func (scope *EvalScope) EvalExpression(expr string, cfg LoadConfig) (*Variable,
 	return ev, nil
 }
 
+func isAssignment(err error) (int, bool) {
+	el, isScannerErr := err.(scanner.ErrorList)
+	if isScannerErr && el[0].Msg == "expected '==', found '='" {
+		return el[0].Pos.Offset, true
+	}
+	return 0, false
+}
+
 // evalToplevelTypeCast implements certain type casts that we only support
 // at the outermost levels of an expression.
 func (scope *EvalScope) evalToplevelTypeCast(t ast.Expr, cfg LoadConfig) (*Variable, error) {

pkg/proc/fncall.go

@@ -7,12 +7,15 @@ import (
 	"fmt"
 	"go/ast"
 	"go/constant"
+	"go/token"
 	"reflect"
 	"sort"
+	"strconv"
 
 	"github.com/go-delve/delve/pkg/dwarf/godwarf"
 	"github.com/go-delve/delve/pkg/dwarf/op"
 	"github.com/go-delve/delve/pkg/dwarf/reader"
+	"github.com/go-delve/delve/pkg/goversion"
 	"github.com/go-delve/delve/pkg/logflags"
 	"golang.org/x/arch/x86/x86asm"
 )
@@ -55,6 +58,7 @@ var (
 	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'")
+	errFuncCallNotAllowedStrAlloc = errors.New("literal string can not be allocated because function calls are not allowed without using 'call'")
 )
 
 type functionCallState struct {
@@ -187,6 +191,8 @@ func finishEvalExpressionWithCalls(p Process, contReq continueRequest, ok bool)
 		} else {
 			err = contReq.err
 		}
+	} else if contReq.ret == nil {
+		p.CurrentThread().Common().returnValues = nil
 	} 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))
@@ -502,7 +508,7 @@ func funcCallCopyOneArg(g *G, scope *EvalScope, fncall *functionCallState, actua
 	// by convertToEface.
 
 	formalArgVar := newVariable(formalArg.name, uintptr(formalArg.off+int64(argFrameAddr)), formalArg.typ, scope.BinInfo, scope.Mem)
-	if err := formalArgVar.setValue(actualArg, actualArg.Name); err != nil {
+	if err := scope.setValue(formalArgVar, actualArg, actualArg.Name); err != nil {
 		return err
 	}
 
@@ -511,7 +517,9 @@ func funcCallCopyOneArg(g *G, scope *EvalScope, fncall *functionCallState, actua
 
 func funcCallArgs(fn *Function, bi *BinaryInfo, includeRet bool) (argFrameSize int64, formalArgs []funcCallArg, err error) {
 	const CFA = 0x1000
-	vrdr := reader.Variables(fn.cu.image.dwarf, fn.offset, reader.ToRelAddr(fn.Entry, fn.cu.image.StaticBase), int(^uint(0)>>1), false)
+	vrdr := reader.Variables(fn.cu.image.dwarf, fn.offset, reader.ToRelAddr(fn.Entry, fn.cu.image.StaticBase), int(^uint(0)>>1), false, true)
+
+	trustArgOrder := bi.Producer() != "" && goversion.ProducerAfterOrEqual(bi.Producer(), 1, 12)
 
 	// typechecks arguments, calculates argument frame size
 	for vrdr.Next() {
@@ -524,16 +532,23 @@ func funcCallArgs(fn *Function, bi *BinaryInfo, includeRet bool) (argFrameSize i
 			return 0, nil, err
 		}
 		typ = resolveTypedef(typ)
-		locprog, _, err := bi.locationExpr(entry, dwarf.AttrLocation, fn.Entry)
+		var off int64
-		if err != nil {
+		if trustArgOrder && fn.Name == "runtime.mallocgc" {
-			return 0, nil, fmt.Errorf("could not get argument location of %s: %v", argname, err)
+			// runtime is always optimized and optimized code sometimes doesn't have
-		}
+			// location info for arguments, but we still want to call runtime.mallocgc.
-		off, _, err := op.ExecuteStackProgram(op.DwarfRegisters{CFA: CFA, FrameBase: CFA}, locprog)
+			off = argFrameSize
-		if err != nil {
+		} else {
-			return 0, nil, fmt.Errorf("unsupported location expression for argument %s: %v", argname, err)
+			locprog, _, err := bi.locationExpr(entry, dwarf.AttrLocation, fn.Entry)
-		}
+			if err != nil {
+				return 0, nil, fmt.Errorf("could not get argument location of %s: %v", argname, err)
+			}
+			off, _, err = op.ExecuteStackProgram(op.DwarfRegisters{CFA: CFA, FrameBase: CFA}, locprog)
+			if err != nil {
+				return 0, nil, fmt.Errorf("unsupported location expression for argument %s: %v", argname, err)
+			}
 
-		off -= CFA
+			off -= CFA
+		}
 
 		if e := off + typ.Size(); e > argFrameSize {
 			argFrameSize = e
@@ -731,6 +746,14 @@ func funcCallStep(callScope *EvalScope, fncall *functionCallState) bool {
 		fncall.retvars = filterVariables(fncall.retvars, func(v *Variable) bool {
 			return (v.Flags & VariableReturnArgument) != 0
 		})
+		if fncall.fn.Name == "runtime.mallocgc" && fncall.retvars[0].Unreadable != nil {
+			// return values never have a location for optimized functions and the
+			// runtime is always optimized. However we want to call runtime.mallocgc,
+			// so we fix the address of the return value manually.
+			fncall.retvars[0].Unreadable = nil
+			lastArg := fncall.formalArgs[len(fncall.formalArgs)-1]
+			fncall.retvars[0].Addr = uintptr(retScope.Regs.CFA + lastArg.off + int64(bi.Arch.PtrSize()))
+		}
 
 		loadValues(fncall.retvars, callScope.callCtx.retLoadCfg)
 
@@ -803,3 +826,46 @@ func (fncall *functionCallState) returnValues() []*Variable {
 	}
 	return fncall.retvars
 }
+
+// allocString allocates spaces for the contents of v if it needs to be allocated
+func (scope *EvalScope) allocString(v *Variable) error {
+	if v.Base != 0 || v.Len == 0 {
+		// already allocated
+		return nil
+	}
+
+	if scope.callCtx == nil {
+		return errFuncCallNotAllowedStrAlloc
+	}
+	savedLoadCfg := scope.callCtx.retLoadCfg
+	scope.callCtx.retLoadCfg = loadFullValue
+	defer func() {
+		scope.callCtx.retLoadCfg = savedLoadCfg
+	}()
+	mallocv, err := scope.evalFunctionCall(&ast.CallExpr{
+		Fun: &ast.SelectorExpr{
+			X:   &ast.Ident{Name: "runtime"},
+			Sel: &ast.Ident{Name: "mallocgc"},
+		},
+		Args: []ast.Expr{
+			&ast.BasicLit{Kind: token.INT, Value: strconv.Itoa(int(v.Len))},
+			&ast.Ident{Name: "nil"},
+			&ast.Ident{Name: "false"},
+		},
+	})
+	if err != nil {
+		return err
+	}
+	if mallocv.Unreadable != nil {
+		return mallocv.Unreadable
+	}
+	if mallocv.DwarfType.String() != "*void" {
+		return fmt.Errorf("unexpected return type for mallocgc call: %v", mallocv.DwarfType.String())
+	}
+	if len(mallocv.Children) != 1 {
+		return errors.New("internal error, could not interpret return value of mallocgc call")
+	}
+	v.Base = uintptr(mallocv.Children[0].Addr)
+	_, err = scope.Mem.WriteMemory(v.Base, []byte(constant.StringVal(v.Value)))
+	return err
+}

pkg/proc/variables.go

@@ -806,7 +806,7 @@ func (scope *EvalScope) SetVariable(name, value string) error {
 		return err
 	}
 
-	return xv.setValue(yv, value)
+	return scope.setValue(xv, yv, value)
 }
 
 // LocalVariables returns all local variables from the current function scope.
@@ -1211,13 +1211,13 @@ func (v *Variable) loadValueInternal(recurseLevel int, cfg LoadConfig) {
 //   is performed.
 // * If srcv and dstv have the same type and are both addressable then the
 //   contents of srcv are copied byte-by-byte into dstv
-func (v *Variable) setValue(srcv *Variable, srcExpr string) error {
+func (scope *EvalScope) setValue(dstv, srcv *Variable, srcExpr string) error {
 	srcv.loadValue(loadSingleValue)
 
-	typerr := srcv.isType(v.RealType, v.Kind)
+	typerr := srcv.isType(dstv.RealType, dstv.Kind)
 	if _, isTypeConvErr := typerr.(*typeConvErr); isTypeConvErr {
 		// attempt iface -> eface and ptr-shaped -> eface conversions.
-		return convertToEface(srcv, v)
+		return convertToEface(srcv, dstv)
 	}
 	if typerr != nil {
 		return typerr
@@ -1228,51 +1228,53 @@ func (v *Variable) setValue(srcv *Variable, srcExpr string) error {
 	}
 
 	// Numerical types
-	switch v.Kind {
+	switch dstv.Kind {
 	case reflect.Float32, reflect.Float64:
 		f, _ := constant.Float64Val(srcv.Value)
-		return v.writeFloatRaw(f, v.RealType.Size())
+		return dstv.writeFloatRaw(f, dstv.RealType.Size())
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
 		n, _ := constant.Int64Val(srcv.Value)
-		return v.writeUint(uint64(n), v.RealType.Size())
+		return dstv.writeUint(uint64(n), dstv.RealType.Size())
 	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
 		n, _ := constant.Uint64Val(srcv.Value)
-		return v.writeUint(n, v.RealType.Size())
+		return dstv.writeUint(n, dstv.RealType.Size())
 	case reflect.Bool:
-		return v.writeBool(constant.BoolVal(srcv.Value))
+		return dstv.writeBool(constant.BoolVal(srcv.Value))
 	case reflect.Complex64, reflect.Complex128:
 		real, _ := constant.Float64Val(constant.Real(srcv.Value))
 		imag, _ := constant.Float64Val(constant.Imag(srcv.Value))
-		return v.writeComplex(real, imag, v.RealType.Size())
+		return dstv.writeComplex(real, imag, dstv.RealType.Size())
 	}
 
 	// nilling nillable variables
 	if srcv == nilVariable {
-		return v.writeZero()
+		return dstv.writeZero()
 	}
 
-	// set a string to ""
+	if srcv.Kind == reflect.String {
-	if srcv.Kind == reflect.String && srcv.Len == 0 {
+		if err := scope.allocString(srcv); err != nil {
-		return v.writeZero()
+			return err
+		}
+		return dstv.writeString(uint64(srcv.Len), uint64(srcv.Base))
 	}
 
 	// slice assignment (this is not handled by the writeCopy below so that
 	// results of a reslice operation can be used here).
 	if srcv.Kind == reflect.Slice {
-		return v.writeSlice(srcv.Len, srcv.Cap, srcv.Base)
+		return dstv.writeSlice(srcv.Len, srcv.Cap, srcv.Base)
 	}
 
 	// allow any integer to be converted to any pointer
-	if t, isptr := v.RealType.(*godwarf.PtrType); isptr {
+	if t, isptr := dstv.RealType.(*godwarf.PtrType); isptr {
-		return v.writeUint(uint64(srcv.Children[0].Addr), int64(t.ByteSize))
+		return dstv.writeUint(uint64(srcv.Children[0].Addr), int64(t.ByteSize))
 	}
 
 	// byte-by-byte copying for everything else, but the source must be addressable
 	if srcv.Addr != 0 {
-		return v.writeCopy(srcv)
+		return dstv.writeCopy(srcv)
 	}
 
-	return fmt.Errorf("can not set variables of type %s (not implemented)", v.Kind.String())
+	return fmt.Errorf("can not set variables of type %s (not implemented)", dstv.Kind.String())
 }
 
 // convertToEface converts srcv into an "interface {}" and writes it to
@@ -1689,6 +1691,12 @@ func (v *Variable) writeSlice(len, cap int64, base uintptr) error {
 	return nil
 }
 
+func (v *Variable) writeString(len, base uint64) error {
+	writePointer(v.bi, v.mem, uint64(v.Addr), base)
+	writePointer(v.bi, v.mem, uint64(v.Addr)+uint64(v.bi.Arch.PtrSize()), len)
+	return nil
+}
+
 func (v *Variable) writeCopy(srcv *Variable) error {
 	buf := make([]byte, srcv.RealType.Size())
 	_, err := srcv.mem.ReadMemory(buf, srcv.Addr)
@@ -2246,7 +2254,7 @@ func (scope *EvalScope) Locals() ([]*Variable, error) {
 
 	var vars []*Variable
 	var depths []int
-	varReader := reader.Variables(scope.image().dwarf, scope.Fn.offset, reader.ToRelAddr(scope.PC, scope.image().StaticBase), scope.Line, true)
+	varReader := reader.Variables(scope.image().dwarf, scope.Fn.offset, reader.ToRelAddr(scope.PC, scope.image().StaticBase), scope.Line, true, false)
 	hasScopes := false
 	for varReader.Next() {
 		entry := varReader.Entry()

service/test/variables_test.go

@@ -1103,10 +1103,10 @@ func TestCallFunction(t *testing.T) {
 		{"call1(one+two, 4)", []string{":int:7"}, nil},
 		{"callpanic()", []string{`~panic:interface {}:interface {}(string) "callpanic panicked"`}, nil},
 		{`stringsJoin(nil, "")`, []string{`:string:""`}, nil},
-		{`stringsJoin(stringslice, ",")`, nil, errors.New("can not set variables of type string (not implemented)")},
 		{`stringsJoin(stringslice, comma)`, []string{`:string:"one,two,three"`}, nil},
 		{`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")},
+		{`noreturncall(2)`, nil, nil},
 
 		// Expression tests
 		{`square(2) + 1`, []string{":int:5"}, nil},
@@ -1143,18 +1143,27 @@ func TestCallFunction(t *testing.T) {
 
 		{"ga.PRcvr(2)", []string{`:string:"2 - 0 = 2"`}, nil},
 
-		// Nested function calls
+		// Nested function calls tests
 
 		{`onetwothree(intcallpanic(2))`, []string{`:[]int:[]int len: 3, cap: 3, [3,4,5]`}, nil},
 		{`onetwothree(intcallpanic(0))`, []string{`~panic:interface {}:interface {}(string) "panic requested"`}, nil},
 		{`onetwothree(intcallpanic(2)+1)`, []string{`:[]int:[]int len: 3, cap: 3, [4,5,6]`}, nil},
 		{`onetwothree(intcallpanic("not a number"))`, nil, errors.New("can not convert \"not a number\" constant to int")},
 
+		// Variable setting tests
+		{`pa2 = getAStructPtr(8); pa2`, []string{`pa2:*main.astruct:*main.astruct {X: 8}`}, 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")},
 	}
 
+	var testcases112 = []testCaseCallFunction{
+		// string allocation requires trusted argument order, which we don't have in Go 1.11
+		{`stringsJoin(stringslice, ",")`, []string{`:string:"one,two,three"`}, nil},
+		{`str = "a new string"; str`, []string{`str:string:"a new string"`}, nil},
+	}
+
 	var testcases113 = []testCaseCallFunction{
 		{`curriedAdd(2)(3)`, []string{`:int:5`}, nil},
 
@@ -1181,6 +1190,12 @@ func TestCallFunction(t *testing.T) {
 			testCallFunction(t, p, tc)
 		}
 
+		if goversion.VersionAfterOrEqual(runtime.Version(), 1, 12) {
+			for _, tc := range testcases112 {
+				testCallFunction(t, p, tc)
+			}
+		}
+
 		if goversion.VersionAfterOrEqual(runtime.Version(), 1, 13) {
 			for _, tc := range testcases113 {
 				testCallFunction(t, p, tc)
@@ -1195,14 +1210,22 @@ func TestCallFunction(t *testing.T) {
 func testCallFunction(t *testing.T, p proc.Process, tc testCaseCallFunction) {
 	const unsafePrefix = "-unsafe "
 
-	expr := tc.expr
+	var callExpr, varExpr string
+
+	if semicolon := strings.Index(tc.expr, ";"); semicolon >= 0 {
+		callExpr = tc.expr[:semicolon]
+		varExpr = tc.expr[semicolon+1:]
+	} else {
+		callExpr = tc.expr
+	}
+
 	checkEscape := true
-	if strings.HasPrefix(expr, unsafePrefix) {
+	if strings.HasPrefix(callExpr, unsafePrefix) {
-		expr = expr[len(unsafePrefix):]
+		callExpr = callExpr[len(unsafePrefix):]
 		checkEscape = false
 	}
 	t.Logf("call %q", tc.expr)
-	err := proc.EvalExpressionWithCalls(p, expr, pnormalLoadConfig, checkEscape)
+	err := proc.EvalExpressionWithCalls(p, callExpr, pnormalLoadConfig, checkEscape)
 	if tc.err != nil {
 		t.Logf("\terr = %v\n", err)
 		if err == nil {
@@ -1225,6 +1248,14 @@ func testCallFunction(t *testing.T, p proc.Process, tc testCaseCallFunction) {
 		retvals[i] = api.ConvertVar(retvalsVar[i])
 	}
 
+	if varExpr != "" {
+		scope, err := proc.GoroutineScope(p.CurrentThread())
+		assertNoError(err, t, "GoroutineScope")
+		v, err := scope.EvalExpression(varExpr, pnormalLoadConfig)
+		assertNoError(err, t, fmt.Sprintf("EvalExpression(%s)", varExpr))
+		retvals = append(retvals, api.ConvertVar(v))
+	}
+
 	for i := range retvals {
 		t.Logf("\t%s = %s", retvals[i].Name, retvals[i].SinglelineString())
 	}