From 79376a15f9aa96f921055d4743fd89ca18504bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sat, 3 Jun 2023 20:31:36 +0100 Subject: [PATCH] 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 | 96 +++++++++++++++++++++++++------------ reverse.go | 18 ++----- testdata/script/cache.txtar | 9 ++-- 3 files changed, 72 insertions(+), 51 deletions(-) diff --git a/main.go b/main.go index 86e7f0f..3d4ae67 100644 --- a/main.go +++ b/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) { - var err 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") - +// parseFiles parses a list of Go files. +// It supports relative file paths, such as those found in listedPackage.CompiledGoFiles, +// as long as dir is set to listedPackage.Dir. +func parseFiles(dir string, paths []string) ([]*ast.File, error) { 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. -// Note that pkgCache gets loaded from all direct package dependencies, -// and gets filled while obfuscating the current package, so it ends up -// containing entries for the current package and its transitive dependencies. +// pkgCache contains information about a package that will be stored in fsCache. +// Note that pkgCache is "deep", containing information about all packages +// which are transitive dependencies as well. 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 _, path := range lpkg.Imports { - if path == "C" { - // `go list -json` shows "C" in Imports but not Deps. A bug? + for _, imp := range lpkg.Imports { + if imp == "C" { + // `go list -json` shows "C" in Imports but not Deps. + // 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 { - filename, _, err := fsCache.GetFile(pkg.GarbleActionID) + if err := func() error { // function literal for the deferred close + if filename, _, err := fsCache.GetFile(lpkg.GarbleActionID); err == nil { + // 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() - // Decode appends new entries to the existing maps - if err := gob.NewDecoder(f).Decode(&computed); err != nil { - return fmt.Errorf("gob decode: %w", err) + computedImp, err := computePkgCache(fsCache, lpkg, pkg, files, info) + if err != nil { + return 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 { diff --git a/reverse.go b/reverse.go index 32d9de7..63dd381 100644 --- a/reverse.go +++ b/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 - for _, goFile := range lpkg.CompiledGoFiles { - // Direct Go files may be relative paths like "foo.go". - // 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) + files, err := parseFiles(lpkg.Dir, lpkg.CompiledGoFiles) + if err != nil { + return err } origImporter := importerForPkg(lpkg) _, info, err := typecheck(lpkg.ImportPath, files, origImporter) diff --git a/testdata/script/cache.txtar b/testdata/script/cache.txtar index 8337207..317fbb0 100644 --- a/testdata/script/cache.txtar +++ b/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 -stderr 'cannot load cache entry for test/main/level1b' -# exec garble build -# exec ./main -# cmp stderr main.stderr +exec garble build +exec ./main +cmp stderr main.stderr # verify with regular Go. go build