Fix bug where structs would get garbled in some packages but not in others (#161)

* fix bug where structs would get garbled in some packages but not in others

* only check if struct/field was not defined in current package

* fix a related bug when two objects share the same name in the same package and one is garbled but the other one is not

* renamed parameter for clarity

main.go

@@ -28,6 +28,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strings"
 	"time"
@@ -702,7 +703,7 @@ func transformCompile(args []string) ([]string, error) {
 			// messy.
 			name = "_cgo_" + name
 		default:
-			file = transformGo(file, info, blacklist, privateNameMap, pkgPath, existingNames, &packageCounter)
+			file = transformGo(file, info, blacklist, pkg.Scope(), privateNameMap, pkgPath, existingNames, &packageCounter)
 
 			// Uncomment for some quick debugging. Do not delete.
 			// fmt.Fprintf(os.Stderr, "\n-- %s/%s --\n", pkgPath, origName)
@@ -940,14 +941,7 @@ func buildBlacklist(files []*ast.File, info *types.Info, pkg *types.Package) map
 		if obj == nil || obj.Pkg() != pkg {
 			return true
 		}
-		blacklist[obj] = struct{}{}
-
-		strct, _ := named.Underlying().(*types.Struct)
-		if strct != nil {
-			for i := 0; i < strct.NumFields(); i++ {
-				blacklist[strct.Field(i)] = struct{}{}
-			}
-		}
+		blacklistStruct(named, blacklist)
 
 		return true
 	}
@@ -1001,7 +995,7 @@ func collectNames(files []*ast.File) map[string]struct{} {
 }
 
 // transformGo garbles the provided Go syntax node.
-func transformGo(file *ast.File, info *types.Info, blacklist map[types.Object]struct{}, privateNameMap map[string]string, pkgPath string, existingNames map[string]struct{}, packageCounter *int) *ast.File {
+func transformGo(file *ast.File, info *types.Info, blacklist map[types.Object]struct{}, filePkgScope *types.Scope, privateNameMap map[string]string, pkgPath string, existingNames map[string]struct{}, packageCounter *int) *ast.File {
 	// Shuffle top level declarations
 	mathrand.Shuffle(len(file.Decls), func(i, j int) {
 		decl1 := file.Decls[i]
@@ -1057,18 +1051,70 @@ func transformGo(file *ast.File, info *types.Info, blacklist map[types.Object]st
 			return true
 		}
 
+		path := pkg.Path()
+		if !isPrivate(path) {
+			return true // only private packages are transformed
+		}
+
 		// log.Printf("%#v %T", node, obj)
+		parentScope := obj.Parent()
 		switch x := obj.(type) {
 		case *types.Var:
-			if parent := obj.Parent(); parent != nil && parent != pkg.Scope() {
+			if parentScope != nil && parentScope != pkg.Scope() {
 				// identifiers of non-global variables never show up in the binary
 				return true
 			}
+
+			// if the struct of this field was not garbled, do not garble
+			// any of that struct's fields
+			if (parentScope != filePkgScope) && (x.IsField() && !x.Embedded()) {
+				parent, ok := cursor.Parent().(*ast.SelectorExpr)
+				if !ok {
+					break
+				}
+				parentType := info.TypeOf(parent.X)
+				if parentType == nil {
+					break
+				}
+				named := namedType(parentType)
+				if named == nil {
+					break
+				}
+				if _, ok := buildInfo.imports[path]; ok {
+					garbledPkg, err := garbledImport(path)
+					if err != nil {
+						panic(err) // shouldn't happen
+					}
+					if garbledPkg.Scope().Lookup(named.Obj().Name()) != nil {
+						blacklistStruct(named, blacklist)
+						return true
+					}
+				}
+			}
 		case *types.TypeName:
-			if obj.Parent() != pkg.Scope() {
+			if parentScope != pkg.Scope() {
 				// identifiers of non-global types never show up in the binary
 				return true
 			}
+
+			// if the type was not garbled in the package were it was defined,
+			// do not garble it here
+			if parentScope != filePkgScope {
+				named := namedType(x.Type())
+				if named == nil {
+					break
+				}
+				if _, ok := buildInfo.imports[path]; ok {
+					garbledPkg, err := garbledImport(path)
+					if err != nil {
+						panic(err) // shouldn't happen
+					}
+					if garbledPkg.Scope().Lookup(x.Name()) != nil {
+						blacklistStruct(named, blacklist)
+						return true
+					}
+				}
+			}
 		case *types.Func:
 			sign := obj.Type().(*types.Signature)
 			if obj.Exported() && sign.Recv() != nil {
@@ -1088,17 +1134,15 @@ func transformGo(file *ast.File, info *types.Info, blacklist map[types.Object]st
 			return true // we only want to rename the above
 		}
 		actionID := buildInfo.actionID
-		path := pkg.Path()
-		if !isPrivate(path) {
-			return true // only private packages are transformed
-		}
 		if id := buildInfo.imports[path].actionID; len(id) > 0 {
 			garbledPkg, err := garbledImport(path)
 			if err != nil {
 				panic(err) // shouldn't happen
 			}
 			// Check if the imported name wasn't garbled, e.g. if it's assembly.
-			if garbledPkg.Scope().Lookup(obj.Name()) != nil {
+			// If the object returned from the garbled package's scope has a different type as the object
+			// we're searching for, they are most likely two separate objects with the same name, so ok to garble
+			if o := garbledPkg.Scope().Lookup(obj.Name()); o != nil && reflect.TypeOf(o) == reflect.TypeOf(obj) {
 				return true
 			}
 			actionID = id
@@ -1134,6 +1178,17 @@ func transformGo(file *ast.File, info *types.Info, blacklist map[types.Object]st
 	return astutil.Apply(file, pre, nil).(*ast.File)
 }
 
+func blacklistStruct(named *types.Named, blacklist map[types.Object]struct{}) {
+	blacklist[named.Obj()] = struct{}{}
+	strct, ok := named.Underlying().(*types.Struct)
+	if !ok {
+		return
+	}
+	for i := 0; i < strct.NumFields(); i++ {
+		blacklist[strct.Field(i)] = struct{}{}
+	}
+}
+
 // implementedOutsideGo returns whether a *types.Func does not have a body, for
 // example when it's implemented in assembly, or when one uses go:linkname.
 //

testdata/scripts/imports.txt

@@ -6,6 +6,7 @@ exec ./main
 cmp stdout main.stdout
 
 ! binsubstr main$exe 'ImportedVar' 'ImportedConst' 'ImportedFunc' 'ImportedType' 'main.go' 'test/main' 'imported.' 'NormalStruct' 'NormalExportedField' 'normalUnexportedField'
+binsubstr main$exe 'ReflectInDefined' 'ExportedField2' 'unexportedField2'
 
 [short] stop # checking that the build is reproducible is slow
 
@@ -50,7 +51,9 @@ func main() {
 	fmt.Println(imported.ImportedConst)
 	fmt.Println(imported.ImportedFunc('x'))
 	fmt.Println(imported.ImportedType(3))
-	fmt.Println(imported.NormalStruct{})
+	fmt.Println(imported.ReflectInDefinedVar.ExportedField2)
+	fmt.Println(imported.ReflectInDefined{ExportedField2: 5})
+	fmt.Println(imported.NormalStruct{SharedName: 3})
 
 	printfWithoutPackage("%T\n", imported.ReflectTypeOf(2))
 	printfWithoutPackage("%T\n", imported.ReflectTypeOfIndirect(4))
@@ -118,8 +121,20 @@ var ReflectValueOfVar = ReflectValueOf{ExportedField: "abc"}
 
 var _ = reflect.TypeOf(ReflectValueOfVar)
 
+type ReflectInDefined struct {
+	ExportedField2   int
+
+	unexportedField2 int
+}
+
+var ReflectInDefinedVar = ReflectInDefined{ExportedField2: 9000}
+
+var _ = reflect.TypeOf(ReflectInDefinedVar)
+
+const SharedName = 2
+
 type NormalStruct struct {
-	NormalExportedField   int
+	SharedName   int
 	normalUnexportedField int
 }
 
@@ -132,7 +147,9 @@ imported var value
 imported const value
 x
 3
-{0 0}
+9000
+{5 0}
+{3 0}
 ReflectTypeOf
 ReflectTypeOfIndirect
 ReflectValueOf{ExportedField:"abc", unexportedField:""}