From 79ad269bbbb55b84e4268db7e9f1c04580bef3d0 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Mon, 17 Jun 2019 18:51:29 +0200 Subject: [PATCH] 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 | 8 +++- pkg/dwarf/reader/variables.go | 31 +++++++----- pkg/proc/eval.go | 16 +++++++ pkg/proc/fncall.go | 88 +++++++++++++++++++++++++++++----- pkg/proc/variables.go | 48 +++++++++++-------- service/test/variables_test.go | 43 ++++++++++++++--- 6 files changed, 185 insertions(+), 49 deletions(-) diff --git a/_fixtures/fncall.go b/_fixtures/fncall.go index c3227593..2de65e87 100644 --- a/_fixtures/fncall.go +++ b/_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) } diff --git a/pkg/dwarf/reader/variables.go b/pkg/dwarf/reader/variables.go index 627fd53a..d2c64a44 100644 --- a/pkg/dwarf/reader/variables.go +++ b/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 - reader *dwarf.Reader - entry *dwarf.Entry - depth int - onlyVisible bool - pc uint64 - line int - err error + dwarf *dwarf.Data + reader *dwarf.Reader + entry *dwarf.Entry + depth int + pc uint64 + line int + 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) diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index 9d815178..f0fd0489 100644 --- a/pkg/proc/eval.go +++ b/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) { diff --git a/pkg/proc/fncall.go b/pkg/proc/fncall.go index 30f5088d..c5f901ec 100644 --- a/pkg/proc/fncall.go +++ b/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) - 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) - } + var off int64 + if trustArgOrder && fn.Name == "runtime.mallocgc" { + // runtime is always optimized and optimized code sometimes doesn't have + // location info for arguments, but we still want to call runtime.mallocgc. + off = argFrameSize + } else { + 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 +} diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index 1b4e1bab..00549960 100644 --- a/pkg/proc/variables.go +++ b/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 && srcv.Len == 0 { - return v.writeZero() + if srcv.Kind == reflect.String { + if err := scope.allocString(srcv); err != nil { + 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 { - return v.writeUint(uint64(srcv.Children[0].Addr), int64(t.ByteSize)) + if t, isptr := dstv.RealType.(*godwarf.PtrType); isptr { + 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() diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 0fd03c91..7c38c6e8 100644 --- a/service/test/variables_test.go +++ b/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) { - expr = expr[len(unsafePrefix):] + if strings.HasPrefix(callExpr, 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()) } -- GitLab