From 56a1fd0257864c54b80ff760c46f3463b6fd2f9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 25 May 2020 21:41:23 +0100
Subject: [PATCH 1/3] support -ldflags=-X=pkg.name=str with garbled names

Because the linker has access to all the build IDs, just like the
compiler, we can support this transparently. Add a test too.

Fixes #21.
---
 main.go                      | 64 ++++++++++++++++++++++++++++++++----
 testdata/scripts/ldflags.txt | 36 ++++++++++++++++++++
 2 files changed, 94 insertions(+), 6 deletions(-)
 create mode 100644 testdata/scripts/ldflags.txt

diff --git a/main.go b/main.go
index 53ae290..1d2ce03 100644
--- a/main.go
+++ b/main.go
@@ -112,8 +112,10 @@ func garbledImport(path string) (*types.Package, error) {
 }
 
 type packageInfo struct {
-	buildID string
-	imports map[string]importedPkg
+	buildID string // from -buildid
+
+	imports     map[string]importedPkg // from -importcfg
+	firstImport string                 // first from -importcfg; the main package when linking
 }
 
 type importedPkg struct {
@@ -310,6 +312,7 @@ func transformCompile(args []string) ([]string, error) {
 	deferred = append(deferred, func() error {
 		return os.RemoveAll(tempDir)
 	})
+
 	// Add our temporary dir to the beginning of -trimpath, so that we don't
 	// leak temporary dirs. Needs to be at the beginning, since there may be
 	// shorter prefixes later in the list, such as $PWD if TMPDIR=$PWD/tmp.
@@ -354,6 +357,8 @@ func transformCompile(args []string) ([]string, error) {
 func isPrivate(pkgPath string) bool {
 	if pkgPath == "main" {
 		// TODO: why don't we see the full package path for main packages?
+		// Hint: it seems like the real import path is at the top of
+		// -importcfg.
 		return true
 	}
 	return GlobsMatchPath(envGoPrivate, pkgPath)
@@ -396,6 +401,9 @@ func readBuildIDs(flags []string) error {
 		if err != nil {
 			return err
 		}
+		if len(buildInfo.imports) == 0 {
+			buildInfo.firstImport = importPath
+		}
 		buildInfo.imports[importPath] = importedPkg{
 			packagefile: objectPath,
 			buildID:     fileID,
@@ -587,6 +595,39 @@ func transformLink(args []string) ([]string, error) {
 		// Nothing to transform; probably just ["-V=full"].
 		return args, nil
 	}
+
+	// Make sure -X works with garbled identifiers. To cover both garbled
+	// and non-garbled names, duplicate each flag with a garbled version.
+	if err := readBuildIDs(flags); err != nil {
+		return nil, err
+	}
+	flagValueIter(flags, "-X", func(val string) {
+		// val is in the form of "pkg.name=str"
+		i := strings.IndexByte(val, '=')
+		if i <= 0 {
+			return
+		}
+		name := val[:i]
+		str := val[i+1:]
+		j := strings.IndexByte(name, '.')
+		if j <= 0 {
+			return
+		}
+		pkg := name[:j]
+		name = name[j+1:]
+
+		pkgPath := pkg
+		if pkgPath == "main" {
+			// The main package is known under its import path in
+			// the import config map.
+			pkgPath = buildInfo.firstImport
+		}
+		if id := buildInfo.imports[pkgPath].buildID; id != "" {
+			name = hashWith(id, name)
+			flags = append(flags, fmt.Sprintf("-X=%s.%s=%s", pkg, name, str))
+		}
+	})
+
 	flags = append(flags, "-w", "-s")
 	return append(flags, paths...), nil
 }
@@ -605,21 +646,32 @@ func splitFlagsFromFiles(args []string, ext string) (flags, paths []string) {
 }
 
 // flagValue retrieves the value of a flag such as "-foo", from strings in the
-// list of arguments like "-foo=bar" or "-foo" "bar".
+// list of arguments like "-foo=bar" or "-foo" "bar". If the flag is repeated,
+// the last value is returned.
 func flagValue(flags []string, name string) string {
+	lastVal := ""
+	flagValueIter(flags, name, func(val string) {
+		lastVal = val
+	})
+	return lastVal
+}
+
+// flagValueIter retrieves all the values for a flag such as "-foo", like
+// flagValue. The difference is that it allows handling complex flags, such as
+// those whose values compose a list.
+func flagValueIter(flags []string, name string, fn func(string)) {
 	for i, arg := range flags {
 		if val := strings.TrimPrefix(arg, name+"="); val != arg {
 			// -name=value
-			return val
+			fn(val)
 		}
 		if arg == name { // -name ...
 			if i+1 < len(flags) {
 				// -name value
-				return flags[i+1]
+				fn(flags[i+1])
 			}
 		}
 	}
-	return ""
 }
 
 func flagSetValue(flags []string, name, value string) []string {
diff --git a/testdata/scripts/ldflags.txt b/testdata/scripts/ldflags.txt
new file mode 100644
index 0000000..370ee2e
--- /dev/null
+++ b/testdata/scripts/ldflags.txt
@@ -0,0 +1,36 @@
+garble build -ldflags='-X=main.unexportedVersion=v1.0.0 -X=test/main/imported.ExportedVar=replaced'
+exec ./main
+cmp stdout main.stdout
+! binsubstr main$exe 'unexportedVersion'
+
+[short] stop # no need to verify this with -short
+
+exec go build -ldflags='-X=main.unexportedVersion=v1.0.0 -X=test/main/imported.ExportedVar=replaced'
+exec ./main
+cmp stdout main.stdout
+binsubstr main$exe 'unexportedVersion'
+
+-- go.mod --
+module test/main
+-- main.go --
+package main
+
+import (
+	"fmt"
+
+	"test/main/imported"
+)
+
+var unexportedVersion = "unknown"
+
+func main() {
+	fmt.Println("version:", unexportedVersion)
+	fmt.Println("var:", imported.ExportedVar)
+}
+-- imported/imported.go --
+package imported
+
+var ExportedVar = "original"
+-- main.stdout --
+version: v1.0.0
+var: replaced

From e4b58b14523cc20c2a9eba3c974bd50a0c0067a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 25 May 2020 22:07:41 +0100
Subject: [PATCH 2/3] reduce unnecessary std imports in tests

Since we have to recompile all dependencies, this reduces a substantial
amount of work, reducing 'go test -short' time from ~16s to ~12s on my
laptop.
---
 testdata/scripts/asm.txt     | 12 +++++-------
 testdata/scripts/cgo.txt     | 10 ++++------
 testdata/scripts/ldflags.txt | 12 +++++-------
 testdata/scripts/modinfo.txt | 23 ++++++++++++-----------
 testdata/scripts/plugin.txt  | 10 ++++------
 5 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/testdata/scripts/asm.txt b/testdata/scripts/asm.txt
index 65ee650..05b842b 100644
--- a/testdata/scripts/asm.txt
+++ b/testdata/scripts/asm.txt
@@ -1,13 +1,13 @@
 garble build
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 binsubstr main$exe 'privateAdd' 'PublicAdd'
 
 [short] stop # no need to verify this with -short
 
 go build
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 
 -- go.mod --
 module test/main
@@ -15,16 +15,14 @@ module test/main
 package main
 
 import (
-	"fmt"
-
 	"test/main/imported"
 )
 
 func privateAdd(x, y int64) int64
 
 func main() {
-	fmt.Println(privateAdd(1, 2))
-	fmt.Println(imported.PublicAdd(3, 4))
+	println(privateAdd(1, 2))
+	println(imported.PublicAdd(3, 4))
 }
 -- main.s --
 TEXT ·privateAdd(SB),$0-24
@@ -44,6 +42,6 @@ TEXT ·PublicAdd(SB),$0-24
 	ADDQ BP, BX
 	MOVQ BX, ret+16(FP)
 	RET
--- main.stdout --
+-- main.stderr --
 3
 7
diff --git a/testdata/scripts/cgo.txt b/testdata/scripts/cgo.txt
index 2d41ea2..370a410 100644
--- a/testdata/scripts/cgo.txt
+++ b/testdata/scripts/cgo.txt
@@ -1,13 +1,13 @@
 garble build
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 binsubstr main$exe 'privateAdd'
 
 [short] stop # no need to verify this with -short
 
 go build
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 
 -- go.mod --
 module test/main
@@ -21,10 +21,8 @@ static int privateAdd(int a, int b) {
 */
 import "C"
 
-import "fmt"
-
 func main() {
-	fmt.Println(C.privateAdd(C.int(1), C.int(2)))
+	println(C.privateAdd(C.int(1), C.int(2)))
 }
--- main.stdout --
+-- main.stderr --
 3
diff --git a/testdata/scripts/ldflags.txt b/testdata/scripts/ldflags.txt
index 370ee2e..170b3be 100644
--- a/testdata/scripts/ldflags.txt
+++ b/testdata/scripts/ldflags.txt
@@ -1,13 +1,13 @@
 garble build -ldflags='-X=main.unexportedVersion=v1.0.0 -X=test/main/imported.ExportedVar=replaced'
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 ! binsubstr main$exe 'unexportedVersion'
 
 [short] stop # no need to verify this with -short
 
 exec go build -ldflags='-X=main.unexportedVersion=v1.0.0 -X=test/main/imported.ExportedVar=replaced'
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 binsubstr main$exe 'unexportedVersion'
 
 -- go.mod --
@@ -16,21 +16,19 @@ module test/main
 package main
 
 import (
-	"fmt"
-
 	"test/main/imported"
 )
 
 var unexportedVersion = "unknown"
 
 func main() {
-	fmt.Println("version:", unexportedVersion)
-	fmt.Println("var:", imported.ExportedVar)
+	println("version:", unexportedVersion)
+	println("var:", imported.ExportedVar)
 }
 -- imported/imported.go --
 package imported
 
 var ExportedVar = "original"
--- main.stdout --
+-- main.stderr --
 version: v1.0.0
 var: replaced
diff --git a/testdata/scripts/modinfo.txt b/testdata/scripts/modinfo.txt
index f99e963..91d58d4 100644
--- a/testdata/scripts/modinfo.txt
+++ b/testdata/scripts/modinfo.txt
@@ -1,13 +1,13 @@
 garble build
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 ! binsubstr main$exe '(devel)'
 
 [short] stop # no need to verify this with -short
 
 exec go build
 exec ./main
-cmp stdout main.stdout-orig
+cmp stderr main.stderr-orig
 binsubstr main$exe '(devel)'
 
 -- go.mod --
@@ -15,15 +15,16 @@ module test/main
 -- main.go --
 package main
 
-import (
-	"fmt"
-	"runtime/debug"
-)
+import "runtime/debug"
 
 func main() {
-	fmt.Println(debug.ReadBuildInfo())
+	if info, ok := debug.ReadBuildInfo(); ok {
+		println("version", info.Main.Version)
+	} else {
+		println("no version")
+	}
 }
--- main.stdout-orig --
-&{test/main {test/main (devel)  <nil>} []} true
--- main.stdout --
-<nil> false
+-- main.stderr-orig --
+version (devel)
+-- main.stderr --
+no version
diff --git a/testdata/scripts/plugin.txt b/testdata/scripts/plugin.txt
index 9d31966..4bf782d 100644
--- a/testdata/scripts/plugin.txt
+++ b/testdata/scripts/plugin.txt
@@ -7,7 +7,7 @@ binsubstr plugin.so 'PublicVar' 'PublicFunc'
 # Note that we need -trimpath; see the caveat section in the README.
 go build -trimpath
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 binsubstr main$exe 'PublicVar' 'PublicFunc'
 ! binsubstr plugin.so 'privateFunc'
 
@@ -17,18 +17,16 @@ go build -buildmode=plugin ./plugin
 binsubstr plugin.so 'PublicVar' 'PublicFunc' 'privateFunc'
 go build
 exec ./main
-cmp stdout main.stdout
+cmp stderr main.stderr
 
 -- go.mod --
 module test/main
 -- plugin/main.go --
 package main
 
-import "fmt"
-
 var PublicVar int
 
-func privateFunc(n int) { fmt.Printf("Hello, number %d\n", n) }
+func privateFunc(n int) { println("Hello, number", n) }
 
 func PublicFunc() { privateFunc(PublicVar) }
 -- main.go --
@@ -52,5 +50,5 @@ func main() {
 	*v.(*int) = 7
 	f.(func())()
 }
--- main.stdout --
+-- main.stderr --
 Hello, number 7

From e8074d46652afc869678aa71c1b999198245e1b0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 25 May 2020 22:27:09 +0100
Subject: [PATCH 3/3] support building ad-hoc plugin packages

That is, plugin packages by source file names, not by package path.

Fixes #19.
---
 main.go                     |  8 ++++----
 testdata/scripts/plugin.txt | 12 +++++++++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/main.go b/main.go
index 1d2ce03..5e9f950 100644
--- a/main.go
+++ b/main.go
@@ -355,10 +355,10 @@ func transformCompile(args []string) ([]string, error) {
 // To allow using garble without GOPRIVATE for standalone main packages, it will
 // default to not matching standard library packages.
 func isPrivate(pkgPath string) bool {
-	if pkgPath == "main" {
-		// TODO: why don't we see the full package path for main packages?
-		// Hint: it seems like the real import path is at the top of
-		// -importcfg.
+	if pkgPath == "main" || strings.HasPrefix(pkgPath, "plugin/unnamed") {
+		// TODO: why don't we see the full package path for main
+		// packages? The linker has it at the top of -importcfg, but not
+		// the compiler.
 		return true
 	}
 	return GlobsMatchPath(envGoPrivate, pkgPath)
diff --git a/testdata/scripts/plugin.txt b/testdata/scripts/plugin.txt
index 4bf782d..9271f48 100644
--- a/testdata/scripts/plugin.txt
+++ b/testdata/scripts/plugin.txt
@@ -13,6 +13,10 @@ binsubstr main$exe 'PublicVar' 'PublicFunc'
 
 [short] stop # no need to verify this with -short
 
+# This used to fail, since in this case the package path for the ad-hoc plugin
+# package isn't "main", but "plugin/unnamed-*".
+garble build -buildmode=plugin plugin/main.go
+
 go build -buildmode=plugin ./plugin
 binsubstr plugin.so 'PublicVar' 'PublicFunc' 'privateFunc'
 go build
@@ -24,11 +28,17 @@ module test/main
 -- plugin/main.go --
 package main
 
-var PublicVar int
+import "test/main/plugin/lib"
+
+var PublicVar int = lib.ImportedFunc()
 
 func privateFunc(n int) { println("Hello, number", n) }
 
 func PublicFunc() { privateFunc(PublicVar) }
+-- plugin/lib/lib.go --
+package lib
+
+func ImportedFunc() int { return 4 }
 -- main.go --
 package main