ensure that all imports are used after obfuscation

garble's -literals flag and its patching of the runtime may leave unused imports.
We used to try to detect those and remove the imports,
but that was still buggy with edge cases like dot imports or renamed imports.

Moreover, it was potentially incorrect.
Completely removing an import from a package means we don't run its init funcs,
which could have side effects changing the behavior of a program.
As an example, database/sql drivers are registered at init time.

Instead, for each import in an obfuscated Go file,
add an unnamed declaration which references the imported package.
This may not be necessary for all imported packages,
as only a minority become unused due to garble,
but it's also relatively harmless to do so.

Fixes #658.

main.go

@@ -40,7 +40,6 @@ import (
 	"golang.org/x/mod/module"
 	"golang.org/x/mod/semver"
 	"golang.org/x/tools/go/ast/astutil"
-
 	"mvdan.cc/garble/internal/linker"
 	"mvdan.cc/garble/internal/literals"
 )
@@ -951,7 +950,7 @@ func transformCompile(args []string) ([]string, error) {
 			if flagTiny {
 				// strip unneeded runtime code
 				stripRuntime(basename, file)
-				tf.removeUnnecessaryImports(file)
+				tf.makeImportsUsed(file)
 			}
 			if basename == "symtab.go" {
 				updateMagicValue(file, magicValue())
@@ -1514,9 +1513,10 @@ type transformer struct {
 func newTransformer() *transformer {
 	return &transformer{
 		info: &types.Info{
-			Types: make(map[ast.Expr]types.TypeAndValue),
-			Defs:  make(map[*ast.Ident]types.Object),
-			Uses:  make(map[*ast.Ident]types.Object),
+			Types:     make(map[ast.Expr]types.TypeAndValue),
+			Defs:      make(map[*ast.Ident]types.Object),
+			Uses:      make(map[*ast.Ident]types.Object),
+			Implicits: make(map[ast.Node]types.Object),
 		},
 		recordTypeDone: make(map[*types.Named]bool),
 		fieldToStruct:  make(map[*types.Var]*types.Struct),
@@ -1687,48 +1687,127 @@ func recordedAsNotObfuscated(obj types.Object) bool {
 	return ok
 }
 
-func (tf *transformer) removeUnnecessaryImports(file *ast.File) {
-	usedImports := make(map[string]bool)
-	ast.Inspect(file, func(n ast.Node) bool {
-		node, ok := n.(*ast.Ident)
-		if !ok {
-			return true
-		}
+func (tf *transformer) resolveImportSpec(imp *ast.ImportSpec) *types.Package {
+	// Simple import has no ast.Ident and is stored in Implicits separately.
+	obj := tf.info.Implicits[imp]
+	if obj == nil {
+		obj = tf.info.Defs[imp.Name] // renamed or dot import
+	}
+	if obj == nil {
+		return nil
+	}
 
-		uses, ok := tf.info.Uses[node]
-		if !ok {
-			return true
-		}
+	pkgObj, ok := obj.(*types.PkgName)
+	if !ok {
+		panic(fmt.Sprintf("unexpected object type for %s: %v", imp.Path.Value, obj))
+	}
+	return pkgObj.Imported()
+}
 
-		if pkg := uses.Pkg(); pkg != nil {
-			usedImports[pkg.Path()] = true
+// isSafeForInstanceType returns true if the passed type is safe for var declaration.
+// Unsafe types: generic types and non-method interfaces.
+func isSafeForInstanceType(typ types.Type) bool {
+	switch t := typ.(type) {
+	case *types.Named:
+		if t.TypeParams().Len() > 0 {
+			return false
 		}
+		return isSafeForInstanceType(t.Underlying())
+	case *types.Signature:
+		return t.TypeParams().Len() == 0
+	case *types.Interface:
+		return t.IsMethodSet()
+	}
+	return true
+}
 
-		return true
-	})
-
+func (tf *transformer) makeImportsUsed(file *ast.File) {
 	for _, imp := range file.Imports {
 		if imp.Name != nil && imp.Name.Name == "_" {
 			continue
 		}
 
-		path, err := strconv.Unquote(imp.Path.Value)
-		if err != nil {
-			panic(err)
+		pkg := tf.resolveImportSpec(imp)
+		if pkg == nil {
+			panic(fmt.Sprintf("import %s not found", imp.Path.Value))
 		}
 
-		// The import path can't be used directly here, because the actual
-		// path resolved via go/types might be different from the naive path.
-		lpkg, err := listPackage(path)
-		if err != nil {
-			panic(err)
-		}
+		generated := false
+		scope := pkg.Scope()
+		for _, name := range scope.Names() {
+			if !token.IsExported(name) {
+				continue
+			}
+			obj := scope.Lookup(name)
+			if obj == nil {
+				panic(fmt.Sprintf("%s not found in %s", name, imp.Path.Value))
+			}
+			if !isSafeForInstanceType(obj.Type()) {
+				continue
+			}
 
-		if usedImports[lpkg.ImportPath] {
-			continue
+			nameIdent := ast.NewIdent(name)
+			getFullName := func() ast.Expr {
+				if imp.Name == nil {
+					return &ast.SelectorExpr{
+						X:   ast.NewIdent(pkg.Name()),
+						Sel: nameIdent,
+					}
+				}
+				if imp.Name.Name == "." {
+					return nameIdent
+				}
+				return &ast.SelectorExpr{
+					X:   ast.NewIdent(imp.Name.Name),
+					Sel: nameIdent,
+				}
+			}
+
+			var decl *ast.GenDecl
+			switch obj.(type) {
+			case *types.Const:
+				// const _ = <value>
+				decl = &ast.GenDecl{
+					Tok: token.CONST,
+					Specs: []ast.Spec{&ast.ValueSpec{
+						Names:  []*ast.Ident{ast.NewIdent("_")},
+						Values: []ast.Expr{getFullName()},
+					}},
+				}
+			case *types.Var, *types.Func:
+				// var _ = <value>
+				decl = &ast.GenDecl{
+					Tok: token.VAR,
+					Specs: []ast.Spec{&ast.ValueSpec{
+						Names:  []*ast.Ident{ast.NewIdent("_")},
+						Values: []ast.Expr{getFullName()},
+					}},
+				}
+			case *types.TypeName:
+				// var _ <type>
+				decl = &ast.GenDecl{
+					Tok: token.VAR,
+					Specs: []ast.Spec{&ast.ValueSpec{
+						Names: []*ast.Ident{ast.NewIdent("_")},
+						Type:  getFullName(),
+					}},
+				}
+			default:
+				continue // Skip *types.Builtin and other
+			}
+
+			// Manually add a new variable for correct name obfuscation
+			tf.info.Uses[nameIdent] = obj
+			file.Decls = append(file.Decls, decl)
+			generated = true
+			break
 		}
 
-		imp.Name = ast.NewIdent("_")
+		if !generated {
+			// A very unlikely situation where there is no suitable declaration for a reference variable
+			// and almost certainly means that there is another import reference in code.
+			log.Printf("generate reference variable for %s failed", imp.Path.Value)
+		}
 	}
 }
 
@@ -1744,7 +1823,7 @@ func (tf *transformer) transformGoFile(file *ast.File) *ast.File {
 		file = literals.Obfuscate(obfRand, file, tf.info, tf.linkerVariableStrings)
 
 		// some imported constants might not be needed anymore, remove unnecessary imports
-		tf.removeUnnecessaryImports(file)
+		tf.makeImportsUsed(file)
 	}
 
 	pre := func(cursor *astutil.Cursor) bool {

testdata/script/literals.txtar

@@ -66,6 +66,7 @@ import (
 	. "test/main/imp_func"
 	. "test/main/imp_var"
 	. "test/main/imp_type"
+	. "test/main/imp_struct"
 )
 
 type structTest struct {
@@ -145,6 +146,7 @@ func main() {
 	byteTest()
 	shadowTest()
 	dotImportTest()
+	multipleTimeImportTest()
 
 	strArray := [2]string{"1: literal in", "an secret array"}
 	println(strArray[0], strArray[1])
@@ -330,7 +332,56 @@ func dotImportTest() {
     println(DotImportedFunc())
     println(DotImportedVar)
     println(DotImportedType("str as dot imported type"))
+    println(DotImportedStruct.Str)
 }
+
+func multipleTimeImportTest() {
+    regularAndUnusedRenamed()
+    regularAndUnusedDotImport()
+    unusedRegularAndDotImport()
+}
+
+func noop(...any) {}
+
+-- regular_and_unused_renamed.go --
+package main
+
+import (
+    "test/main/imp_mult"
+    imp_mult2 "test/main/imp_mult"
+)
+
+func regularAndUnusedRenamed() {
+    imp_mult.MultDummy()
+    noop(imp_mult.MultImpStr, imp_mult2.MultImpStr)
+}
+
+-- regular_and_unused_dotimport.go --
+package main
+
+import (
+    "test/main/imp_mult"
+    . "test/main/imp_mult"
+)
+
+func regularAndUnusedDotImport() {
+    imp_mult.MultDummy()
+    noop(imp_mult.MultImpStr, MultImpStr)
+}
+
+-- unused_regular_and_dotimport.go --
+package main
+
+import (
+    "test/main/imp_mult"
+    . "test/main/imp_mult"
+)
+
+func unusedRegularAndDotImport() {
+    MultDummy()
+    noop(imp_mult.MultImpStr, MultImpStr)
+}
+
 -- imp/imported.go --
 package imported
 
@@ -357,6 +408,18 @@ package imp_type
 
 type DotImportedType string
 
+-- imp_struct/imported.go --
+package imp_struct
+
+var DotImportedStruct = struct{ Str string }{Str: "string in dot imported struct"}
+
+-- imp_mult/imported.go --
+package imp_mult
+
+const MultImpStr = "str from package imported multiple time"
+
+func MultDummy() {}
+
 -- directives.go --
 // If we misplace any of the directives below,
 // cmd/compile will complain with "misplaced compiler directive".
@@ -445,5 +508,6 @@ const str from dot imported var
 str from dot imported func
 str from dot imported var
 str as dot imported type
+string in dot imported struct
 1: literal in an secret array
 2: literal in a secret slice