Skip to content

Commit

Permalink
support reverse on packages using cgo
Browse files Browse the repository at this point in the history
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 burrowers#555.
  • Loading branch information
mvdan committed Sep 23, 2022
1 parent fc91758 commit 4a37d41
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 12 deletions.
13 changes: 9 additions & 4 deletions reverse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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) {

Expand Down
10 changes: 5 additions & 5 deletions shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down
52 changes: 49 additions & 3 deletions testdata/script/cgo.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -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()

Expand All @@ -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();
Expand All @@ -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

0 comments on commit 4a37d41

Please sign in to comment.