From e8e06f6ad6bceed1d38de40c75458fd50c6b21e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Fri, 23 Sep 2022 15:14:06 +0100
Subject: [PATCH] support reverse on packages using cgo

The reverse feature relied on `GoFiles` from `go list`,
but that list may not be enough to typecheck a package:

	typecheck error: $WORK/main.go:3:15: undeclared name: longMain

`go help list` shows:

	GoFiles         []string   // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
	CgoFiles        []string   // .go source files that import "C"
	CompiledGoFiles []string   // .go files presented to compiler (when using -compiled)

In other words, to mimic the same list of Go files fed to the compiler,
we want CompiledGoFiles.

Note that, since the cgo files show up as generated files,
we currently do not support reversing their filenames.
That is left as a TODO for now.

Updates #555.
---
 reverse.go                | 13 +++++++---
 shared.go                 | 10 ++++----
 testdata/script/cgo.txtar | 52 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/reverse.go b/reverse.go
index 51c64c9..ff334af 100644
--- a/reverse.go
+++ b/reverse.go
@@ -80,9 +80,14 @@ One can reverse a captured panic stack trace as follows:
 		addHashedWithPackage(lpkg.ImportPath)
 
 		var files []*ast.File
-		for _, goFile := range lpkg.GoFiles {
-			fullGoFile := filepath.Join(lpkg.Dir, goFile)
-			file, err := parser.ParseFile(fset, fullGoFile, nil, parser.SkipObjectResolution)
+		for _, goFile := range lpkg.CompiledGoFiles {
+			// Direct Go files may be relative paths like "foo.go".
+			// Compiled Go files, such as those generated from cgo,
+			// may be absolute paths inside the Go build cache.
+			if !filepath.IsAbs(goFile) {
+				goFile = filepath.Join(lpkg.Dir, goFile)
+			}
+			file, err := parser.ParseFile(fset, goFile, nil, parser.SkipObjectResolution)
 			if err != nil {
 				return err
 			}
@@ -93,7 +98,7 @@ One can reverse a captured panic stack trace as follows:
 			return err
 		}
 		for i, file := range files {
-			goFile := lpkg.GoFiles[i]
+			goFile := lpkg.CompiledGoFiles[i]
 			ast.Inspect(file, func(node ast.Node) bool {
 				switch node := node.(type) {
 
diff --git a/shared.go b/shared.go
index b2ae248..06fc56d 100644
--- a/shared.go
+++ b/shared.go
@@ -140,10 +140,10 @@ type listedPackage struct {
 	ImportMap  map[string]string
 	Standard   bool
 
-	Dir        string
-	GoFiles    []string
-	Imports    []string
-	Incomplete bool
+	Dir             string
+	CompiledGoFiles []string
+	Imports         []string
+	Incomplete      bool
 
 	// The fields below are not part of 'go list', but are still reused
 	// between garble processes. Use "Garble" as a prefix to ensure no
@@ -180,7 +180,7 @@ func appendListedPackages(packages []string, withDeps bool) error {
 	// TODO: perhaps include all top-level build flags set by garble,
 	// including -buildvcs=false.
 	// They shouldn't affect "go list" here, but might as well be consistent.
-	args := []string{"list", "-json", "-export", "-trimpath", "-e"}
+	args := []string{"list", "-json", "-export", "-compiled", "-trimpath", "-e"}
 	if withDeps {
 		args = append(args, "-deps")
 	}
diff --git a/testdata/script/cgo.txtar b/testdata/script/cgo.txtar
index c1e74e7..0f48b23 100644
--- a/testdata/script/cgo.txtar
+++ b/testdata/script/cgo.txtar
@@ -10,6 +10,15 @@ cmp stdout main.stdout
 
 [short] stop # no need to verify this with -short
 
+# Ensure that reversing works with cgo.
+env GARBLE_TEST_REVERSING=true
+exec ./main
+cp stdout reversing.stdout
+stdin reversing.stdout
+garble reverse .
+cmp stdout reversed.stdout
+env GARBLE_TEST_REVERSING=false
+
 env GOGARBLE=*
 garble build
 exec ./main
@@ -33,7 +42,34 @@ go 1.18
 -- main.go --
 package main
 
-import "os/user"
+func main() {
+	regularFunc()
+	cgoFunc()
+}
+-- regular_main.go --
+package main
+
+import (
+	"fmt"
+	"os"
+	"runtime"
+)
+
+func regularFunc() {
+	if os.Getenv("GARBLE_TEST_REVERSING") == "true" {
+		_, filename, _, _ := runtime.Caller(0)
+		fmt.Println("regular filename:", filename)
+	}
+}
+-- cgo_main.go --
+package main
+
+import (
+	"fmt"
+	"os"
+	"os/user"
+	"runtime"
+)
 
 /*
 #include "separate.h"
@@ -54,9 +90,8 @@ struct portedStruct {
 };
 */
 import "C"
-import "fmt"
 
-func main() {
+func cgoFunc() {
 	fmt.Println(C.privateAdd(C.int(1), C.int(2)))
 	_, _ = user.Current()
 
@@ -69,6 +104,11 @@ func main() {
 //export goCallback
 func goCallback() {
 	fmt.Println("go callback")
+	// TODO: support reversing filenames in cgo files
+	if false && os.Getenv("GARBLE_TEST_REVERSING") == "true" {
+		_, filename, _, _ := runtime.Caller(0)
+		fmt.Println("cgo filename:", filename)
+	}
 }
 -- separate.h --
 void separateFunction();
@@ -83,3 +123,9 @@ void separateFunction() {
 true
 go callback
 go callback
+-- reversed.stdout --
+regular filename: test/main/regular_main.go
+3
+true
+go callback
+go callback