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.
pull/551/head
Daniel Martí 2 years ago committed by lu4p
parent 99f9b88363
commit 3fbefbbacf

@ -715,15 +715,6 @@ func transformCompile(args []string) ([]string, error) {
// generating it. // generating it.
flags = append(flags, "-dwarf=false") 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 var files []*ast.File
for _, path := range paths { for _, path := range paths {
file, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution|parser.ParseComments) 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. // See exporttest/*.go in testdata/scripts/test.txt.
// For now, spot the pattern and avoid the unnecessary error; // For now, spot the pattern and avoid the unnecessary error;
// the dependency is unused, so the packagefile line is redundant. // 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) { if strings.HasSuffix(curPkg.ImportPath, ".test]") && strings.HasPrefix(curPkg.ImportPath, impPath) {
continue continue
} }
@ -1290,21 +1282,6 @@ func (tf *transformer) prefillObjectMaps(files []*ast.File) error {
return true return true
} }
for _, file := range files { 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) ast.Inspect(file, visit)
} }
return nil return nil
@ -1464,11 +1441,10 @@ func recordedObjectString(obj types.Object) objectString {
// recordAsNotObfuscated records all the objects whose names we cannot obfuscate. // recordAsNotObfuscated records all the objects whose names we cannot obfuscate.
// An object is any named entity, such as a declared variable or type. // An object is any named entity, such as a declared variable or type.
// //
// So far, it records: // 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,
// - Types which are used for reflection. // we should probably rename "not obfuscated" and "cannot obfuscate" to be
// - Declarations exported via "//export". // directly about reflection, e.g. "used in reflection".
// - Types or variables from external packages which were not obfuscated.
func recordAsNotObfuscated(obj types.Object) { func recordAsNotObfuscated(obj types.Object) {
if obj.Pkg().Path() != curPkg.ImportPath { if obj.Pkg().Path() != curPkg.ImportPath {
panic("called recordedAsNotObfuscated with a foreign object") panic("called recordedAsNotObfuscated with a foreign object")

@ -3,6 +3,7 @@
env GOGARBLE=test/main env GOGARBLE=test/main
garble build garble build
! stderr 'warning' # check that the C toolchain is happy
exec ./main exec ./main
cmp stdout main.stdout cmp stdout main.stdout
! binsubstr main$exe 'PortedField' ! binsubstr main$exe 'PortedField'
@ -20,6 +21,7 @@ exec ./main
cmp stdout main.stdout cmp stdout main.stdout
go build go build
! stderr 'warning' # check that the C toolchain is happy
exec ./main exec ./main
cmp stdout main.stdout cmp stdout main.stdout
binsubstr main$exe 'privateAdd' binsubstr main$exe 'privateAdd'
@ -34,6 +36,8 @@ package main
import "os/user" import "os/user"
/* /*
#include "separate.h"
static int privateAdd(int a, int b) { static int privateAdd(int a, int b) {
return a + b; return a + b;
} }
@ -42,6 +46,7 @@ extern void goCallback();
static void callGoCallback() { static void callGoCallback() {
goCallback(); goCallback();
separateFunction();
} }
struct portedStruct { struct portedStruct {
@ -65,7 +70,16 @@ func main() {
func goCallback() { func goCallback() {
fmt.Println("go callback") fmt.Println("go callback")
} }
-- separate.h --
void separateFunction();
-- separate.c --
#include "_cgo_export.h"
void separateFunction() {
goCallback();
}
-- main.stdout -- -- main.stdout --
3 3
true true
go callback go callback
go callback

Loading…
Cancel
Save