From 3fbefbbacff03ff3124686fbb26c1c5cb2fba2da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Wed, 15 Jun 2022 19:45:48 +0100
Subject: [PATCH] fix TODOs about code which is now unused

The _gomod_.go file inserted by the Go toolchain no longer shows up;
it's likely that either the -trimpath or -buildvcs=false flags are
preventing that extra bit of work from happening entirely.
The modinfo.txt test ensures that we're not breaking,
and the inner lines of code weren't hit as part of `go test`.

It also appears that we don't need to avoid obfuscating functions
defined with an `//export` directive. This is likely because cgo runs as
a pre-process step compared to the compiler, so us removing the
directive later does not make a difference.
We might need to revisit this in the future if we implement obfuscating
Go code instead of builds, e.g. `garble export`.

Just in case, I've expanded the cgo.txt test to also include one more
kind of cgo integration: an "import C" block including a C header file.

Either of these changes are slightly risky, as our tests don't cover all
edge cases. We've just done a release, so now is the time to try them.
---
 main.go                  | 34 +++++-----------------------------
 testdata/scripts/cgo.txt | 14 ++++++++++++++
 2 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/main.go b/main.go
index 6b11b0c..b55b414 100644
--- a/main.go
+++ b/main.go
@@ -715,15 +715,6 @@ func transformCompile(args []string) ([]string, error) {
 	// generating it.
 	flags = append(flags, "-dwarf=false")
 
-	for i, path := range paths {
-		if filepath.Base(path) == "_gomod_.go" {
-			// never include module info
-			// TODO: this seems to no longer trigger for our tests?
-			paths = append(paths[:i], paths[i+1:]...)
-			break
-		}
-	}
-
 	var files []*ast.File
 	for _, path := range paths {
 		file, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution|parser.ParseComments)
@@ -991,6 +982,7 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
 			// See exporttest/*.go in testdata/scripts/test.txt.
 			// For now, spot the pattern and avoid the unnecessary error;
 			// the dependency is unused, so the packagefile line is redundant.
+			// This still triggers as of go1.19beta1.
 			if strings.HasSuffix(curPkg.ImportPath, ".test]") && strings.HasPrefix(curPkg.ImportPath, impPath) {
 				continue
 			}
@@ -1290,21 +1282,6 @@ func (tf *transformer) prefillObjectMaps(files []*ast.File) error {
 		return true
 	}
 	for _, file := range files {
-		for _, group := range file.Comments {
-			for _, comment := range group.List {
-				name := strings.TrimPrefix(comment.Text, "//export ")
-				if name == comment.Text {
-					continue
-				}
-				name = strings.TrimSpace(name)
-				obj := tf.pkg.Scope().Lookup(name)
-				if obj == nil {
-					continue // not found; skip
-				}
-				// TODO(mvdan): it seems like removing this doesn't break any tests.
-				recordAsNotObfuscated(obj)
-			}
-		}
 		ast.Inspect(file, visit)
 	}
 	return nil
@@ -1464,11 +1441,10 @@ func recordedObjectString(obj types.Object) objectString {
 // recordAsNotObfuscated records all the objects whose names we cannot obfuscate.
 // An object is any named entity, such as a declared variable or type.
 //
-// So far, it records:
-//
-//   - Types which are used for reflection.
-//   - Declarations exported via "//export".
-//   - Types or variables from external packages which were not obfuscated.
+// As of June 2022, this only records types which are used in reflection.
+// TODO(mvdan): If this is still the case in a year's time,
+// we should probably rename "not obfuscated" and "cannot obfuscate" to be
+// directly about reflection, e.g. "used in reflection".
 func recordAsNotObfuscated(obj types.Object) {
 	if obj.Pkg().Path() != curPkg.ImportPath {
 		panic("called recordedAsNotObfuscated with a foreign object")
diff --git a/testdata/scripts/cgo.txt b/testdata/scripts/cgo.txt
index 9cf290e..3eed3b6 100644
--- a/testdata/scripts/cgo.txt
+++ b/testdata/scripts/cgo.txt
@@ -3,6 +3,7 @@
 env GOGARBLE=test/main
 
 garble build
+! stderr 'warning' # check that the C toolchain is happy
 exec ./main
 cmp stdout main.stdout
 ! binsubstr main$exe 'PortedField'
@@ -20,6 +21,7 @@ exec ./main
 cmp stdout main.stdout
 
 go build
+! stderr 'warning' # check that the C toolchain is happy
 exec ./main
 cmp stdout main.stdout
 binsubstr main$exe 'privateAdd'
@@ -34,6 +36,8 @@ package main
 import "os/user"
 
 /*
+#include "separate.h"
+
 static int privateAdd(int a, int b) {
 	return a + b;
 }
@@ -42,6 +46,7 @@ extern void goCallback();
 
 static void callGoCallback() {
 	goCallback();
+	separateFunction();
 }
 
 struct portedStruct {
@@ -65,7 +70,16 @@ func main() {
 func goCallback() {
 	fmt.Println("go callback")
 }
+-- separate.h --
+void separateFunction();
+-- separate.c --
+#include "_cgo_export.h"
+
+void separateFunction() {
+	goCallback();
+}
 -- main.stdout --
 3
 true
 go callback
+go callback