From f0d79a38d4afc2daf1df92117859340e90327491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 19 May 2022 12:09:01 +0100 Subject: [PATCH] remove a couple of easy TODOs First, I tried to follow my own past advice to only set GarbleActionID if ToObfuscate is true. However, that broke at least three parts of transformCompile, as the hash is used for more than I recalled. Give up on that idea, because the current code is working as intended. Better document what GarbleActionID is and what we use it for. Second, now that https://go.dev/cl/348741 was shipped with Go 1.18, using the logger when its output is io.Discard is already a no-op. So we no longer need our debugf wrapper to apply the no-op logic. --- main.go | 32 +++++++++++--------------------- shared.go | 15 +++++++++------ 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/main.go b/main.go index 9ac2527..7b1cd58 100644 --- a/main.go +++ b/main.go @@ -192,16 +192,6 @@ func (w *uniqueLineWriter) Write(p []byte) (n int, err error) { return w.out.Write(p) } -// debugf is like log.Printf, but it is a no-op by default. -// TODO(mvdan): use our own debug logger instead of hijacking the global one, -// and drop the "flagDebug" no-op check now that we require Go 1.18 or later. -func debugf(format string, args ...any) { - if !flagDebug { - return - } - log.Printf(format, args...) -} - // debugSince is like time.Since but resulting in shorter output. // A build process takes at least hundreds of milliseconds, // so extra decimal points in the order of microseconds aren't meaningful. @@ -393,7 +383,7 @@ func mainErr(args []string) error { } cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - debugf("calling via toolexec: %s", cmd) + log.Printf("calling via toolexec: %s", cmd) return cmd.Run() } @@ -429,14 +419,14 @@ func mainErr(args []string) error { transformed := args[1:] if transform != nil { startTime := time.Now() - debugf("transforming %s with args: %s", tool, strings.Join(transformed, " ")) + log.Printf("transforming %s with args: %s", tool, strings.Join(transformed, " ")) var err error if transformed, err = transform(transformed); err != nil { return err } - debugf("transformed args for %s in %s: %s", tool, debugSince(startTime), strings.Join(transformed, " ")) + log.Printf("transformed args for %s in %s: %s", tool, debugSince(startTime), strings.Join(transformed, " ")) } else { - debugf("skipping transform on %s with args: %s", tool, strings.Join(transformed, " ")) + log.Printf("skipping transform on %s with args: %s", tool, strings.Join(transformed, " ")) } cmd := exec.Command(args[0], transformed...) cmd.Stdout = os.Stdout @@ -674,7 +664,7 @@ func transformAsm(args []string) ([]string, error) { } newName := hashWithPackage(curPkg, name) - debugf("asm name %q hashed with %x to %q", name, curPkg.GarbleActionID, newName) + log.Printf("asm name %q hashed with %x to %q", name, curPkg.GarbleActionID, newName) buf.WriteString(newName) } @@ -761,7 +751,7 @@ func transformCompile(args []string) ([]string, error) { if flagSeed.present() { randSeed = flagSeed.bytes } - // debugf("seeding math/rand with %x\n", randSeed) + // log.Printf("seeding math/rand with %x\n", randSeed) mathrand.Seed(int64(binary.BigEndian.Uint64(randSeed))) if err := tf.prefillObjectMaps(files); err != nil { @@ -782,7 +772,7 @@ func transformCompile(args []string) ([]string, error) { for i, file := range files { filename := filepath.Base(paths[i]) - debugf("obfuscating %s", filename) + log.Printf("obfuscating %s", filename) if curPkg.ImportPath == "runtime" && flagTiny { // strip unneeded runtime code stripRuntime(filename, file) @@ -1117,7 +1107,7 @@ func loadCachedOutputs() error { } loaded++ } - debugf("%d cached output files loaded in %s", loaded, debugSince(startTime)) + log.Printf("%d cached output files loaded in %s", loaded, debugSince(startTime)) return nil } @@ -1674,7 +1664,7 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { hashToUse := lpkg.GarbleActionID debugName := "variable" - // debugf("%s: %#v %T", fset.Position(node.Pos()), node, obj) + // log.Printf("%s: %#v %T", fset.Position(node.Pos()), node, obj) switch obj := obj.(type) { case *types.Var: if !obj.IsField() { @@ -1701,7 +1691,7 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { panic("could not find for " + name) } node.Name = hashWithStruct(strct, name) - debugf("%s %q hashed with struct fields to %q", debugName, name, node.Name) + log.Printf("%s %q hashed with struct fields to %q", debugName, name, node.Name) return true case *types.TypeName: @@ -1729,7 +1719,7 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File { node.Name = hashWithPackage(lpkg, name) // TODO: probably move the debugf lines inside the hash funcs - debugf("%s %q hashed with %x… to %q", debugName, name, hashToUse[:4], node.Name) + log.Printf("%s %q hashed with %x… to %q", debugName, name, hashToUse[:4], node.Name) return true } post := func(cursor *astutil.Cursor) bool { diff --git a/shared.go b/shared.go index 583e1ee..7595c96 100644 --- a/shared.go +++ b/shared.go @@ -8,6 +8,7 @@ import ( "encoding/gob" "encoding/json" "fmt" + "log" "os" "os/exec" "path/filepath" @@ -67,7 +68,7 @@ func loadSharedCache() error { return fmt.Errorf(`cannot open shared file: %v; did you run "garble [command]"?`, err) } defer func() { - debugf("shared cache loaded in %s from %s", debugSince(startTime), f.Name()) + log.Printf("shared cache loaded in %s from %s", debugSince(startTime), f.Name()) }() defer f.Close() if err := gob.NewDecoder(f).Decode(&cache); err != nil { @@ -148,8 +149,10 @@ type listedPackage struct { // between garble processes. Use "Garble" as a prefix to ensure no // collisions with the JSON fields from 'go list'. - // TODO(mvdan): consider filling this iff ToObfuscate==true, - // which will help ensure we don't obfuscate any of their names otherwise. + // GarbleActionID is a hash combining the Action ID from BuildID, + // with Garble's own inputs as per addGarbleToHash. + // It is set even when ToObfuscate is false, as it is also used for random + // seeds and build cache paths, and not just to obfuscate names. GarbleActionID []byte `json:"-"` // ToObfuscate records whether the package should be obfuscated. @@ -166,7 +169,7 @@ func (p *listedPackage) obfuscatedImportPath() string { return p.ImportPath } newPath := hashWithPackage(p, p.ImportPath) - debugf("import path %q hashed with %x to %q", p.ImportPath, p.GarbleActionID, newPath) + log.Printf("import path %q hashed with %x to %q", p.ImportPath, p.GarbleActionID, newPath) return newPath } @@ -186,7 +189,7 @@ func appendListedPackages(packages []string, withDeps bool) error { cmd := exec.Command("go", args...) defer func() { - debugf("original build info obtained in %s via: go %s", debugSince(startTime), strings.Join(args, " ")) + log.Printf("original build info obtained in %s via: go %s", debugSince(startTime), strings.Join(args, " ")) }() stdout, err := cmd.StdoutPipe() @@ -375,7 +378,7 @@ func listPackage(path string) (*listedPackage, error) { panic(fmt.Sprintf("runtime listed a std package we can't find: %s", path)) } listedRuntimeLinknamed = true - debugf("listed %d missing runtime-linknamed packages in %s", len(missing), debugSince(startTime)) + log.Printf("listed %d missing runtime-linknamed packages in %s", len(missing), debugSince(startTime)) return pkg, nil } // Packages other than runtime can list any package,