From 2ee6604408d6570983137e1295a33bc4bba295f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 1 Mar 2021 11:34:06 +0000
Subject: [PATCH] replace the -p path when assembling (#247)

The asm tool runs twice for a package with assembly. The second time it
does, the path given to the -p flag matters, just like in the compiler,
as we generate an object file.

We don't have a -buildid flag in the asm tool, so obtaining the action
ID to obfuscate the package path with is a bit tricky. We store it from
transformCompile, and read it from transformAsm. See the detailed docs
for more.

This was the last "skip" line in the tests due to Go 1.16. After all PRs
are merged, one last PR documenting that 1.16 is supported will be sent,
closing the issue for good.

It's unclear why this wasn't an issue in Go 1.15. My best guess is that
the ABI changes only happened in Go 1.16, and this causes exported asm
funcs to start showing up in object files with their package paths.

Updates #124.
---
 main.go                  | 53 ++++++++++++++++++++++++++++++++++++++++
 testdata/scripts/asm.txt |  2 --
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/main.go b/main.go
index 53555e0..a31d3f3 100644
--- a/main.go
+++ b/main.go
@@ -388,10 +388,46 @@ func toolexecCmd(command string, args []string) (*exec.Cmd, error) {
 }
 
 var transformFuncs = map[string]func([]string) (args []string, _ error){
+	"asm":     transformAsm,
 	"compile": transformCompile,
 	"link":    transformLink,
 }
 
+func transformAsm(args []string) ([]string, error) {
+	flags, paths := splitFlagsFromFiles(args, ".s")
+
+	symAbis := false
+	// Note that flagValue only supports "-foo=true" bool flags, but the std
+	// flag is generally just "-std".
+	// TODO: Better support boolean flags for the tools.
+	for _, flag := range flags {
+		if flag == "-gensymabis" {
+			symAbis = true
+		}
+	}
+	curPkgPath = flagValue(flags, "-p")
+
+	// If we are generating symbol ABIs, the output does not actually
+	// contain curPkgPath. Exported APIs show up as "".FooBar.
+	// Otherwise, we are assembling, and curPkgPath does make its way into
+	// the output object file.
+	// To obfuscate the path in the -p flag, we need the current action ID,
+	// which we recover from the file that transformCompile wrote for us.
+	if !symAbis && curPkgPath != "main" && isPrivate(curPkgPath) {
+		savedActionID := filepath.Join(sharedTempDir, strings.ReplaceAll(curPkgPath, "/", ","))
+		var err error
+		curActionID, err = ioutil.ReadFile(savedActionID)
+		if err != nil {
+			return nil, fmt.Errorf("could not read build ID: %v", err)
+		}
+
+		newPkgPath := hashWith(curActionID, curPkgPath)
+		flags = flagSetValue(flags, "-p", newPkgPath)
+	}
+
+	return append(flags, paths...), nil
+}
+
 func transformCompile(args []string) ([]string, error) {
 	var err error
 	flags, paths := splitFlagsFromFiles(args, ".go")
@@ -433,6 +469,22 @@ func transformCompile(args []string) ([]string, error) {
 		return nil, err
 	}
 
+	// Tools which run after the main compile run, such as asm, also need
+	// the current action ID to obfuscate the package path in their -p flag.
+	// They lack the -buildid flag, so store it in a unique file here to be
+	// recovered by the other tools later, such as transformAsm.
+	// Import paths include slashes, which usually cannot be in filenames,
+	// so replace those with commas, which should be fine and cannot be part
+	// of an import path.
+	// We only write each file once, as we compile each package once.
+	// Each filename is also unique, since import paths are unique.
+	// TODO: perhaps error if the file already exists, to double check that
+	// the assumptions above are correct.
+	savedActionID := filepath.Join(sharedTempDir, strings.ReplaceAll(curPkgPath, "/", ","))
+	if err := ioutil.WriteFile(savedActionID, curActionID, 0o666); err != nil {
+		return nil, fmt.Errorf("could not store action ID: %v", err)
+	}
+
 	var files []*ast.File
 	for _, path := range paths {
 		file, err := parser.ParseFile(fset, path, nil, parser.ParseComments)
@@ -830,6 +882,7 @@ func fillBuildInfo(flags []string) (newImportCfg string, _ error) {
 	case "", "true":
 		return "", fmt.Errorf("could not find -buildid argument")
 	}
+
 	curActionID = decodeHash(splitActionID(buildID))
 	curImportCfg = flagValue(flags, "-importcfg")
 	if curImportCfg == "" {
diff --git a/testdata/scripts/asm.txt b/testdata/scripts/asm.txt
index 94ac048..47b5b9e 100644
--- a/testdata/scripts/asm.txt
+++ b/testdata/scripts/asm.txt
@@ -1,5 +1,3 @@
-[go1.16] skip 'TODO(mvdan): fix for Go 1.16'
-
 env GOPRIVATE=test/main
 
 garble build