diff --git a/Documentation/cli/expr.md b/Documentation/cli/expr.md index 0b0105b5fa41cb311247da04e4e2aea32b3a5144..fcf027f03342ff80e0c472797b7361194fed2a39 100644 --- a/Documentation/cli/expr.md +++ b/Documentation/cli/expr.md @@ -106,3 +106,7 @@ Packages with the same name can be disambiguated by using the full package path. (dlv) p "some/package".A (dlv) p "some/other/package".A ``` + +# Pointers in Cgo + +Char pointers are always treated as NUL terminated strings, both indexing and the slice operator can be applied to them. Other C pointers can also be used similarly to Go slices, with indexing and the slice operator. In both of these cases it is up to the user to respect array bounds. diff --git a/_fixtures/testvariablescgo/test.c b/_fixtures/testvariablescgo/test.c new file mode 100644 index 0000000000000000000000000000000000000000..2299e142571fbada22be6be79e06868e4759ba13 --- /dev/null +++ b/_fixtures/testvariablescgo/test.c @@ -0,0 +1,38 @@ +#include +#include +#include + +#ifdef __amd64__ +#define BREAKPOINT asm("int3;") +#elif __i386__ +#define BREAKPOINT asm("int3;") +#elif __aarch64__ +#define BREAKPOINT asm("brk 0;") +#endif + +#define N 100 + +struct align_check { + int a; + char b; +}; + +void testfn(void) { + const char *s0 = "a string"; + const char *longstring = "averylongstring0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g0123456789h0123456789"; + int *v = malloc(sizeof(int) * N); + struct align_check *v_align_check = malloc(sizeof(struct align_check) * N); + + for (int i = 0; i < N; i++) { + v[i] = i; + v_align_check[i].a = i; + v_align_check[i].b = i; + } + + char *s = malloc(strlen(s0) + 1); + strcpy(s, s0); + + BREAKPOINT; + + printf("%s %s %p %p\n", s, longstring, v, v_align_check); +} diff --git a/_fixtures/testvariablescgo/testvariablescgo.go b/_fixtures/testvariablescgo/testvariablescgo.go new file mode 100644 index 0000000000000000000000000000000000000000..be4e199b12d08d35896d50e1425656686227871a --- /dev/null +++ b/_fixtures/testvariablescgo/testvariablescgo.go @@ -0,0 +1,11 @@ +package main + +// #cgo CFLAGS: -g -Wall -O0 -std=gnu99 +/* +extern void testfn(void); +*/ +import "C" + +func main() { + C.testfn() +} diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index d541bb87156cd2a38fe1246e56f1b3a2e95d9da9..f43ee5b2836a582a34ee6b6b113e2eee64162c16 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -1214,7 +1214,9 @@ func (scope *EvalScope) evalIndex(node *ast.IndexExpr) (*Variable, error) { return nil, xev.Unreadable } - xev = xev.maybeDereference() + if xev.Flags&VariableCPtr == 0 { + xev = xev.maybeDereference() + } idxev, err := scope.evalAST(node.Index) if err != nil { @@ -1228,11 +1230,13 @@ func (scope *EvalScope) evalIndex(node *ast.IndexExpr) (*Variable, error) { if xev == nilVariable { return nil, cantindex } - _, isarrptr := xev.RealType.(*godwarf.PtrType).Type.(*godwarf.ArrayType) - if !isarrptr { - return nil, cantindex + if xev.Flags&VariableCPtr == 0 { + _, isarrptr := xev.RealType.(*godwarf.PtrType).Type.(*godwarf.ArrayType) + if !isarrptr { + return nil, cantindex + } + xev = xev.maybeDereference() } - xev = xev.maybeDereference() fallthrough case reflect.Slice, reflect.Array, reflect.String: @@ -1309,6 +1313,11 @@ func (scope *EvalScope) evalReslice(node *ast.SliceExpr) (*Variable, error) { return nil, fmt.Errorf("map index out of bounds") } return xev, nil + case reflect.Ptr: + if xev.Flags&VariableCPtr != 0 { + return xev.reslice(low, high) + } + fallthrough default: return nil, fmt.Errorf("can not slice \"%s\" (type %s)", exprToString(node.X), xev.TypeString()) } @@ -1844,7 +1853,13 @@ func sameType(t1, t2 godwarf.Type) bool { } func (v *Variable) sliceAccess(idx int) (*Variable, error) { - if idx < 0 || int64(idx) >= v.Len { + wrong := false + if v.Flags&VariableCPtr == 0 { + wrong = idx < 0 || int64(idx) >= v.Len + } else { + wrong = idx < 0 + } + if wrong { return nil, fmt.Errorf("index out of bounds") } mem := v.mem @@ -1889,7 +1904,18 @@ func (v *Variable) mapAccess(idx *Variable) (*Variable, error) { } func (v *Variable) reslice(low int64, high int64) (*Variable, error) { - if low < 0 || low >= v.Len || high < 0 || high > v.Len { + wrong := false + cptrNeedsFakeSlice := false + if v.Flags&VariableCPtr == 0 { + wrong = low < 0 || low >= v.Len || high < 0 || high > v.Len + } else { + wrong = low < 0 || high < 0 + if high == 0 { + high = low + } + cptrNeedsFakeSlice = v.Kind != reflect.String + } + if wrong { return nil, fmt.Errorf("index out of bounds") } @@ -1901,7 +1927,7 @@ func (v *Variable) reslice(low int64, high int64) (*Variable, error) { } typ := v.DwarfType - if _, isarr := v.DwarfType.(*godwarf.ArrayType); isarr { + if _, isarr := v.DwarfType.(*godwarf.ArrayType); isarr || cptrNeedsFakeSlice { typ = fakeSliceType(v.fieldType) } diff --git a/pkg/proc/proc_general_test.go b/pkg/proc/proc_general_test.go index 2e35469d8ef7174e941e8bc1e359205017e5c61f..9ff216085bda2067011009b0efd708a4081ceabf 100644 --- a/pkg/proc/proc_general_test.go +++ b/pkg/proc/proc_general_test.go @@ -24,3 +24,73 @@ func TestIssue554(t *testing.T) { t.Fatalf("should be false") } } + +type dummyMem struct { + t *testing.T + mem []byte + base uint64 + reads []memRead +} + +type memRead struct { + addr uint64 + size int +} + +func (dm *dummyMem) ReadMemory(buf []byte, addr uintptr) (int, error) { + dm.t.Logf("read addr=%#x size=%#x\n", addr, len(buf)) + dm.reads = append(dm.reads, memRead{uint64(addr), len(buf)}) + a := int64(addr) - int64(dm.base) + if a < 0 { + panic("reading below base") + } + if int(a)+len(buf) > len(dm.mem) { + panic("reading beyond end of mem") + } + copy(buf, dm.mem[a:]) + return len(buf), nil +} + +func (dm *dummyMem) WriteMemory(uintptr, []byte) (int, error) { + panic("not supported") +} + +func TestReadCStringValue(t *testing.T) { + const tgt = "a test string" + const maxstrlen = 64 + + dm := &dummyMem{t: t} + dm.mem = make([]byte, maxstrlen) + copy(dm.mem, tgt) + + for _, tc := range []struct { + base uint64 + numreads int + }{ + {0x5000, 1}, + {0x5001, 1}, + {0x4fff, 2}, + {uint64(0x5000 - len(tgt) - 1), 1}, + {uint64(0x5000-len(tgt)-1) + 1, 2}, + } { + t.Logf("base is %#x\n", tc.base) + dm.base = tc.base + dm.reads = dm.reads[:0] + out, done, err := readCStringValue(dm, uintptr(tc.base), LoadConfig{MaxStringLen: maxstrlen}) + if err != nil { + t.Errorf("base=%#x readCStringValue: %v", tc.base, err) + } + if !done { + t.Errorf("base=%#x expected done but wasn't", tc.base) + } + if out != tgt { + t.Errorf("base=%#x got %q expected %q", tc.base, out, tgt) + } + if len(dm.reads) != tc.numreads { + t.Errorf("base=%#x wrong number of reads %d (expected %d)", tc.base, len(dm.reads), tc.numreads) + } + if tc.base == 0x4fff && dm.reads[0].size != 1 { + t.Errorf("base=%#x first read in not of one byte", tc.base) + } + } +} diff --git a/pkg/proc/variables.go b/pkg/proc/variables.go index 51a695bf202440e48f7ae20f2514a751dbb9ea41..97b8157fcc40832fcbf4a84251e9e31f24165e81 100644 --- a/pkg/proc/variables.go +++ b/pkg/proc/variables.go @@ -75,6 +75,8 @@ const ( // the variable is the return value of a function call and allocated on a // frame that no longer exists) VariableFakeAddress + // VariableCPrt means the variable is a C pointer + VariableCPtr ) // Variable represents a variable. It contains the address, name, @@ -612,6 +614,18 @@ func newVariable(name string, addr uintptr, dwarfType godwarf.Type, bi *BinaryIn v.Kind = reflect.Ptr if _, isvoid := t.Type.(*godwarf.VoidType); isvoid { v.Kind = reflect.UnsafePointer + } else if isCgoType(bi, t) { + v.Flags |= VariableCPtr + v.fieldType = t.Type + v.stride = alignAddr(v.fieldType.Size(), v.fieldType.Align()) + v.Len = 0 + if isCgoCharPtr(bi, t) { + v.Kind = reflect.String + } + if v.Addr != 0 { + n, err := readUintRaw(v.mem, v.Addr, int64(v.bi.Arch.PtrSize())) + v.Base, v.Unreadable = uintptr(n), err + } } case *godwarf.ChanType: v.Kind = reflect.Chan @@ -656,6 +670,14 @@ func newVariable(name string, addr uintptr, dwarfType godwarf.Type, bi *BinaryIn } case *godwarf.IntType: v.Kind = reflect.Int + case *godwarf.CharType: + // Rest of the code assumes that Kind == reflect.Int implies RealType == + // godwarf.IntType. + v.RealType = &godwarf.IntType{BasicType: t.BasicType} + v.Kind = reflect.Int + case *godwarf.UcharType: + v.RealType = &godwarf.IntType{BasicType: t.BasicType} + v.Kind = reflect.Int case *godwarf.UintType: v.Kind = reflect.Uint case *godwarf.FloatType: @@ -1204,7 +1226,18 @@ func (v *Variable) loadValueInternal(recurseLevel int, cfg LoadConfig) { case reflect.String: var val string - val, v.Unreadable = readStringValue(DereferenceMemory(v.mem), v.Base, v.Len, cfg) + if v.Flags&VariableCPtr == 0 { + val, v.Unreadable = readStringValue(DereferenceMemory(v.mem), v.Base, v.Len, cfg) + } else { + var done bool + val, done, v.Unreadable = readCStringValue(DereferenceMemory(v.mem), v.Base, cfg) + if v.Unreadable == nil { + v.Len = int64(len(val)) + if !done { + v.Len++ + } + } + } v.Value = constant.MakeString(val) case reflect.Slice, reflect.Array: @@ -1341,9 +1374,50 @@ func readStringValue(mem MemoryReadWriter, addr uintptr, strlen int64, cfg LoadC return "", fmt.Errorf("could not read string at %#v due to %s", addr, err) } - retstr := *(*string)(unsafe.Pointer(&val)) + return string(val), nil +} + +func readCStringValue(mem MemoryReadWriter, addr uintptr, cfg LoadConfig) (string, bool, error) { + buf := make([]byte, cfg.MaxStringLen) // + val := buf[:0] // part of the string we've already read + + for len(buf) > 0 { + // Reads some memory for the string but (a) never more than we would + // need (considering cfg.MaxStringLen), and (b) never cross a page boundary + // until we're sure we have to. + // The page check is needed to avoid getting an I/O error for reading + // memory we don't even need. + // We don't know how big a page is but 1024 is a reasonable minimum common + // divisor for all architectures. + curaddr := addr + uintptr(len(val)) + maxsize := int(alignAddr(int64(curaddr+1), 1024) - int64(curaddr)) + size := len(buf) + if size > maxsize { + size = maxsize + } + + _, err := mem.ReadMemory(buf[:size], curaddr) + if err != nil { + return "", false, fmt.Errorf("could not read string at %#v due to %s", addr, err) + } + + done := false + for i := 0; i < size; i++ { + if buf[i] == 0 { + done = true + size = i + break + } + } + + val = val[:len(val)+size] + buf = buf[size:] + if done { + return string(val), true, nil + } + } - return retstr, nil + return string(val), false, nil } const ( @@ -2157,6 +2231,37 @@ func popcnt(x uint64) int { return int(x) & (1<<7 - 1) } +func isCgoType(bi *BinaryInfo, typ godwarf.Type) bool { + cu := bi.Images[typ.Common().Index].findCompileUnitForOffset(typ.Common().Offset) + if cu == nil { + return false + } + return !cu.isgo +} + +func isCgoCharPtr(bi *BinaryInfo, typ *godwarf.PtrType) bool { + if !isCgoType(bi, typ) { + return false + } + + fieldtyp := typ.Type +resolveQualTypedef: + for { + switch t := fieldtyp.(type) { + case *godwarf.QualType: + fieldtyp = t.Type + case *godwarf.TypedefType: + fieldtyp = t.Type + default: + break resolveQualTypedef + } + } + + _, ischar := fieldtyp.(*godwarf.CharType) + _, isuchar := fieldtyp.(*godwarf.UcharType) + return ischar || isuchar +} + func (cm constantsMap) Get(typ godwarf.Type) *constantType { ctyp := cm[dwarfRef{typ.Common().Index, typ.Common().Offset}] if ctyp == nil { diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 61b544b64f8e679a1fd867cc23826224b8ce9d8c..657c81dc1d2bbb5711f726552c4df13a63a853cc 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -1519,3 +1519,46 @@ func TestPluginVariables(t *testing.T) { assertVariable(t, vb, varTest{"b", true, `github.com/go-delve/delve/_fixtures/internal/pluginsupport.SomethingElse(*github.com/go-delve/delve/_fixtures/plugin2.asomethingelse) *{x: 1, y: 4}`, ``, `github.com/go-delve/delve/_fixtures/internal/pluginsupport.SomethingElse`, nil}) }) } + +func TestCgoEval(t *testing.T) { + testcases := []varTest{ + {"s", true, `"a string"`, `"a string"`, "*char", nil}, + {"longstring", true, `"averylongstring0123456789a0123456789b0123456789c0123456789d01234...+1 more"`, `"averylongstring0123456789a0123456789b0123456789c0123456789d01234...+1 more"`, "*const char", nil}, + {"longstring[64:]", false, `"56789e0123456789f0123456789g0123456789h0123456789"`, `"56789e0123456789f0123456789g0123456789h0123456789"`, "*const char", nil}, + {"s[3]", false, "116", "116", "char", nil}, + {"v", true, "*0", "(*int)(…", "*int", nil}, + {"v[1]", false, "1", "1", "int", nil}, + {"v[90]", false, "90", "90", "int", nil}, + {"v[:5]", false, "[]int len: 5, cap: 5, [0,1,2,3,4]", "[]int len: 5, cap: 5, [...]", "[]int", nil}, + {"v_align_check", true, "*align_check {a: 0, b: 0}", "(*struct align_check)(…", "*struct align_check", nil}, + {"v_align_check[1]", false, "align_check {a: 1, b: 1}", "align_check {a: 1, b: 1}", "align_check", nil}, + {"v_align_check[90]", false, "align_check {a: 90, b: 90}", "align_check {a: 90, b: 90}", "align_check", nil}, + } + + protest.AllowRecording(t) + withTestProcess("testvariablescgo/", t, func(p *proc.Target, fixture protest.Fixture) { + assertNoError(p.Continue(), t, "Continue() returned an error") + for _, tc := range testcases { + variable, err := evalVariable(p, tc.name, pnormalLoadConfig) + if err != nil && err.Error() == "evaluating methods not supported on this version of Go" { + // this type of eval is unsupported with the current version of Go. + continue + } + if tc.err == nil { + assertNoError(err, t, fmt.Sprintf("EvalExpression(%s) returned an error", tc.name)) + assertVariable(t, variable, tc) + variable, err := evalVariable(p, tc.name, pshortLoadConfig) + assertNoError(err, t, fmt.Sprintf("EvalExpression(%s, pshortLoadConfig) returned an error", tc.name)) + assertVariable(t, variable, tc.alternateVarTest()) + } else { + if err == nil { + t.Fatalf("Expected error %s, got no error (%s)", tc.err.Error(), tc.name) + } + if tc.err.Error() != err.Error() { + t.Fatalf("Unexpected error. Expected %s got %s", tc.err.Error(), err.Error()) + } + } + + } + }) +}