support computing missing pkgCache entries

Some users had been running into "cannot load cache entry" errors,
which could happen if garble's cache files in GOCACHE were removed
when Go's own cache files were not.

Now that we've moved to our own separate cache directory,
and that we've refactored the codebase to depend less on globals
and no longer assume that we're loading info for the current package,
we can now compute a pkgCache entry for a dependency if needed.

We add a pkgCache.CopyFrom method to be able to append map entries
from one pkgCache to another without needing an encoding/gob roundtrip.

We also add a parseFiles helper, since we now have three bits of code
which need to parse a list of Go files from disk.

Fixes #708.

main.go

@@ -36,6 +36,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/rogpeppe/go-internal/cache"
+	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
 	"golang.org/x/mod/module"
 	"golang.org/x/mod/semver"
@@ -887,22 +888,36 @@ func (tf *transformer) writeSourceFile(basename, obfuscated string, content []by
 	return dstPath, nil
 }
 
-func (tf *transformer) transformCompile(args []string) ([]string, error) {
+// parseFiles parses a list of Go files.
-	var err error
+// It supports relative file paths, such as those found in listedPackage.CompiledGoFiles,
-	flags, paths := splitFlagsFromFiles(args, ".go")
+// as long as dir is set to listedPackage.Dir.
-
+func parseFiles(dir string, paths []string) ([]*ast.File, error) {
-	// We will force the linker to drop DWARF via -w, so don't spend time
-	// generating it.
-	flags = append(flags, "-dwarf=false")
-
 	var files []*ast.File
 	for _, path := range paths {
+		if !filepath.IsAbs(path) {
+			path = filepath.Join(dir, path)
+		}
 		file, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution|parser.ParseComments)
 		if err != nil {
 			return nil, err
 		}
 		files = append(files, file)
 	}
+	return files, nil
+}
+
+func (tf *transformer) transformCompile(args []string) ([]string, error) {
+	flags, paths := splitFlagsFromFiles(args, ".go")
+
+	// We will force the linker to drop DWARF via -w, so don't spend time
+	// generating it.
+	flags = append(flags, "-dwarf=false")
+
+	// The Go file paths given to the compiler are always absolute paths.
+	files, err := parseFiles("", paths)
+	if err != nil {
+		return nil, err
+	}
 
 	// Even if loadPkgCache below finds a direct cache hit,
 	// other parts of garble still need type information to obfuscate.
@@ -1253,10 +1268,9 @@ type (
 	}
 )
 
-// pkgCache contains information that will be stored in fsCache.
+// pkgCache contains information about a package that will be stored in fsCache.
-// Note that pkgCache gets loaded from all direct package dependencies,
+// Note that pkgCache is "deep", containing information about all packages
-// and gets filled while obfuscating the current package, so it ends up
+// which are transitive dependencies as well.
-// containing entries for the current package and its transitive dependencies.
 type pkgCache struct {
 	// ReflectAPIs is a static record of what std APIs use reflection on their
 	// parameters, so we can avoid obfuscating types used with them.
@@ -1283,6 +1297,12 @@ type pkgCache struct {
 	EmbeddedAliasFields map[objectString]typeName
 }
 
+func (c *pkgCache) CopyFrom(c2 pkgCache) {
+	maps.Copy(c.ReflectAPIs, c2.ReflectAPIs)
+	maps.Copy(c.ReflectObjects, c2.ReflectObjects)
+	maps.Copy(c.EmbeddedAliasFields, c2.EmbeddedAliasFields)
+}
+
 func openCache() (*cache.Cache, error) {
 	dir := os.Getenv("GARBLE_CACHE") // e.g. "~/.cache/garble"
 	if dir == "" {
@@ -1321,7 +1341,10 @@ func loadPkgCache(lpkg *listedPackage, pkg *types.Package, files []*ast.File, in
 		}
 		return loaded, nil
 	}
+	return computePkgCache(fsCache, lpkg, pkg, files, info)
+}
 
+func computePkgCache(fsCache *cache.Cache, lpkg *listedPackage, pkg *types.Package, files []*ast.File, info *types.Info) (pkgCache, error) {
 	// Not yet in the cache. Load the cache entries for all direct dependencies,
 	// build our cache entry, and write it to disk.
 	// Note that practically all errors from Cache.GetFile are a cache miss;
@@ -1331,7 +1354,6 @@ func loadPkgCache(lpkg *listedPackage, pkg *types.Package, files []*ast.File, in
 	// TODO: if A (curPkg) imports B and C, and B also imports C,
 	// then loading the gob files from both B and C is unnecessary;
 	// loading B's gob file would be enough. Is there an easy way to do that?
-	startTime := time.Now()
 	computed := pkgCache{
 		ReflectAPIs: map[funcFullName]map[int]bool{
 			"reflect.TypeOf":  {0: true},
@@ -1340,41 +1362,55 @@ func loadPkgCache(lpkg *listedPackage, pkg *types.Package, files []*ast.File, in
 		ReflectObjects:      map[objectString]struct{}{},
 		EmbeddedAliasFields: map[objectString]typeName{},
 	}
-	loaded := 0
+	for _, imp := range lpkg.Imports {
-	for _, path := range lpkg.Imports {
+		if imp == "C" {
-		if path == "C" {
+			// `go list -json` shows "C" in Imports but not Deps.
-			// `go list -json` shows "C" in Imports but not Deps. A bug?
+			// See https://go.dev/issue/60453.
 			continue
 		}
-		pkg, err := listPackage(lpkg, path)
+		// Shadowing lpkg ensures we don't use the wrong listedPackage below.
+		lpkg, err := listPackage(lpkg, imp)
 		if err != nil {
 			panic(err) // shouldn't happen
 		}
-		if pkg.BuildID == "" {
+		if lpkg.BuildID == "" {
 			continue // nothing to load
 		}
-		// this function literal is used for the deferred close
+		if err := func() error { // function literal for the deferred close
-		if err := func() error {
+			if filename, _, err := fsCache.GetFile(lpkg.GarbleActionID); err == nil {
-			filename, _, err := fsCache.GetFile(pkg.GarbleActionID)
+				// Cache hit; append new entries to computed.
+				f, err := os.Open(filename)
+				if err != nil {
+					return err
+				}
+				defer f.Close()
+				if err := gob.NewDecoder(f).Decode(&computed); err != nil {
+					return fmt.Errorf("gob decode: %w", err)
+				}
+				return nil
+			}
+			// Missing or corrupted entry in the cache for a dependency.
+			// Could happen if GARBLE_CACHE was emptied but GOCACHE was not.
+			// Compute it, which can recurse if many entries are missing.
+			files, err := parseFiles(lpkg.Dir, lpkg.CompiledGoFiles)
 			if err != nil {
 				return err
 			}
-			f, err := os.Open(filename)
+			origImporter := importerForPkg(lpkg)
+			pkg, info, err := typecheck(lpkg.ImportPath, files, origImporter)
 			if err != nil {
 				return err
 			}
-			defer f.Close()
+			computedImp, err := computePkgCache(fsCache, lpkg, pkg, files, info)
-			// Decode appends new entries to the existing maps
+			if err != nil {
-			if err := gob.NewDecoder(f).Decode(&computed); err != nil {
+				return err
-				return fmt.Errorf("gob decode: %w", err)
 			}
+			computed.CopyFrom(computedImp)
 			return nil
 		}(); err != nil {
-			return pkgCache{}, fmt.Errorf("cannot load cache entry for %s: %w", path, err)
+			return pkgCache{}, fmt.Errorf("pkgCache load for %s: %w", imp, err)
 		}
-		loaded++
 	}
-	log.Printf("%d cached output files loaded in %s", loaded, debugSince(startTime))
 
 	// Fill EmbeddedAliasFields from the type info.
 	for name, obj := range info.Uses {

reverse.go

@@ -8,11 +8,9 @@ import (
 	"flag"
 	"fmt"
 	"go/ast"
-	"go/parser"
 	"go/types"
 	"io"
 	"os"
-	"path/filepath"
 	"strings"
 )
 
@@ -77,19 +75,9 @@ One can reverse a captured panic stack trace as follows:
 		// Package paths are obfuscated, too.
 		addHashedWithPackage(lpkg.ImportPath)
 
-		var files []*ast.File
+		files, err := parseFiles(lpkg.Dir, lpkg.CompiledGoFiles)
-		for _, goFile := range lpkg.CompiledGoFiles {
+		if err != nil {
-			// Direct Go files may be relative paths like "foo.go".
+			return err
-			// Compiled Go files, such as those generated from cgo,
-			// may be absolute paths inside the Go build cache.
-			if !filepath.IsAbs(goFile) {
-				goFile = filepath.Join(lpkg.Dir, goFile)
-			}
-			file, err := parser.ParseFile(fset, goFile, nil, parser.SkipObjectResolution)
-			if err != nil {
-				return fmt.Errorf("go parse: %w", err)
-			}
-			files = append(files, file)
 		}
 		origImporter := importerForPkg(lpkg)
 		_, info, err := typecheck(lpkg.ImportPath, files, origImporter)

testdata/script/cache.txtar

@@ -17,12 +17,9 @@ rm garble-cache
 # level1c has the garble build cached with all files available.
 exec garble build ./level1c
 
-# TODO: this test now fails due to our fragile caching.
+exec garble build
-! exec garble build
+exec ./main
-stderr 'cannot load cache entry for test/main/level1b'
+cmp stderr main.stderr
-# exec garble build
-# exec ./main
-# cmp stderr main.stderr
 
 # verify with regular Go.
 go build