diff --git a/_fixtures/internal/dir.io/dir.go b/_fixtures/internal/dir.io/dir.go index 3ce3c9e8..16852a5b 100644 --- a/_fixtures/internal/dir.io/dir.go +++ b/_fixtures/internal/dir.io/dir.go @@ -3,3 +3,9 @@ package dirio type SomeType struct { X int } + +var A string = "something" + +func SomeFunction() { + println("hello world") +} diff --git a/_fixtures/pkgrenames.go b/_fixtures/pkgrenames.go index ce8bf36d..2f31d305 100644 --- a/_fixtures/pkgrenames.go +++ b/_fixtures/pkgrenames.go @@ -8,6 +8,7 @@ import ( pkg1 "go/ast" pkg2 "net/http" + "github.com/go-delve/delve/_fixtures/internal/dir.io" "github.com/go-delve/delve/_fixtures/internal/dir0/pkg" "github.com/go-delve/delve/_fixtures/internal/dir0/renamedpackage" dir1pkg "github.com/go-delve/delve/_fixtures/internal/dir1/pkg" @@ -51,5 +52,5 @@ func main() { m := t.Method(0) fmt.Println(m.Type.In(0)) fmt.Println(m.Type.String()) - fmt.Println(badexpr, req, amap, amap2, dir0someType, dir1someType, amap3, anarray, achan, aslice, afunc, astruct, astruct2, iface2iface, iface3, pkg.SomeVar, pkg.A, dir1pkg.A) + fmt.Println(badexpr, req, amap, amap2, dir0someType, dir1someType, amap3, anarray, achan, aslice, afunc, astruct, astruct2, iface2iface, iface3, pkg.SomeVar, pkg.A, dir1pkg.A, dirio.A, dirio.SomeFunction) } diff --git a/pkg/dwarf/godwarf/type.go b/pkg/dwarf/godwarf/type.go index 2bb97fe4..829d2782 100644 --- a/pkg/dwarf/godwarf/type.go +++ b/pkg/dwarf/godwarf/type.go @@ -26,6 +26,7 @@ const ( AttrGoElem dwarf.Attr = 0x2902 AttrGoEmbeddedField dwarf.Attr = 0x2903 AttrGoRuntimeType dwarf.Attr = 0x2904 + AttrGoPackageName dwarf.Attr = 0x2905 ) // Basic type encodings -- the value for AttrEncoding in a TagBaseType Entry. diff --git a/pkg/proc/bininfo.go b/pkg/proc/bininfo.go index 57a69434..6ec05bef 100644 --- a/pkg/proc/bininfo.go +++ b/pkg/proc/bininfo.go @@ -61,8 +61,16 @@ type BinaryInfo struct { closer io.Closer sepDebugCloser io.Closer - // Maps package names to package paths, needed to lookup types inside DWARF info - packageMap map[string]string + // PackageMap maps package names to package paths, needed to lookup types inside DWARF info. + // On Go1.12 this mapping is determined by using the last element of a package path, for example: + // github.com/go-delve/delve + // will map to 'delve' because it ends in '/delve'. + // Starting with Go1.13 debug_info will contain a special attribute + // (godwarf.AttrGoPackageName) containing the canonical package name for + // each package. + // If multiple packages have the same name the map entry will have more + // than one item in the slice. + PackageMap map[string][]string frameEntries frame.FrameDescriptionEntries @@ -1253,7 +1261,7 @@ func (bi *BinaryInfo) registerTypeToPackageMap(entry *dwarf.Entry) { return } name := path[slash+1:] - bi.packageMap[name] = path + bi.PackageMap[name] = []string{path} } func (bi *BinaryInfo) loadDebugInfoMaps(image *Image, debugLineBytes []byte, wg *sync.WaitGroup, cont func()) { @@ -1267,8 +1275,8 @@ func (bi *BinaryInfo) loadDebugInfoMaps(image *Image, debugLineBytes []byte, wg if bi.consts == nil { bi.consts = make(map[dwarfRef]*constantType) } - if bi.packageMap == nil { - bi.packageMap = make(map[string]string) + if bi.PackageMap == nil { + bi.PackageMap = make(map[string][]string) } if bi.inlinedCallLines == nil { bi.inlinedCallLines = make(map[fileLine][]uint64) @@ -1329,6 +1337,10 @@ func (bi *BinaryInfo) loadDebugInfoMaps(image *Image, debugLineBytes []byte, wg cu.producer = cu.producer[:semicolon] } } + gopkg, _ := entry.Val(godwarf.AttrGoPackageName).(string) + if cu.isgo && gopkg != "" { + bi.PackageMap[gopkg] = append(bi.PackageMap[gopkg], escapePackagePath(strings.Replace(cu.name, "\\", "/", -1))) + } bi.compileUnits = append(bi.compileUnits, cu) if entry.Children { bi.loadDebugInfoMapsCompileUnit(ctxt, image, reader, cu) @@ -1370,6 +1382,8 @@ func (bi *BinaryInfo) loadDebugInfoMaps(image *Image, debugLineBytes []byte, wg // loadDebugInfoMapsCompileUnit loads entry from a single compile unit. func (bi *BinaryInfo) loadDebugInfoMapsCompileUnit(ctxt *loadDebugInfoMapsContext, image *Image, reader *reader.Reader, cu *compileUnit) { + hasAttrGoPkgName := goversion.ProducerAfterOrEqual(cu.producer, 1, 13) + for entry, err := reader.Next(); entry != nil; entry, err = reader.Next() { if err != nil { image.setLoadError("error reading debug_info: %v", err) @@ -1391,7 +1405,7 @@ func (bi *BinaryInfo) loadDebugInfoMapsCompileUnit(ctxt *loadDebugInfoMapsContex bi.types[name] = dwarfRef{image.index, entry.Offset} } } - if cu != nil && cu.isgo { + if cu != nil && cu.isgo && !hasAttrGoPkgName { bi.registerTypeToPackageMap(entry) } image.registerRuntimeTypeToDIE(entry, ctxt.ardr) @@ -1684,8 +1698,13 @@ func (bi *BinaryInfo) expandPackagesInType(expr ast.Expr) { case *ast.SelectorExpr: switch x := e.X.(type) { case *ast.Ident: - if path, ok := bi.packageMap[x.Name]; ok { - x.Name = path + if len(bi.PackageMap[x.Name]) > 0 { + // There's no particular reason to expect the first entry to be the + // correct one if the package name is ambiguous, but trying all possible + // expansions of all types mentioned in the expression is complicated + // and, besides type assertions, users can always specify the type they + // want exactly, using a string. + x.Name = bi.PackageMap[x.Name][0] } default: bi.expandPackagesInType(e.X) @@ -1697,6 +1716,17 @@ func (bi *BinaryInfo) expandPackagesInType(expr ast.Expr) { } } +// escapePackagePath returns pkg with '.' replaced with '%2e' (in all +// elements of the path except the first one) like Go does in variable and +// type names. +func escapePackagePath(pkg string) string { + slash := strings.Index(pkg, "/") + if slash < 0 { + slash = 0 + } + return pkg[:slash] + strings.Replace(pkg[slash:], ".", "%2e", -1) +} + // Looks up symbol (either functions or global variables) at address addr. // Used by disassembly formatter. func (bi *BinaryInfo) symLookup(addr uint64) (string, uint64) { diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index 4f42e84b..aa26acaa 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -379,7 +379,21 @@ func (scope *EvalScope) PackageVariables(cfg LoadConfig) ([]*Variable, error) { return vars, nil } -func (scope *EvalScope) findGlobal(name string) (*Variable, error) { +func (scope *EvalScope) findGlobal(pkgName, varName string) (*Variable, error) { + for _, pkgPath := range scope.BinInfo.PackageMap[pkgName] { + v, err := scope.findGlobalInternal(pkgPath + "." + varName) + if err != nil || v != nil { + return v, err + } + } + v, err := scope.findGlobalInternal(pkgName + "." + varName) + if err != nil || v != nil { + return v, err + } + return nil, fmt.Errorf("could not find symbol value for %s.%s", pkgName, varName) +} + +func (scope *EvalScope) findGlobalInternal(name string) (*Variable, error) { for _, pkgvar := range scope.BinInfo.packageVars { if pkgvar.name == name || strings.HasSuffix(pkgvar.name, "/"+name) { reader := pkgvar.cu.image.dwarfReader @@ -426,7 +440,7 @@ func (scope *EvalScope) findGlobal(name string) (*Variable, error) { } } } - return nil, fmt.Errorf("could not find symbol value for %s", name) + return nil, nil } // image returns the image containing the current function. @@ -608,7 +622,7 @@ func (scope *EvalScope) evalAST(t ast.Expr) (*Variable, error) { return scope.g.variable.clone(), nil } else if maybePkg.Name == "runtime" && node.Sel.Name == "frameoff" { return newConstant(constant.MakeInt64(scope.frameOffset), scope.Mem), nil - } else if v, err := scope.findGlobal(maybePkg.Name + "." + node.Sel.Name); err == nil { + } else if v, err := scope.findGlobal(maybePkg.Name, node.Sel.Name); err == nil { return v, nil } } @@ -616,7 +630,7 @@ func (scope *EvalScope) evalAST(t ast.Expr) (*Variable, error) { if maybePkg, ok := node.X.(*ast.BasicLit); ok && maybePkg.Kind == token.STRING { pkgpath, err := strconv.Unquote(maybePkg.Value) if err == nil { - if v, err := scope.findGlobal(pkgpath + "." + node.Sel.Name); err == nil { + if v, err := scope.findGlobal(pkgpath, node.Sel.Name); err == nil { return v, nil } } @@ -1013,7 +1027,7 @@ func (scope *EvalScope) evalIdent(node *ast.Ident) (*Variable, error) { // if it's not a local variable then it could be a package variable w/o explicit package name if scope.Fn != nil { - if v, err := scope.findGlobal(scope.Fn.PackageName() + "." + node.Name); err == nil { + if v, err := scope.findGlobal(scope.Fn.PackageName(), node.Name); err == nil { v.Name = node.Name return v, nil } diff --git a/pkg/proc/moduledata.go b/pkg/proc/moduledata.go index 8f44471f..90f6e113 100644 --- a/pkg/proc/moduledata.go +++ b/pkg/proc/moduledata.go @@ -15,7 +15,7 @@ type moduleData struct { func loadModuleData(bi *BinaryInfo, mem MemoryReadWriter) ([]moduleData, error) { scope := globalScope(bi, bi.Images[0], mem) var md *Variable - md, err := scope.findGlobal("runtime.firstmoduledata") + md, err := scope.findGlobal("runtime", "firstmoduledata") if err != nil { return nil, err } @@ -131,7 +131,7 @@ func resolveNameOff(bi *BinaryInfo, mds []moduleData, typeAddr uintptr, off uint func reflectOffsMapAccess(bi *BinaryInfo, off uintptr, mem MemoryReadWriter) (*Variable, error) { scope := globalScope(bi, bi.Images[0], mem) - reflectOffs, err := scope.findGlobal("runtime.reflectOffs") + reflectOffs, err := scope.findGlobal("runtime", "reflectOffs") if err != nil { return nil, err } diff --git a/service/debugger/locations.go b/service/debugger/locations.go index 16827fdf..16171aeb 100644 --- a/service/debugger/locations.go +++ b/service/debugger/locations.go @@ -216,7 +216,7 @@ func stripReceiverDecoration(in string) string { return in[2 : len(in)-1] } -func (spec *FuncLocationSpec) Match(sym proc.Function) bool { +func (spec *FuncLocationSpec) Match(sym proc.Function, packageMap map[string][]string) bool { if spec.BaseName != sym.BaseName() { return false } @@ -231,17 +231,26 @@ func (spec *FuncLocationSpec) Match(sym proc.Function) bool { return false } } else { - if !partialPathMatch(spec.PackageName, sym.PackageName()) { + if !packageMatch(spec.PackageName, sym.PackageName(), packageMap) { return false } } } - if spec.PackageOrReceiverName != "" && !partialPathMatch(spec.PackageOrReceiverName, sym.PackageName()) && spec.PackageOrReceiverName != recv { + if spec.PackageOrReceiverName != "" && !packageMatch(spec.PackageOrReceiverName, sym.PackageName(), packageMap) && spec.PackageOrReceiverName != recv { return false } return true } +func packageMatch(specPkg, symPkg string, packageMap map[string][]string) bool { + for _, pkg := range packageMap[specPkg] { + if partialPackageMatch(pkg, symPkg) { + return true + } + } + return partialPackageMatch(specPkg, symPkg) +} + func (loc *RegexLocationSpec) Find(d *Debugger, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool) ([]api.Location, error) { funcs := d.target.BinInfo().Functions matches, err := regexFilterFuncs(loc.FuncRegex, funcs) @@ -304,6 +313,10 @@ func partialPathMatch(expr, path string) bool { expr = strings.ToLower(filepath.ToSlash(expr)) path = strings.ToLower(filepath.ToSlash(path)) } + return partialPackageMatch(expr, path) +} + +func partialPackageMatch(expr, path string) bool { if len(expr) < len(path)-1 { return strings.HasSuffix(path, expr) && (path[len(path)-len(expr)-1] == '/') } else { @@ -347,7 +360,7 @@ func (loc *NormalLocationSpec) Find(d *Debugger, scope *proc.EvalScope, locStr s var candidateFuncs []string if loc.FuncBase != nil { for _, f := range d.target.BinInfo().Functions { - if !loc.FuncBase.Match(f) { + if !loc.FuncBase.Match(f, d.target.BinInfo().PackageMap) { continue } if loc.Base == f.Name { diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index d61839a9..b0d56cc0 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -725,6 +725,13 @@ func TestClientServer_FindLocations(t *testing.T) { } c.ClearBreakpoint(bp.ID) }) + + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 13) { + withTestClient2("pkgrenames", t, func(c service.Client) { + someFuncLoc := findLocationHelper(t, c, "github.com/go-delve/delve/_fixtures/internal/dir%2eio.SomeFunction:0", false, 1, 0)[0] + findLocationHelper(t, c, "dirio.SomeFunction:0", false, 1, someFuncLoc) + }) + } } func TestClientServer_FindLocationsAddr(t *testing.T) { diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 074269ef..787168d5 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -971,6 +971,23 @@ func TestIssue426(t *testing.T) { }) } +func testPackageRenamesHelper(t *testing.T, p proc.Process, testcases []varTest) { + for _, tc := range testcases { + variable, err := evalVariable(p, tc.name, pnormalLoadConfig) + if tc.err == nil { + assertNoError(err, t, fmt.Sprintf("EvalExpression(%s) returned an error", tc.name)) + assertVariable(t, variable, tc) + } 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()) + } + } + } +} + func TestPackageRenames(t *testing.T) { // Tests that the concrete type of an interface variable is resolved // correctly in a few edge cases, in particular: @@ -999,41 +1016,43 @@ func TestPackageRenames(t *testing.T) { {"aslice", true, `interface {}([]github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType) [{X: 3},{X: 4}]`, "", "interface {}", nil}, {"afunc", true, `interface {}(func(github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType, github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType)) main.main.func1`, "", "interface {}", nil}, {"astruct", true, `interface {}(*struct { A github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType; B github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType }) *{A: github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType {X: 1, Y: 2}, B: github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType {X: 3}}`, "", "interface {}", nil}, - {"astruct2", true, `interface {}(*struct { github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType; X int }) *{SomeType: github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType {X: 1, Y: 2}, X: 10}`, "", "interface {}", nil}, {"iface2iface", true, `interface {}(*interface { AMethod(int) int; AnotherMethod(int) int }) **github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType {X: 4}`, "", "interface {}", nil}, {`"dir0/pkg".A`, false, "0", "", "int", nil}, {`"dir1/pkg".A`, false, "1", "", "int", nil}, } - ver, _ := goversion.Parse(runtime.Version()) - if ver.Major > 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 7, -1, 0, 0, ""}) { - // Not supported on 1.6 or earlier + testcases1_8 := []varTest{ + // before 1.9 embedded struct fields have fieldname == type + {"astruct2", true, `interface {}(*struct { github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType; X int }) *{github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType: github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType {X: 1, Y: 2}, X: 10}`, "", "interface {}", nil}, + } + + testcases1_9 := []varTest{ + {"astruct2", true, `interface {}(*struct { github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType; X int }) *{SomeType: github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType {X: 1, Y: 2}, X: 10}`, "", "interface {}", nil}, + } + + testcases1_13 := []varTest{ + // needs DW_AT_go_package_name attribute added to Go1.13 + {`dirio.A`, false, `"something"`, "", "string", nil}, + } + + if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 7) { return } protest.AllowRecording(t) withTestProcess("pkgrenames", t, func(p proc.Process, fixture protest.Fixture) { assertNoError(proc.Continue(p), t, "Continue() returned an error") - for _, tc := range testcases { - if ver.Major > 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 9, -1, 0, 0, ""}) { - // before 1.9 embedded struct field have fieldname == type - if tc.name == "astruct2" { - tc.value = `interface {}(*struct { github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType; X int }) *{github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType: github.com/go-delve/delve/_fixtures/internal/dir1/pkg.SomeType {X: 1, Y: 2}, X: 10}` - } - } - variable, err := evalVariable(p, tc.name, pnormalLoadConfig) - if tc.err == nil { - assertNoError(err, t, fmt.Sprintf("EvalExpression(%s) returned an error", tc.name)) - assertVariable(t, variable, tc) - } 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()) - } - } + testPackageRenamesHelper(t, p, testcases) + + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 9) { + testPackageRenamesHelper(t, p, testcases1_9) + } else { + testPackageRenamesHelper(t, p, testcases1_8) + } + + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 13) { + testPackageRenamesHelper(t, p, testcases1_13) } }) } @@ -1049,7 +1068,7 @@ func TestConstants(t *testing.T) { {"bitZero", true, "1", "", "main.BitFieldType", nil}, {"bitOne", true, "2", "", "main.BitFieldType", nil}, {"constTwo", true, "2", "", "main.ConstType", nil}, - {"pkg.SomeConst", true, "2", "", "int", nil}, + {"pkg.SomeConst", false, "2", "", "int", nil}, } ver, _ := goversion.Parse(runtime.Version()) if ver.Major > 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}) {