avoid one more call to 'go tool buildid' (#253)

We use it to get the content ID of garble's binary, which is used for
both the garble action IDs, as well as 'go tool compile -V=full'.

Since those two happen in separate processes, both used to call 'go tool
buildid' separately. Store it in the gob cache the first time, and reuse
it the second time.

Since each call to cmd/go costs about 10ms (new process, running its
many init funcs, etc), this results in a nice speed-up for our small
benchmark. Most builds will take many seconds though, so note that a
~15ms speedup there will likely not be noticeable.

While at it, simplify the buildInfo global, as now it just contains a
map representation of the -importcfg contents. It now has better names,
docs, and a simpler representation.

We also stop using the term "garbled import", as it was a bit confusing.
"obfuscated types.Package" is a much better description.

	name     old time/op       new time/op       delta
	Build-8        106ms ± 1%         92ms ± 0%  -14.07%  (p=0.010 n=6+4)

	name     old bin-B         new bin-B         delta
	Build-8        6.60M ± 0%        6.60M ± 0%   -0.01%  (p=0.002 n=6+6)

	name     old sys-time/op   new sys-time/op   delta
	Build-8        208ms ± 5%        149ms ± 3%  -28.27%  (p=0.004 n=6+5)

	name     old user-time/op  new user-time/op  delta
	Build-8        433ms ± 3%        384ms ± 3%  -11.35%  (p=0.002 n=6+6)

hash.go

@@ -18,11 +18,7 @@ const buildIDSeparator = "/"
 
 // splitActionID returns the action ID half of a build ID, the first component.
 func splitActionID(buildID string) string {
-	i := strings.Index(buildID, buildIDSeparator)
-	if i < 0 {
-		return buildID
-	}
-	return buildID[:i]
+	return buildID[:strings.Index(buildID, buildIDSeparator)]
 }
 
 // splitContentID returns the content ID half of a build ID, the last component.
@@ -80,24 +76,15 @@ func alterToolVersion(tool string, args []string) error {
 }
 
 func ownContentID(toolID []byte) ([]byte, error) {
-	// We can't rely on the module version to exist, because it's
-	// missing in local builds without 'go get'.
-	// For now, use 'go tool buildid' on the garble binary.
-	// Just like Go's own cache, we use hex-encoded sha256 sums.
-	// Once https://github.com/golang/go/issues/37475 is fixed, we
-	// can likely just use that.
-	binaryBuildID, err := buildidOf(cache.ExecPath)
-	if err != nil {
-		return nil, err
-	}
-	binaryContentID := decodeHash(splitContentID(binaryBuildID))
-
 	// 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
 	// content ID.
 	h := sha256.New()
 	h.Write(toolID)
-	h.Write(binaryContentID)
+	if len(cache.BinaryContentID) == 0 {
+		panic("missing binary content ID")
+	}
+	h.Write(cache.BinaryContentID)
 
 	// We also need to add the selected options to the full version string,
 	// because all of them result in different output. We use spaces to

main.go

@@ -102,13 +102,12 @@ var (
 	// Basic information about the package being currently compiled or linked.
 	curPkg *listedPackage
 
-	buildInfo = struct {
-		// TODO: do we still need imports?
-		imports map[string]importedPkg // parsed importCfg plus cached info
-	}{imports: make(map[string]importedPkg)}
-
-	garbledImporter = importer.ForCompiler(fset, "gc", func(path string) (io.ReadCloser, error) {
-		return os.Open(buildInfo.imports[path].packagefile)
+	// These are pulled from -importcfg in the current obfuscated build.
+	// As such, they contain export data for the dependencies which might be
+	// themselves obfuscated, depending on GOPRIVATE.
+	importCfgEntries map[string]*importCfgEntry
+	garbledImporter  = importer.ForCompiler(fset, "gc", func(path string) (io.ReadCloser, error) {
+		return os.Open(importCfgEntries[path].packagefile)
 	}).(types.ImporterFrom)
 
 	opts *flagOptions
@@ -116,29 +115,26 @@ var (
 	envGoPrivate = os.Getenv("GOPRIVATE") // complemented by 'go env' later
 )
 
-func garbledImport(path string) (*types.Package, error) {
-	ipkg, ok := buildInfo.imports[path]
+func obfuscatedTypesPackage(path string) *types.Package {
+	entry, ok := importCfgEntries[path]
 	if !ok {
-		return nil, fmt.Errorf("could not find imported package %q", path)
-	}
-	if ipkg.pkg != nil {
-		return ipkg.pkg, nil // cached
+		return nil
 	}
-	if opts.GarbleDir == "" {
-		return nil, fmt.Errorf("$GARBLE_DIR unset; did you run via 'garble build'?")
+	if entry.cachedPkg != nil {
+		return entry.cachedPkg
 	}
 	pkg, err := garbledImporter.ImportFrom(path, opts.GarbleDir, 0)
 	if err != nil {
-		return nil, err
+		return nil
 	}
-	ipkg.pkg = pkg // cache for later use
-	return pkg, nil
+	entry.cachedPkg = pkg // cache for later use
+	return pkg
 }
 
-type importedPkg struct {
+type importCfgEntry struct {
 	packagefile string
 
-	pkg *types.Package
+	cachedPkg *types.Package
 }
 
 func main1() int {
@@ -462,7 +458,7 @@ func transformCompile(args []string) ([]string, error) {
 	}
 	curPkg = cache.ListedPackages[curPkgPathFull]
 
-	newImportCfg, err := fillBuildInfo(flags)
+	newImportCfg, err := processImportCfg(flags)
 	if err != nil {
 		return nil, err
 	}
@@ -515,6 +511,7 @@ func transformCompile(args []string) ([]string, error) {
 		origImporter = importer.Default()
 	}
 
+	// TODO(mvdan): can we use IgnoreFuncBodies=true?
 	origTypesConfig := types.Config{Importer: origImporter}
 	tf.pkg, err = origTypesConfig.Check(curPkgPathFull, fset, files, tf.info)
 	if err != nil {
@@ -557,7 +554,6 @@ func transformCompile(args []string) ([]string, error) {
 	newPkgPath := curPkgPath
 	if curPkgPath != "main" && isPrivate(curPkgPath) {
 		newPkgPath = hashWith(curPkg.GarbleActionID, curPkgPath)
-		// println("compile -p:", curPkgPath, newPkgPath)
 		flags = flagSetValue(flags, "-p", newPkgPath)
 	}
 
@@ -730,8 +726,8 @@ func (tf *transformer) handleDirectives(comments []string) {
 		if err != nil {
 			continue // probably a made up symbol name
 		}
-		garbledPkg, _ := garbledImport(pkgPath)
-		if garbledPkg != nil && garbledPkg.Scope().Lookup(name) != nil {
+		obfPkg := obfuscatedTypesPackage(pkgPath)
+		if obfPkg != nil && obfPkg.Scope().Lookup(name) != nil {
 			continue // the name exists and was not garbled
 		}
 
@@ -855,10 +851,10 @@ func isPrivate(path string) bool {
 	return module.MatchPrefixPatterns(envGoPrivate, path)
 }
 
-// fillBuildInfo initializes the global buildInfo struct via the supplied flags,
-// and constructs a new importcfg with the obfuscated import paths changed as
+// processImportCfg initializes importCfgEntries via the supplied flags, and
+// constructs a new importcfg with the obfuscated import paths changed as
 // necessary.
-func fillBuildInfo(flags []string) (newImportCfg string, _ error) {
+func processImportCfg(flags []string) (newImportCfg string, _ error) {
 	importCfg := flagValue(flags, "-importcfg")
 	if importCfg == "" {
 		return "", fmt.Errorf("could not find -importcfg argument")
@@ -868,7 +864,9 @@ func fillBuildInfo(flags []string) (newImportCfg string, _ error) {
 		return "", err
 	}
 
+	importCfgEntries = make(map[string]*importCfgEntry)
 	importMap := make(map[string]string)
+
 	for _, line := range strings.SplitAfter(string(data), "\n") {
 		line = strings.TrimSpace(line)
 		if line == "" || strings.HasPrefix(line, "#") {
@@ -896,11 +894,11 @@ func fillBuildInfo(flags []string) (newImportCfg string, _ error) {
 			}
 			importPath, objectPath := args[:j], args[j+1:]
 
-			impPkg := importedPkg{packagefile: objectPath}
-			buildInfo.imports[importPath] = impPkg
+			impPkg := &importCfgEntry{packagefile: objectPath}
+			importCfgEntries[importPath] = impPkg
 
 			if otherPath, ok := importMap[importPath]; ok {
-				buildInfo.imports[otherPath] = impPkg
+				importCfgEntries[otherPath] = impPkg
 			}
 		}
 	}
@@ -916,7 +914,6 @@ func fillBuildInfo(flags []string) (newImportCfg string, _ error) {
 	}
 	for beforePath, afterPath := range importMap {
 		if isPrivate(beforePath) {
-			println(beforePath, afterPath)
 			pkg, err := listPackage(beforePath)
 			if err != nil {
 				panic(err) // shouldn't happen
@@ -925,7 +922,7 @@ func fillBuildInfo(flags []string) (newImportCfg string, _ error) {
 		}
 		fmt.Fprintf(newCfg, "importmap %s=%s\n", beforePath, afterPath)
 	}
-	for impPath, pkg := range buildInfo.imports {
+	for impPath, pkg := range importCfgEntries {
 		if isPrivate(impPath) {
 			pkg, err := listPackage(impPath)
 			if err != nil {
@@ -1095,8 +1092,8 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 					// We're not obfuscating cgo names.
 					return true
 				}
-				if garbledPkg, _ := garbledImport(path); garbledPkg != nil {
-					if garbledPkg.Scope().Lookup(named.Obj().Name()) != nil {
+				if obfPkg := obfuscatedTypesPackage(path); obfPkg != nil {
+					if obfPkg.Scope().Lookup(named.Obj().Name()) != nil {
 						recordStruct(named, tf.ignoreObjects)
 						return true
 					}
@@ -1115,8 +1112,8 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 				if named == nil {
 					break
 				}
-				if garbledPkg, _ := garbledImport(path); garbledPkg != nil {
-					if garbledPkg.Scope().Lookup(x.Name()) != nil {
+				if obfPkg := obfuscatedTypesPackage(path); obfPkg != nil {
+					if obfPkg.Scope().Lookup(x.Name()) != nil {
 						recordStruct(named, tf.ignoreObjects)
 						return true
 					}
@@ -1145,21 +1142,19 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 		if err != nil {
 			panic(err) // shouldn't happen
 		}
-		// 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 buildInfo.imports[path].packagefile != "" {
-			garbledPkg, err := garbledImport(path)
-			if err != nil {
-				panic(err) // shouldn't happen
-			}
-			// Check if the imported name wasn't garbled, e.g. if it's assembly.
-			// If the object returned from the garbled package's scope has a different type as the object
-			// we're searching for, they are most likely two separate objects with the same name, so ok to garble
-			if o := garbledPkg.Scope().Lookup(obj.Name()); o != nil && reflect.TypeOf(o) == reflect.TypeOf(obj) {
-				return true
-			}
+
+		obfPkg := obfuscatedTypesPackage(path)
+		// Check if the imported name wasn't garbled, e.g. if it's assembly.
+		// If the object returned from the garbled package's scope has a
+		// different type as the object we're searching for, they are
+		// most likely two separate objects with the same name, so ok to
+		// garble
+		if obfPkg == nil {
+			// TODO(mvdan): This is probably a bug.
+			// Add a test case where an indirect package has a name
+			// that we did not obfuscate.
+		} else if o := obfPkg.Scope().Lookup(obj.Name()); o != nil && reflect.TypeOf(o) == reflect.TypeOf(obj) {
+			return true
 		}
 
 		origName := node.Name
@@ -1236,7 +1231,7 @@ func transformLink(args []string) ([]string, error) {
 
 	curPkg = cache.ListedPackages[cache.MainImportPath]
 
-	newImportCfg, err := fillBuildInfo(flags)
+	newImportCfg, err := processImportCfg(flags)
 	if err != nil {
 		return nil, err
 	}

reverse.go

@@ -33,6 +33,8 @@ func commandReverse(args []string) error {
 	}
 	listArgs = append(listArgs, flags...)
 	listArgs = append(listArgs, mainPkg)
+	// TODO: We most likely no longer need this "list -toolexec" call, since
+	// we use the original build IDs.
 	cmd, err := toolexecCmd("list", listArgs)
 	if err != nil {
 		return err
@@ -70,9 +72,6 @@ func commandReverse(args []string) error {
 		if isPrivate(pkg.ImportPath) {
 			privatePkgPaths = append(privatePkgPaths, pkg.ImportPath)
 		}
-		// The action ID, and possibly the export file, will be used
-		// later to reconstruct the mapping of obfuscated names.
-		buildInfo.imports[pkg.ImportPath] = importedPkg{packagefile: pkg.Export}
 	}
 
 	if err := cmd.Wait(); err != nil {

shared.go

@@ -15,7 +15,8 @@ import (
 	"strings"
 )
 
-// sharedCache this data is sharedCache between the different garble processes.
+// sharedCache is shared as a read-only cache between the many garble toolexec
+// sub-processes.
 //
 // Note that we fill this cache once from the root process in saveListedPackages,
 // store it into a temporary file via gob encoding, and then reuse that file
@@ -30,6 +31,15 @@ type sharedCache struct {
 	// allows us to obtain the non-garbled export data of all dependencies, useful
 	// for type checking of the packages as we obfuscate them.
 	ListedPackages map[string]*listedPackage
+
+	// We can't rely on the module version to exist, because it's
+	// missing in local builds without 'go get'.
+	// For now, use 'go tool buildid' on the garble binary.
+	// Just like Go's own cache, we use hex-encoded sha256 sums.
+	// Once https://github.com/golang/go/issues/37475 is fixed, we
+	// can likely just use that.
+	BinaryContentID []byte
+
 	MainImportPath string // TODO: remove with TOOLEXEC_IMPORTPATH
 }
 
@@ -190,7 +200,7 @@ func setListedPackages(patterns []string) error {
 	if err != nil {
 		return err
 	}
-	binaryContentID := decodeHash(splitContentID(binaryBuildID))
+	cache.BinaryContentID = decodeHash(splitContentID(binaryBuildID))
 
 	dec := json.NewDecoder(stdout)
 	cache.ListedPackages = make(map[string]*listedPackage)
@@ -211,7 +221,7 @@ func setListedPackages(patterns []string) error {
 			actionID := decodeHash(splitActionID(buildID))
 			h := sha256.New()
 			h.Write(actionID)
-			h.Write(binaryContentID)
+			h.Write(cache.BinaryContentID)
 
 			pkg.GarbleActionID = h.Sum(nil)[:buildIDComponentLength]
 		}