record types into ignoreObjects more reliably

Our previous logic only took care of fairly simple types, such as a
simple struct or a pointer to a struct. If we had a struct embedding
another struct, we'd fail to record the objects for the fields in the
inner struct, and that would lead to miscompilation:

	> garble build
	[stderr]
	# test/main
	LZmt64Nm.go:7: outer.InnerField undefined (type *CcUt1wkQ.EmbeddingOuter has no field or method InnerField)

To fix this issue, make the function that records all objects under a
types.Type smarter. Since it now does more than just dealing with
structs, it's also renamed.

Since the function now walks types properly, we get to remove the extra
ast.Inspect in recordReflectArgs, which is nice.

We also make it a method, to avoid the map parameter. A boolean
parameter is also added, since we need this feature to only look at the
current package when looking at reflect calls.

Finally, we add a test case, a simplified version of the original bug
report.

Fixes #315.

main.go

@@ -950,22 +950,6 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
 func (tf *transformer) recordReflectArgs(files []*ast.File) {
 	tf.ignoreObjects = make(map[types.Object]bool)
 
-	visitReflectArg := func(node ast.Node) bool {
-		expr, _ := node.(ast.Expr) // info.TypeOf(nil) will just return nil
-		named := namedType(tf.info.TypeOf(expr))
-		if named == nil {
-			return true
-		}
-
-		obj := named.Obj()
-		if obj == nil || obj.Pkg() != tf.pkg {
-			return true
-		}
-		recordStruct(named, tf.ignoreObjects)
-
-		return true
-	}
-
 	visit := func(node ast.Node) bool {
 		if opts.GarbleLiterals {
 			literals.RecordUsedAsConstants(node, tf.info, tf.ignoreObjects)
@@ -987,7 +971,7 @@ func (tf *transformer) recordReflectArgs(files []*ast.File) {
 
 		if fnType.Pkg().Path() == "reflect" && (fnType.Name() == "TypeOf" || fnType.Name() == "ValueOf") {
 			for _, arg := range call.Args {
-				ast.Inspect(arg, visitReflectArg)
+				tf.recordIgnore(tf.info.TypeOf(arg), false)
 			}
 		}
 		return true
@@ -1103,7 +1087,7 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 				// TODO(mvdan): add a test and think how to fix this
 				if obfPkg := obfuscatedTypesPackage(path); obfPkg != nil {
 					if obfPkg.Scope().Lookup(named.Obj().Name()) != nil {
-						recordStruct(named, tf.ignoreObjects)
+						tf.recordIgnore(named, true)
 						return true
 					}
 				}
@@ -1124,7 +1108,7 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 				// The type is directly referenced by name,
 				// so obfuscatedTypesPackage can't return nil.
 				if obfuscatedTypesPackage(path).Scope().Lookup(obj.Name()) != nil {
-					recordStruct(named, tf.ignoreObjects)
+					tf.recordIgnore(named, true)
 					return true
 				}
 			}
@@ -1194,17 +1178,40 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 	return astutil.Apply(file, pre, post).(*ast.File)
 }
 
-// recordStruct adds the given named type to the map, plus all of its fields if
+// recordIgnore adds any named types (including fields) under typ to
-// it is a struct. This function is mainly used for types used via reflection,
+// ignoreObjects.
-// so we want to record their members too.
+//
-func recordStruct(named *types.Named, m map[types.Object]bool) {
+// When allPkgs is false, we stop if we encounter a named type defined in a
-	m[named.Obj()] = true
+// dependency package. This is useful to only record uses of reflection on local
-	strct, ok := named.Underlying().(*types.Struct)
+// types.
-	if !ok {
+func (tf *transformer) recordIgnore(t types.Type, allPkgs bool) {
-		return
+	switch t := t.(type) {
-	}
+	case *types.Named:
-	for i := 0; i < strct.NumFields(); i++ {
+		obj := t.Obj()
-		m[strct.Field(i)] = true
+		if !allPkgs && obj.Pkg() != tf.pkg {
+			return // not from the current package
+		}
+		if tf.ignoreObjects[obj] {
+			return // prevent endless recursion
+		}
+		tf.ignoreObjects[obj] = true
+
+		// Record the underlying type, too.
+		tf.recordIgnore(t.Underlying(), allPkgs)
+
+	case *types.Struct:
+		for i := 0; i < t.NumFields(); i++ {
+			field := t.Field(i)
+
+			// Record the field itself, too.
+			tf.ignoreObjects[field] = true
+
+			tf.recordIgnore(field.Type(), allPkgs)
+		}
+
+	case interface{ Elem() types.Type }:
+		// Get past pointers, slices, etc.
+		tf.recordIgnore(t.Elem(), allPkgs)
 	}
 }
 

testdata/scripts/syntax.txt

@@ -86,6 +86,14 @@ func neverInlined() {
 	println("This func is never inlined.")
 }
 
+type EmbeddingOuter struct {
+	EmbeddingInner
+}
+
+type EmbeddingInner struct {
+	SomeField int
+}
+
 func main() {
 	switch V := V.(type) {
 	case int:
@@ -101,8 +109,23 @@ func main() {
 	println(extra.Func())
 	sub.Test()
 	neverInlined()
+
+	// A harder case of detecting reflection.
+	// The type is defined in a dependency,
+	// and the types involved are more complex.
+	outer := &sub.EmbeddingOuter{}
+	outer.InnerField = 3
+	enc, _ = json.Marshal(outer)
+	println(string(enc))
+}
+
+type RecursiveStruct struct {
+	*RecursiveStruct
+	list []RecursiveStruct
 }
 
+var _ = reflect.TypeOf(RecursiveStruct{})
+
 -- scopes.go --
 package main
 
@@ -136,6 +159,8 @@ func input(localNameParam string) (localNameReturn string) { return localNamePar
 -- sub/names.go --
 package sub
 
+import "reflect"
+
 var someGlobalVar0 = "0"
 var someGlobalVar1 = "1"
 var someGlobalVar2 = "2"
@@ -156,6 +181,19 @@ func TestFoo(s string) {}
 
 func TestBar(*struct{}) {}
 
+var _ = reflect.TypeOf([]*struct{EmbeddingOuter}{})
+
+type EmbeddingOuter struct {
+	EmbeddingInner
+	Anon struct {
+		AnonField int
+	}
+}
+
+type EmbeddingInner struct {
+	InnerField int
+}
+
 -- main.stderr --
 nil case
 {"Foo":3}
@@ -163,3 +201,4 @@ nil case
 1 4 5 1 input
 This is a separate module to obfuscate.
 This func is never inlined.
+{"InnerField":3,"Anon":{"AnonField":0}}