From 481e3a1f091d54a7cf26d95cf32777245fae328d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 5 Dec 2022 18:29:10 +0000
Subject: [PATCH] default to GOGARBLE=*, stop using GOPRIVATE

We can drop the code that kicked in when GOGARBLE was empty.
We can also add the value in addGarbleToHash unconditionally,
as we never allow it to be empty.

In the tests, remove all GOGARBLE lines where it just meant "obfuscate
everything" or "obfuscate the entire main module".

cgo.txtar had "obfuscate everything" as a separate step,
so remove it entirely.

linkname.txtar started failing because the imported package did not
import strings, so listPackage errored out. This wasn't a problem when
strings itself wasn't obfuscated, as transformLinkname silently left
strings.IndexByte untouched. It is a problem when IndexByte does get
obfuscated. Make that kind of listPackage error visible, and fix it.

reflect.txtar started failing with "unreachable method" runtime throws.
It's not clear to me why; it appears that GOGARBLE=* makes the linker
think that ExportedMethodName is suddenly unreachable.
Work around the problem by making the method explicitly reachable,
and leave a TODO as a reminder to investigate.

Finally, gogarble.txtar no longer needs to test for GOPRIVATE.
The rest of the test is left the same, as we still want the various
values for GOGARBLE to continue to work just like before.

Fixes #594.
---
 CHANGELOG.md                     |  3 +++
 README.md                        | 10 ++++-----
 bench_test.go                    |  1 -
 hash.go                          |  4 +---
 main.go                          | 38 +++++++++++++++-----------------
 scripts/check-third-party.sh     |  2 --
 shared.go                        |  4 +---
 testdata/script/asm.txtar        |  2 --
 testdata/script/cgo.txtar        |  8 -------
 testdata/script/crossbuild.txtar |  1 -
 testdata/script/debugdir.txtar   |  2 --
 testdata/script/embed.txtar      |  2 --
 testdata/script/gogarble.txtar   |  8 ++-----
 testdata/script/help.txtar       |  2 --
 testdata/script/implement.txtar  |  2 --
 testdata/script/imports.txtar    |  6 ++---
 testdata/script/ldflags.txtar    |  2 --
 testdata/script/linkname.txtar   |  3 +--
 testdata/script/literals.txtar   |  3 ---
 testdata/script/modinfo.txtar    |  2 --
 testdata/script/plugin.txtar     |  2 --
 testdata/script/position.txtar   |  2 --
 testdata/script/reflect.txtar    |  8 +++++--
 testdata/script/reverse.txtar    |  2 --
 testdata/script/seed-cache.txtar |  1 -
 testdata/script/seed.txtar       |  2 --
 testdata/script/syntax.txtar     |  2 --
 testdata/script/tiny.txtar       |  2 --
 testdata/script/typeparams.txtar |  2 --
 29 files changed, 38 insertions(+), 90 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8d68196..c6037dc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@ and adds initial support for the upcoming Go 1.20.
 
 Noteworthy changes include:
 
+* `GOGARBLE=*` is now the default to obfuscate all packages - [#594]
+* `GOPRIVATE` is no longer used, being deprecated in [v0.5.0]
 * Obfuscate assembly source code filenames - [#605]
 * Randomize the lengths of obfuscated names
 * Support obfuscating `time` and `syscall`
@@ -157,6 +159,7 @@ Known bugs:
 * obfuscating the standard library with `GOPRIVATE=*` is not well supported yet
 * `garble test` is temporarily disabled, as it is currently broken
 
+[#594]: https://github.com/burrowers/garble/issues/594
 [#605]: https://github.com/burrowers/garble/issues/605
 
 [v0.7.2]: https://github.com/burrowers/garble/releases/tag/v0.7.2
diff --git a/README.md b/README.md
index b20e06d..5876e54 100644
--- a/README.md
+++ b/README.md
@@ -34,12 +34,10 @@ order to:
 * [Obfuscate literals](#literal-obfuscation), if the `-literals` flag is given
 * Remove [extra information](#tiny-mode), if the `-tiny` flag is given
 
-The tool obfuscates the packages matching `GOGARBLE`, a comma-separated list of
-glob patterns of module path prefixes, as documented in `go help private`.
-To obfuscate all the packages in a build, use `GOGARBLE=*`.
-When `GOGARBLE` is empty, it assumes the value of `GOPRIVATE`.
-When `GOPRIVATE` is also empty, then `GOGARBLE` assumes the value of the current
-module path, to obfuscate all packages under the current module.
+By default, the tool obfuscates all the packages being built.
+You can manually specify which packages to obfuscate via `GOGARBLE`,
+a comma-separated list of glob patterns matching package path prefixes.
+This format is borrowed from `GOPRIVATE`; see `go help private`.
 
 Note that commands like `garble build` will use the `go` version found in your
 `$PATH`. To use different versions of Go, you can
diff --git a/bench_test.go b/bench_test.go
index ed8f544..f0834f3 100644
--- a/bench_test.go
+++ b/bench_test.go
@@ -84,7 +84,6 @@ func BenchmarkBuild(b *testing.B) {
 		gocache, err := os.MkdirTemp(b.TempDir(), "gocache-*")
 		qt.Assert(b, err, qt.IsNil)
 		env := append(os.Environ(),
-			"GOGARBLE=*",
 			"GOCACHE="+gocache,
 			"GARBLE_WRITE_ALLOCS=true",
 		)
diff --git a/hash.go b/hash.go
index e5d27d2..3218fa5 100644
--- a/hash.go
+++ b/hash.go
@@ -98,9 +98,7 @@ func addGarbleToHash(inputHash []byte) []byte {
 	// 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
 	// separate the env vars and flags, to reduce the chances of collisions.
-	if cache.GOGARBLE != "" {
-		fmt.Fprintf(hasher, " GOGARBLE=%s", cache.GOGARBLE)
-	}
+	fmt.Fprintf(hasher, " GOGARBLE=%s", cache.GOGARBLE)
 	appendFlags(hasher, true)
 	// addGarbleToHash returns the sum buffer, so we need a new copy.
 	// Otherwise the next use of the global sumBuffer would conflict.
diff --git a/main.go b/main.go
index b32aa53..191ddee 100644
--- a/main.go
+++ b/main.go
@@ -36,7 +36,6 @@ import (
 
 	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
-	"golang.org/x/mod/modfile"
 	"golang.org/x/mod/module"
 	"golang.org/x/mod/semver"
 	"golang.org/x/tools/go/ast/astutil"
@@ -998,7 +997,20 @@ func (tf *transformer) transformLinkname(localName, newName string) (string, str
 
 	lpkg, err := listPackage(pkgPath)
 	if err != nil {
-		// Probably a made up name like above, but with a dot.
+		// TODO(mvdan): use errors.As or errors.Is instead
+		if strings.Contains(err.Error(), "path not found") {
+			// Probably a made up name like above, but with a dot.
+			return localName, newName
+		}
+		if strings.Contains(err.Error(), "refusing to list") {
+			fmt.Fprintf(os.Stderr,
+				"//go:linkname refers to %s - add `import _ %q` so garble can find the package",
+				newName, pkgPath)
+			return localName, newName
+		}
+		if err != nil {
+			panic(err) // shouldn't happen
+		}
 		return localName, newName
 	}
 	if lpkg.ToObfuscate {
@@ -2185,7 +2197,8 @@ func flagSetValue(flags []string, name, value string) []string {
 
 func fetchGoEnv() error {
 	out, err := exec.Command("go", "env", "-json",
-		"GOOS", "GOPRIVATE", "GOMOD", "GOVERSION", "GOCACHE",
+		// Keep in sync with sharedCache.GoEnv.
+		"GOOS", "GOMOD", "GOVERSION",
 	).CombinedOutput()
 	if err != nil {
 		// TODO: cover this in the tests.
@@ -2201,23 +2214,8 @@ To install Go, see: https://go.dev/doc/install
 		return fmt.Errorf(`cannot unmarshal from "go env -json": %w`, err)
 	}
 	cache.GOGARBLE = os.Getenv("GOGARBLE")
-	if cache.GOGARBLE != "" {
-		// GOGARBLE is non-empty; nothing to do.
-	} else if cache.GoEnv.GOPRIVATE != "" {
-		// GOGARBLE is empty and GOPRIVATE is non-empty.
-		// Set GOGARBLE to GOPRIVATE's value.
-		cache.GOGARBLE = cache.GoEnv.GOPRIVATE
-	} else {
-		// If GOPRIVATE isn't set and we're in a module, use its module
-		// path as a GOPRIVATE default. Include a _test variant too.
-		// TODO(mvdan): we shouldn't need the _test variant here,
-		// as the import path should not include it; only the package name.
-		if mod, err := os.ReadFile(cache.GoEnv.GOMOD); err == nil {
-			modpath := modfile.ModulePath(mod)
-			if modpath != "" {
-				cache.GOGARBLE = modpath + "," + modpath + "_test"
-			}
-		}
+	if cache.GOGARBLE == "" {
+		cache.GOGARBLE = "*" // we default to obfuscating everything
 	}
 	return nil
 }
diff --git a/scripts/check-third-party.sh b/scripts/check-third-party.sh
index 1d2517a..54b4550 100755
--- a/scripts/check-third-party.sh
+++ b/scripts/check-third-party.sh
@@ -38,8 +38,6 @@ modules=(
 
 SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 
-export GOGARBLE="*"
-
 exit_code=0
 
 show() {
diff --git a/shared.go b/shared.go
index 2156eed..3d2ab2a 100644
--- a/shared.go
+++ b/shared.go
@@ -44,14 +44,12 @@ type sharedCache struct {
 	GOGARBLE string
 
 	// Filled directly from "go env".
-	// Remember to update the exec call when adding or removing names.
+	// Keep in sync with fetchGoEnv.
 	GoEnv struct {
 		GOOS string // i.e. the GOOS build target
 
-		GOPRIVATE string
 		GOMOD     string
 		GOVERSION string
-		GOCACHE   string
 	}
 }
 
diff --git a/testdata/script/asm.txtar b/testdata/script/asm.txtar
index 26eb7eb..0212e6a 100644
--- a/testdata/script/asm.txtar
+++ b/testdata/script/asm.txtar
@@ -3,8 +3,6 @@
 # TODO: support arm64, at least
 [!amd64] skip 'the assembly is only written for amd64'
 
-env GOGARBLE=test/main
-
 garble build
 exec ./main
 cmp stderr main.stderr
diff --git a/testdata/script/cgo.txtar b/testdata/script/cgo.txtar
index 570ae74..cacfcb0 100644
--- a/testdata/script/cgo.txtar
+++ b/testdata/script/cgo.txtar
@@ -1,7 +1,5 @@
 [!cgo] skip 'this test requires cgo to be enabled'
 
-env GOGARBLE=test/main
-
 garble build
 ! stderr 'warning' # check that the C toolchain is happy
 exec ./main
@@ -19,12 +17,6 @@ garble reverse .
 cmp stdout reversed.stdout
 env GARBLE_TEST_REVERSING=false
 
-env GOGARBLE=*
-garble build
-exec ./main
-cmp stdout main.stdout
-env GOGARBLE=test/main
-
 garble -tiny build
 exec ./main
 cmp stdout main.stdout
diff --git a/testdata/script/crossbuild.txtar b/testdata/script/crossbuild.txtar
index ff8b242..faa14c9 100644
--- a/testdata/script/crossbuild.txtar
+++ b/testdata/script/crossbuild.txtar
@@ -9,7 +9,6 @@
 [arm] env GOARCH=arm64
 
 # A fairly average Go build, importing some std libraries.
-env GOGARBLE='*'
 garble build
 -- go.mod --
 module test/main
diff --git a/testdata/script/debugdir.txtar b/testdata/script/debugdir.txtar
index 40f4ac9..a1207f7 100644
--- a/testdata/script/debugdir.txtar
+++ b/testdata/script/debugdir.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=*
-
 garble -debugdir ./debug1 build
 exists 'debug1/test/main/imported/imported.go' 'debug1/test/main/main.go' 'debug1/reflect/type.go'
 exists 'debug1/runtime/map.go' 'debug1/runtime/funcdata.h' 'debug1/runtime/asm.s'
diff --git a/testdata/script/embed.txtar b/testdata/script/embed.txtar
index c8f9052..d045508 100644
--- a/testdata/script/embed.txtar
+++ b/testdata/script/embed.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=*
-
 garble build
 
 exec ./main
diff --git a/testdata/script/gogarble.txtar b/testdata/script/gogarble.txtar
index 5c6cd4c..07901b6 100644
--- a/testdata/script/gogarble.txtar
+++ b/testdata/script/gogarble.txtar
@@ -1,13 +1,8 @@
-# Ensure that "does not match any packages" works with GOPRIVATE and GOGARBLE.
+# Ensure that "does not match any packages" works.
 env GOGARBLE=match-absolutely/nothing
 ! garble build -o=out ./standalone
 stderr '^GOGARBLE="match-absolutely/nothing" does not match any packages to be built$'
 
-env GOGARBLE=
-env GOPRIVATE=match-absolutely/nothing
-! garble build -o=out ./standalone
-stderr '^GOGARBLE="match-absolutely/nothing" does not match any packages to be built$'
-
 # A build where just some packages are obfuscated.
 env GOGARBLE=test/main/imported
 garble -literals build -o=out ./importer
@@ -23,6 +18,7 @@ garble build -o=out ./stdimporter
 
 [short] stop # rebuilding std is slow
 
+# Go back to the default of obfuscating all packages.
 env GOGARBLE='*'
 
 # Try garbling all of std, given some std packages.
diff --git a/testdata/script/help.txtar b/testdata/script/help.txtar
index c0c7ac3..0eb4829 100644
--- a/testdata/script/help.txtar
+++ b/testdata/script/help.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=*
-
 ! garble
 stderr 'Garble obfuscates Go code'
 stderr 'garble \[garble flags\] command'
diff --git a/testdata/script/implement.txtar b/testdata/script/implement.txtar
index 3dff9e5..7eb51ca 100644
--- a/testdata/script/implement.txtar
+++ b/testdata/script/implement.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 garble build
 exec ./main
 cmp stdout main.stdout
diff --git a/testdata/script/imports.txtar b/testdata/script/imports.txtar
index 5659e0a..e527d4f 100644
--- a/testdata/script/imports.txtar
+++ b/testdata/script/imports.txtar
@@ -1,7 +1,5 @@
-# Note that this is the only test with a module where we rely on the detection
-# of GOGARBLE.
-# Also note that, since this is the only test using "real" external modules
-# fetched via GOPROXY, go.mod and go.sum should declare the dependencies.
+# Since this is the only test using "real" external modules fetched via GOPROXY,
+# go.mod and go.sum should declare the dependencies.
 
 # For now, use a throwaway module download cache instead of the host machine's.
 # Usually it would be fine to reuse the host's, since we expose exact copies of
diff --git a/testdata/script/ldflags.txtar b/testdata/script/ldflags.txtar
index c41e795..6ade829 100644
--- a/testdata/script/ldflags.txtar
+++ b/testdata/script/ldflags.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=*
-
 # Note the proper domain, since the dot adds an edge case.
 #
 # Also note that there are three forms of -X allowed:
diff --git a/testdata/script/linkname.txtar b/testdata/script/linkname.txtar
index 410934c..6bb9540 100644
--- a/testdata/script/linkname.txtar
+++ b/testdata/script/linkname.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main,big.chungus/meme
-
 garble build
 exec ./main
 cmp stderr main.stderr
@@ -76,6 +74,7 @@ func main() {
 package imported
 
 import (
+	_ "strings"
 	_ "unsafe"
 )
 
diff --git a/testdata/script/literals.txtar b/testdata/script/literals.txtar
index 757692c..9ed2cb4 100644
--- a/testdata/script/literals.txtar
+++ b/testdata/script/literals.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=*
-
 garble -literals build
 exec ./main$exe
 cmp stderr main.stderr
@@ -52,7 +50,6 @@ grep '^\s+\w+ = .*\bappend\(\w+,(\s+\w+\[\d+\][\^\-+]\w+\[\d+\],?)+\)$' debug1/t
 
 # Finally, sanity check that we can build all of std with -literals.
 # Analogous to gogarble.txt.
-env GOGARBLE='*'
 garble -literals build std
 -- go.mod --
 module test/main
diff --git a/testdata/script/modinfo.txtar b/testdata/script/modinfo.txtar
index a947ad8..9a7af7f 100644
--- a/testdata/script/modinfo.txtar
+++ b/testdata/script/modinfo.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 [exec:git] exec git init -q
 [exec:git] exec git config user.name "name"
 [exec:git] exec git config user.email "name@email.local"
diff --git a/testdata/script/plugin.txtar b/testdata/script/plugin.txtar
index 0fbdc4b..f6a6675 100644
--- a/testdata/script/plugin.txtar
+++ b/testdata/script/plugin.txtar
@@ -2,8 +2,6 @@ skip # TODO: get plugins working properly. See issue #87
 
 [windows] skip 'Go plugins are not supported on Windows'
 
-env GOGARBLE=test/main
-
 garble build -buildmode=plugin ./plugin
 binsubstr plugin.so 'PublicVar' 'PublicFunc'
 ! binsubstr plugin.so 'privateFunc'
diff --git a/testdata/script/position.txtar b/testdata/script/position.txtar
index 2e4bb5f..e0701b7 100644
--- a/testdata/script/position.txtar
+++ b/testdata/script/position.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 garble build
 exec ./main
 ! stdout 'garble_main\.go|garble_other_filename|is sorted'
diff --git a/testdata/script/reflect.txtar b/testdata/script/reflect.txtar
index 4f838b5..1ebb71e 100644
--- a/testdata/script/reflect.txtar
+++ b/testdata/script/reflect.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 garble build
 exec ./main
 cmp stdout main.stdout
@@ -32,6 +30,8 @@ import (
 	"test/main/importedpkg2"
 )
 
+var Sink interface{}
+
 func main() {
 	// Fields still work fine when they are not obfuscated.
 	fmt.Println(importedpkg.ReflectInDefinedVar.ExportedField2)
@@ -41,9 +41,13 @@ func main() {
 	printfWithoutPackage("%T\n", importedpkg.ReflectTypeOf(2))
 	printfWithoutPackage("%T\n", importedpkg.ReflectTypeOfIndirect(4))
 
+
 	// More complex use of reflect.
 	v := importedpkg.ReflectValueOfVar
 	printfWithoutPackage("%#v\n", v)
+	// Keep the method from being unreachable, otherwise Call below may panic.
+	// TODO(mvdan): This only started being necessary with GOGARBLE=*. Why?
+	Sink = v.ExportedMethodName
 	method := reflect.ValueOf(&v).MethodByName("ExportedMethodName")
 	if method.IsValid() {
 		fmt.Println(method.Call(nil))
diff --git a/testdata/script/reverse.txtar b/testdata/script/reverse.txtar
index 98d9854..ca48d3b 100644
--- a/testdata/script/reverse.txtar
+++ b/testdata/script/reverse.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 # Unknown build flags should result in errors.
 ! garble reverse -badflag=foo .
 stderr 'flag provided but not defined'
diff --git a/testdata/script/seed-cache.txtar b/testdata/script/seed-cache.txtar
index 99e2300..d0597f5 100644
--- a/testdata/script/seed-cache.txtar
+++ b/testdata/script/seed-cache.txtar
@@ -7,7 +7,6 @@
 # In the past, this threw off garble's extra cached gob files.
 
 env SEED1=OQg9kACEECQ
-env GOGARBLE=*
 
 # First, ensure that mod1's garbletest.v2 is in the cache.
 cd mod1
diff --git a/testdata/script/seed.txtar b/testdata/script/seed.txtar
index 15d10a7..f77acf9 100644
--- a/testdata/script/seed.txtar
+++ b/testdata/script/seed.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 # Note that in this test we use "! bincmp" on plaintext output files,
 # as a workaround for "cmp" not supporting "! cmp".
 # TODO: now that obfuscation with -seed is deterministic,
diff --git a/testdata/script/syntax.txtar b/testdata/script/syntax.txtar
index 7cb297b..661df9f 100644
--- a/testdata/script/syntax.txtar
+++ b/testdata/script/syntax.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE='test/main,private.source'
-
 garble build
 exec ./main$exe
 cmp stderr main.stderr
diff --git a/testdata/script/tiny.txtar b/testdata/script/tiny.txtar
index 5eb3eaf..4693d27 100644
--- a/testdata/script/tiny.txtar
+++ b/testdata/script/tiny.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=test/main
-
 # Tiny mode
 garble -tiny build
 ! binsubstr main$exe 'garble_main.go' 'fmt/print.go'
diff --git a/testdata/script/typeparams.txtar b/testdata/script/typeparams.txtar
index 02dd341..42bc3cf 100644
--- a/testdata/script/typeparams.txtar
+++ b/testdata/script/typeparams.txtar
@@ -1,5 +1,3 @@
-env GOGARBLE=*
-
 garble build
 ! binsubstr main$exe ${WORK} 'garble_main.go' 'GenericFunc' 'GenericVector' 'PredeclaredSignedInteger' 'StringableSignedInteger' 'CombineEmbeds' 'GenericParam'
 -- go.mod --