diff --git a/hash.go b/hash.go index db5a86b..e6312c0 100644 --- a/hash.go +++ b/hash.go @@ -18,8 +18,8 @@ import ( const buildIDSeparator = "/" -// actionID returns the action ID half of a build ID, the first element. -func actionID(buildID string) string { +// splitActionID returns the action ID half of a build ID, the first element. +func splitActionID(buildID string) string { i := strings.Index(buildID, buildIDSeparator) if i < 0 { return buildID @@ -27,12 +27,12 @@ func actionID(buildID string) string { return buildID[:i] } -// contentID returns the content ID half of a build ID, the last element. -func contentID(buildID string) string { +// splitContentID returns the content ID half of a build ID, the last element. +func splitContentID(buildID string) string { return buildID[strings.LastIndex(buildID, buildIDSeparator)+1:] } -// decodeHash isthe opposite of hashToString, but with a panic for error +// decodeHash is the opposite of hashToString, but with a panic for error // handling since it should never happen. func decodeHash(str string) []byte { h, err := base64.RawURLEncoding.DecodeString(str) @@ -59,7 +59,7 @@ func alterToolVersion(tool string, args []string) error { var toolID []byte if f[2] == "devel" { // On the development branch, use the content ID part of the build ID. - toolID = decodeHash(contentID(f[len(f)-1])) + toolID = decodeHash(splitContentID(f[len(f)-1])) } else { // For a release, the output is like: "compile version go1.9.1 X:framepointer". // Use the whole line. @@ -96,7 +96,7 @@ func ownContentID(toolID []byte) (string, error) { if err != nil { return "", err } - ownID := decodeHash(contentID(buildID)) + ownID := decodeHash(splitContentID(buildID)) // Join the two content IDs together into a single base64-encoded sha256 // sum. This includes the original tool's content ID, and garble's own diff --git a/main.go b/main.go index 9df81d7..79de186 100644 --- a/main.go +++ b/main.go @@ -253,33 +253,33 @@ func mainErr(args []string) error { // Note that we also need to pass build flags to 'go list', such // as -tags. - listFlags := filterBuildFlags(flags) + cache.BuildFlags = filterBuildFlags(flags) if cmd == "test" { - listFlags = append(listFlags, "-test") + cache.BuildFlags = append(cache.BuildFlags, "-test") } if err := setGoPrivate(); err != nil { return err } - if err := setListedPackages(listFlags, args); err != nil { + if err := setListedPackages(args); err != nil { return err } - - sharedName, err := saveShared() + cache.ExecPath, err = os.Executable() if err != nil { return err } - defer os.Remove(sharedName) - execPath, err := os.Executable() + sharedName, err := saveShared() if err != nil { return err } + defer os.Remove(sharedName) + goArgs := []string{ cmd, "-trimpath", - "-toolexec=" + execPath, + "-toolexec=" + cache.ExecPath, } if cmd == "test" { // vet is generally not useful on garbled code; keep it @@ -694,7 +694,7 @@ func fillBuildInfo(flags []string) error { case "", "true": return fmt.Errorf("could not find -buildid argument") } - curActionID = decodeHash(actionID(buildID)) + curActionID = decodeHash(splitActionID(buildID)) curImportCfg = flagValue(flags, "-importcfg") if curImportCfg == "" { return fmt.Errorf("could not find -importcfg argument") @@ -742,7 +742,7 @@ func fillBuildInfo(flags []string) error { } impPkg := importedPkg{ packagefile: objectPath, - actionID: decodeHash(actionID(buildID)), + actionID: decodeHash(splitActionID(buildID)), } buildInfo.imports[importPath] = impPkg @@ -992,7 +992,66 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { default: return true // we only want to rename the above } + + // Handle the case where the name is defined in an indirectly + // imported package. Since only direct imports show up in our + // importcfg, buildInfo.imports will not initially contain the + // package path we want. + // + // This edge case can happen, for example, if package A imports + // package B and calls its API, and B's API returns C's struct. + // Suddenly, A can use struct field names defined in C, even + // though A never directly imports C. + // + // For this rare case, for now, do an extra "go list -toolexec" + // call to retrieve its export path. + // TODO: Think about ways to avoid this extra exec call. Perhaps + // add an extra archive header to record all direct and indirect + // importcfg data, like we do with private name maps. + if _, e := buildInfo.imports[path]; !e && path != curPkgPath { + goArgs := []string{ + "list", + "-json", + "-export", + "-trimpath", + "-toolexec=" + cache.ExecPath, + } + goArgs = append(goArgs, cache.BuildFlags...) + goArgs = append(goArgs, path) + + cmd := exec.Command("go", goArgs...) + cmd.Dir = opts.GarbleDir + out, err := cmd.Output() + if err != nil { + if err := err.(*exec.ExitError); err != nil { + panic(fmt.Sprintf("%v: %s", err, err.Stderr)) + } + panic(err) + } + var pkg listedPackage + if err := json.Unmarshal(out, &pkg); err != nil { + panic(err) // shouldn't happen + } + buildID, err := buildidOf(pkg.Export) + if err != nil { + panic(err) // shouldn't happen + } + // Adding it to buildInfo.imports allows us to reuse the + // "if" branch below. Plus, if this edge case triggers + // multiple times in a single package compile, we can + // call "go list" once and cache its result. + buildInfo.imports[path] = importedPkg{ + packagefile: pkg.Export, + actionID: decodeHash(splitActionID(buildID)), + } + // log.Printf("fetched indirect dependency %q from: %s", path, pkg.Export) + } + actionID := curActionID + // TODO: Make this check less prone to bugs, like the one we had + // with indirect dependencies. If "path" is not our current + // package, then it must exist in buildInfo.imports. Otherwise + // we should panic. if id := buildInfo.imports[path].actionID; len(id) > 0 { garbledPkg, err := garbledImport(path) if err != nil { @@ -1007,16 +1066,21 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { actionID = id } + origName := node.Name + _ = origName // used for debug prints below + // The exported names cannot be shortened as counter synchronization // between packages is not currently implemented if token.IsExported(node.Name) { node.Name = hashWith(actionID, node.Name) + // log.Printf("%q hashed with %x to %q", origName, actionID, node.Name) return true } fullName := tf.pkg.Path() + "." + node.Name if name, ok := tf.privateNameMap[fullName]; ok { node.Name = name + // log.Printf("%q retrieved private name %q for %q", origName, name, path) return true } @@ -1029,10 +1093,9 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { } } - // orig := node.Name tf.privateNameMap[fullName] = name node.Name = name - // log.Printf("%q hashed with %q to %q", orig, actionID, node.Name) + // log.Printf("%q assigned private name %q for %q", origName, name, path) return true } return astutil.Apply(file, pre, nil).(*ast.File) diff --git a/shared.go b/shared.go index 6e50550..d8d4b97 100644 --- a/shared.go +++ b/shared.go @@ -16,8 +16,11 @@ import ( // shared this data is shared between the different garble processes type shared struct { - Options options - ListedPackages listedPackages + ExecPath string // absolute path to the garble binary being used + BuildFlags []string // build flags fed to the original "garble ..." command + + Options options // garble options being used, i.e. our own flags + ListedPackages listedPackages // non-garbled view of all packages to build } var cache *shared @@ -146,9 +149,9 @@ type listedPackage struct { // setListedPackages gets information about the current package // and all of its dependencies -func setListedPackages(flags, patterns []string) error { +func setListedPackages(patterns []string) error { args := []string{"list", "-json", "-deps", "-export"} - args = append(args, flags...) + args = append(args, cache.BuildFlags...) args = append(args, patterns...) cmd := exec.Command("go", args...) diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index f8bdd97..fe554d2 100644 --- a/testdata/scripts/imports.txt +++ b/testdata/scripts/imports.txt @@ -60,8 +60,8 @@ package main import ( "fmt" - "strings" "reflect" + "strings" "test/main/imported" @@ -75,8 +75,9 @@ func main() { fmt.Println(imported.ImportedType(3)) fmt.Println(imported.ReflectInDefinedVar.ExportedField2) fmt.Println(imported.ReflectInDefined{ExportedField2: 5}) - fmt.Println(imported.NormalStruct{SharedName: 3}) - + normal := imported.NormalStruct{SharedName: 3} + normal.IndirectStruct.Field = 23 + fmt.Println(normal) printfWithoutPackage("%T\n", imported.ReflectTypeOf(2)) printfWithoutPackage("%T\n", imported.ReflectTypeOfIndirect(4)) @@ -112,7 +113,10 @@ func init() { fmt.Println("buildtag init func") } -- imported/imported.go -- package imported -import "reflect" +import ( + "reflect" + "test/main/imported/indirect" +) var ImportedVar = "imported var value" @@ -131,19 +135,19 @@ type ReflectTypeOfIndirect int var _ = reflect.TypeOf(new([]*ReflectTypeOfIndirect)) type ReflectValueOf struct { - ExportedField string + ExportedField string unexportedField string } -func (r *ReflectValueOf) ExportedMethodName() string { return "method: "+r.ExportedField } +func (r *ReflectValueOf) ExportedMethodName() string { return "method: " + r.ExportedField } var ReflectValueOfVar = ReflectValueOf{ExportedField: "abc"} var _ = reflect.TypeOf(ReflectValueOfVar) type ReflectInDefined struct { - ExportedField2 int + ExportedField2 int unexportedField2 int } @@ -155,13 +159,21 @@ var _ = reflect.TypeOf(ReflectInDefinedVar) const SharedName = 2 type NormalStruct struct { - SharedName int + SharedName int + IndirectStruct indirect.Indirect normalUnexportedField int } // ImportedType comes after the calls to reflect, to ensure no false positives. type ImportedType int +-- imported/indirect/indirect.go -- +package indirect + +type Indirect struct { + Field int +} + -- main.stdout -- buildtag init func imported var value @@ -170,7 +182,7 @@ x 3 9000 {5 0} -{3 0} +{3 {23} 0} ReflectTypeOf ReflectTypeOfIndirect ReflectValueOf{ExportedField:"abc", unexportedField:""}