Browse Source

slight simplifications and alloc reductions

Reuse a buffer and a map across loop iterations, because we can.

Make recordTypeDone only track named types, as that is enough to detect
type cycles. Without named types, there can be no cycles.

These two reduce allocs by a fraction of a percent:

	name      old time/op         new time/op         delta
	Build-16          10.4s ± 2%          10.4s ± 1%    ~     (p=0.739 n=10+10)

	name      old bin-B           new bin-B           delta
	Build-16          5.51M ± 0%          5.51M ± 0%    ~     (all equal)

	name      old cached-time/op  new cached-time/op  delta
	Build-16          391ms ± 9%          407ms ± 7%    ~     (p=0.095 n=10+9)

	name      old mallocs/op      new mallocs/op      delta
	Build-16          34.5M ± 0%          34.4M ± 0%  -0.12%  (p=0.000 n=10+10)

	name      old sys-time/op     new sys-time/op     delta
	Build-16          5.87s ± 5%          5.82s ± 5%    ~     (p=0.182 n=10+9)

It doesn't seem like much, but remember that these stats are for the
entire set of processes, where garble only accounts for about 10% of the
total wall time when compared to the compiler or linker. So a ~0.1%
decrease globally is still significant.

linkerVariableStrings is also indexed by *types.Var rather than types.Object,
since -ldflags=-X only supports setting the string value of variables.
This shouldn't make a significant difference in terms of allocs,
but at least the map is less prone to confusion with other object types.
To ensure the new code doesn't trip up on non-variables, we add test cases.

Finally, for the sake of clarity, index into the types.Info maps like
Defs and Uses rather than calling ObjectOf if we know whether the
identifier we have is a definition of a name or the use of a defined name.
This isn't better in terms of performance, as ObjectOf is a tiny method,
but just like with linkerVariableStrings before, the new code is clearer.
Daniel Martí 1 month ago
parent
commit
21bd89ff73
3 changed files with 30 additions and 20 deletions
  1. 2 2
      internal/literals/literals.go
  2. 21 17
      main.go
  3. 7 1
      testdata/scripts/ldflags.txt

+ 2 - 2
internal/literals/literals.go

@@ -34,7 +34,7 @@ func randObfuscator() obfuscator {
 }
 
 // Obfuscate replaces literals with obfuscated anonymous functions.
-func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, linkStrings map[types.Object]string) *ast.File {
+func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, linkStrings map[*types.Var]string) *ast.File {
 	pre := func(cursor *astutil.Cursor) bool {
 		switch node := cursor.Node().(type) {
 		case *ast.GenDecl:
@@ -44,7 +44,7 @@ func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, linkString
 			}
 		case *ast.ValueSpec:
 			for _, name := range node.Names {
-				obj := info.ObjectOf(name)
+				obj := info.Defs[name].(*types.Var)
 				if _, e := linkStrings[obj]; e {
 					// Skip this entire ValueSpec to not break -ldflags=-X.
 					// TODO: support obfuscating those injected strings, too.

+ 21 - 17
main.go

@@ -34,6 +34,7 @@ import (
 	"unicode"
 	"unicode/utf8"
 
+	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
 	"golang.org/x/mod/modfile"
 	"golang.org/x/mod/module"
@@ -612,6 +613,7 @@ func transformAsm(args []string) ([]string, error) {
 	const middleDot = '·'
 	middleDotLen := utf8.RuneLen(middleDot)
 
+	var buf bytes.Buffer
 	for _, path := range paths {
 		// Read the entire file into memory.
 		// If we find issues with large files, we can use bufio.
@@ -619,10 +621,10 @@ func transformAsm(args []string) ([]string, error) {
 		if err != nil {
 			return nil, err
 		}
+		buf.Reset()
 
 		// Find all middle-dot names, and replace them.
 		remaining := content
-		var buf bytes.Buffer
 		for {
 			i := bytes.IndexRune(remaining, middleDot)
 			if i < 0 {
@@ -1110,12 +1112,13 @@ func loadCachedOutputs() error {
 }
 
 func (tf *transformer) findReflectFunctions(files []*ast.File) {
+	seenReflectParams := make(map[*types.Var]bool)
 	visitFuncDecl := func(funcDecl *ast.FuncDecl) {
-		funcObj := tf.info.ObjectOf(funcDecl.Name).(*types.Func)
+		funcObj := tf.info.Defs[funcDecl.Name].(*types.Func)
 		funcType := funcObj.Type().(*types.Signature)
 		funcParams := funcType.Params()
 
-		seenReflectParams := make(map[*types.Var]bool)
+		maps.Clear(seenReflectParams)
 		for i := 0; i < funcParams.Len(); i++ {
 			seenReflectParams[funcParams.At(i)] = false
 		}
@@ -1129,7 +1132,7 @@ func (tf *transformer) findReflectFunctions(files []*ast.File) {
 			if !ok {
 				return true
 			}
-			calledFunc, _ := tf.info.ObjectOf(sel.Sel).(*types.Func)
+			calledFunc, _ := tf.info.Uses[sel.Sel].(*types.Func)
 			if calledFunc == nil || calledFunc.Pkg() == nil {
 				return true
 			}
@@ -1150,7 +1153,7 @@ func (tf *transformer) findReflectFunctions(files []*ast.File) {
 					if !ok {
 						continue
 					}
-					obj, _ := tf.info.ObjectOf(ident).(*types.Var)
+					obj, _ := tf.info.Uses[ident].(*types.Var)
 					if obj == nil {
 						continue
 					}
@@ -1203,7 +1206,7 @@ func (tf *transformer) findReflectFunctions(files []*ast.File) {
 // Since we obfuscate one package at a time, we only detect those if the type
 // definition and the reflect usage are both in the same package.
 func (tf *transformer) prefillObjectMaps(files []*ast.File) error {
-	tf.linkerVariableStrings = make(map[types.Object]string)
+	tf.linkerVariableStrings = make(map[*types.Var]string)
 
 	// TODO: this is a linker flag that affects how we obfuscate a package at
 	// compile time. Note that, if the user changes ldflags, then Go may only
@@ -1238,9 +1241,9 @@ func (tf *transformer) prefillObjectMaps(files []*ast.File) error {
 			return // not the current package
 		}
 
-		obj := tf.pkg.Scope().Lookup(name)
+		obj, _ := tf.pkg.Scope().Lookup(name).(*types.Var)
 		if obj == nil {
-			return // not found; skip
+			return // no such variable; skip
 		}
 		tf.linkerVariableStrings[obj] = stringValue
 	})
@@ -1261,7 +1264,7 @@ func (tf *transformer) prefillObjectMaps(files []*ast.File) error {
 			ident = sel.Sel
 		}
 
-		fnType, _ := tf.info.ObjectOf(ident).(*types.Func)
+		fnType, _ := tf.info.Uses[ident].(*types.Func)
 		if fnType == nil || fnType.Pkg() == nil {
 			return true
 		}
@@ -1297,10 +1300,11 @@ type transformer struct {
 	// linkerVariableStrings is also initialized by prefillObjectMaps.
 	// It records objects for variables used in -ldflags=-X flags,
 	// as well as the strings the user wants to inject them with.
-	linkerVariableStrings map[types.Object]string
+	linkerVariableStrings map[*types.Var]string
 
-	// recordTypeDone helps avoid cycles in recordType.
-	recordTypeDone map[types.Type]bool
+	// recordTypeDone helps avoid type cycles in recordType.
+	// We only need to track named types, as all cycles must use them.
+	recordTypeDone map[*types.Named]bool
 
 	// fieldToStruct helps locate struct types from any of their field
 	// objects. Useful when obfuscating field names.
@@ -1315,7 +1319,7 @@ func newTransformer() *transformer {
 			Defs:  make(map[*ast.Ident]types.Object),
 			Uses:  make(map[*ast.Ident]types.Object),
 		},
-		recordTypeDone: make(map[types.Type]bool),
+		recordTypeDone: make(map[*types.Named]bool),
 		fieldToStruct:  make(map[*types.Var]*types.Struct),
 	}
 }
@@ -1370,19 +1374,19 @@ func (tf *transformer) typecheck(files []*ast.File) error {
 // Right now, all it does is fill the fieldToStruct field.
 // Since types can be recursive, we need a map to avoid cycles.
 func (tf *transformer) recordType(used, origin types.Type) {
-	if tf.recordTypeDone[used] {
-		return
-	}
 	if origin == nil {
 		origin = used
 	}
 	type Container interface{ Elem() types.Type }
-	tf.recordTypeDone[used] = true
 	switch used := used.(type) {
 	case Container:
 		origin := origin.(Container)
 		tf.recordType(used.Elem(), origin.Elem())
 	case *types.Named:
+		if tf.recordTypeDone[used] {
+			return
+		}
+		tf.recordTypeDone[used] = true
 		// If we have a generic struct like
 		//
 		//	type Foo[T any] struct { Bar T }

+ 7 - 1
testdata/scripts/ldflags.txt

@@ -7,7 +7,7 @@ env GOGARBLE=*
 #   -X=name=value
 #   -X name=value
 #   -X "name=value" (or with single quotes, allows spaces in value)
-env LDFLAGS='-X=main.unexportedVersion=v1.22.33 -X=main.replacedWithEmpty= -X "main.replacedWithSpaces= foo bar " -X=domain.test/main/imported.ExportedUnset=garble_replaced -X=domain.test/missing/path.missingVar=value'
+env LDFLAGS='-X=main.unexportedVersion=v1.22.33 -X=main.replacedWithEmpty= -X "main.replacedWithSpaces= foo bar " -X=domain.test/main/imported.ExportedUnset=garble_replaced -X=domain.test/missing/path.missingVar=value -X=main.someType=notAVariable'
 
 garble build -ldflags=${LDFLAGS}
 exec ./main
@@ -47,6 +47,12 @@ var notReplacedBefore, replacedWithEmpty, notReplacedAfter = "kept_before", "ori
 
 var replacedWithSpaces = "original"
 
+type someType int
+
+const someConst = "untouchable"
+
+func someFunc() string { return "untouchable" }
+
 func main() {
 	fmt.Printf("version: %q\n", unexportedVersion)
 	fmt.Printf("becomes empty: %q\n", replacedWithEmpty)