rewrite go:linkname directives with garbled names (#200)

If code includes a linkname directive pointing at a name in an imported
package, like:

	//go:linkname localName importedpackage.RemoteName
	func localName()

We should rewrite the comment to replace "RemoteName" with its
obfuscated counterpart, if the package in question was obfuscated and
that name was as well.

We already had some code to handle linkname directives, but only to
ensure that "localName" was never obfuscated. This behavior is kept, to
ensure that the directive applies to the right name. In the future, we
could instead rewrite "localName" in the directive, like we do with
"RemoteName".

Add plenty of tests, too. The linkname directive used to be tested in
imports.txt and syntax.txt, but that was hard to maintain as each file
tested different edge cases.

Now that we have build caching, adding one extra testscript file isn't a
big problem anymoree. Add linkname.txt, which is self-explanatory. The
other two scripts also get a bit less complex.

Fixes #197.

line_obfuscator.go

@@ -39,25 +39,6 @@ func isDirective(text string, directives []string) bool {
 	return false
 }
 
-// TODO(mvdan): replace getLocalName with proper support for go:linkname
-
-func getLocalName(text string) (string, bool) {
-	if !strings.HasPrefix(text, "//go:linkname") {
-		return "", false
-	}
-	parts := strings.Fields(text)
-	if len(parts) < 2 {
-		return "", false
-	}
-
-	name := strings.TrimSpace(parts[1])
-	if len(name) == 0 {
-		return "", false
-	}
-
-	return name, true
-}
-
 func prependComment(group *ast.CommentGroup, comment *ast.Comment) *ast.CommentGroup {
 	if group == nil {
 		return &ast.CommentGroup{List: []*ast.Comment{comment}}
@@ -68,6 +49,8 @@ func prependComment(group *ast.CommentGroup, comment *ast.Comment) *ast.CommentG
 }
 
 // Remove all comments from CommentGroup except //go: directives.
+// go:linkname directives are removed, since they're collected and rewritten
+// separately.
 func clearCommentGroup(group *ast.CommentGroup) *ast.CommentGroup {
 	if group == nil {
 		return nil
@@ -75,7 +58,7 @@ func clearCommentGroup(group *ast.CommentGroup) *ast.CommentGroup {
 
 	var comments []*ast.Comment
 	for _, comment := range group.List {
-		if strings.HasPrefix(comment.Text, "//go:") {
+		if strings.HasPrefix(comment.Text, "//go:") && !strings.HasPrefix(comment.Text, "//go:linkname") {
 			comments = append(comments, &ast.Comment{Text: comment.Text})
 		}
 	}

main.go

@@ -438,20 +438,55 @@ func transformCompile(args []string) ([]string, error) {
 	detachedComments := make([][]string, len(files))
 
 	for i, file := range files {
-		for _, group := range file.Comments {
-			for _, comment := range group.List {
-				if name, _ := getLocalName(comment.Text); name != "" {
-					obj := tf.pkg.Scope().Lookup(name)
-					if obj != nil {
-						tf.blacklist[obj] = struct{}{}
-					}
-				}
+		name := filepath.Base(filepath.Clean(paths[i]))
+		cgoFile := strings.HasPrefix(name, "_cgo_")
+		comments, file := transformLineInfo(file, cgoFile)
+
+		for i, comment := range comments {
+			if !strings.HasPrefix(comment, "//go:linkname ") {
+				continue
+			}
+			fields := strings.Fields(comment)
+			if len(fields) != 3 {
+				continue
+			}
+			// This directive has two arguments: "go:linkname localName newName"
+			localName := fields[1]
+
+			// The local name must not be obfuscated.
+			obj := tf.pkg.Scope().Lookup(localName)
+			if obj != nil {
+				tf.blacklist[obj] = struct{}{}
 			}
+
+			// If the new name is of the form "pkgpath.Name", and
+			// we've obfuscated "Name" in that package, rewrite the
+			// directive to use the obfuscated name.
+			newName := strings.Split(fields[2], ".")
+			if len(newName) != 2 {
+				continue
+			}
+			pkg, name := newName[0], newName[1]
+			if pkg == "runtime" && strings.HasPrefix(name, "cgo") {
+				continue // ignore cgo-generated linknames
+			}
+			listedPkg, ok := buildInfo.imports[pkg]
+			if !ok {
+				continue // probably a made up symbol name
+			}
+			garbledPkg, _ := garbledImport(pkg)
+			if garbledPkg != nil && garbledPkg.Scope().Lookup(name) != nil {
+				continue // the name exists and was not garbled
+			}
+
+			// The name exists and was obfuscated; replace the
+			// comment with the obfuscated name.
+			obfName := hashWith(listedPkg.actionID, name)
+			fields[2] = pkg + "." + obfName
+			comments[i] = strings.Join(fields, " ")
 		}
+		detachedComments[i], files[i] = comments, file
 
-		name := filepath.Base(filepath.Clean(paths[i]))
-		cgoFile := strings.HasPrefix(name, "_cgo_")
-		detachedComments[i], files[i] = transformLineInfo(file, cgoFile)
 	}
 
 	obfSrcArchive := &bytes.Buffer{}
@@ -507,8 +542,7 @@ func transformCompile(args []string) ([]string, error) {
 		obfSrc := &bytes.Buffer{}
 		printWriter := io.MultiWriter(tempFile, obfSrc)
 
-		fileDetachedComments := detachedComments[i]
-		for _, comment := range fileDetachedComments {
+		for _, comment := range detachedComments[i] {
 			if _, err := printWriter.Write([]byte(comment + "\n")); err != nil {
 				return nil, err
 			}
@@ -896,7 +930,6 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 			// if the struct of this field was not garbled, do not garble
 			// any of that struct's fields
 			if (parentScope != tf.pkg.Scope()) && (x.IsField() && !x.Embedded()) {
-				// fmt.Println(node.Name)
 				parent, ok := cursor.Parent().(*ast.SelectorExpr)
 				if !ok {
 					break
@@ -974,7 +1007,8 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 			actionID = id
 		}
 
-		// The exported names cannot be shortened as counter synchronization between packages is not currently implemented
+		// The exported names cannot be shortened as counter synchronization
+		// between packages is not currently implemented
 		if token.IsExported(node.Name) {
 			node.Name = hashWith(actionID, node.Name)
 			return true
@@ -1108,7 +1142,9 @@ func transformLink(args []string) ([]string, error) {
 			pkgPath = buildInfo.firstImport
 		}
 		if id := buildInfo.imports[pkgPath].actionID; len(id) > 0 {
-			// If the name is not in the map file, it means that the name was not obfuscated or is public
+			// We use privateNameMap because unexported names are obfuscated
+			// to short names like "A", "B", "C" etc, which is not reproducible
+			// here. If the name isn't in the map, a hash will do.
 			newName, ok := privateNameMap[pkg+"."+name]
 			if !ok {
 				newName = hashWith(id, name)

testdata/scripts/imports.txt

@@ -62,16 +62,12 @@ import (
 	"fmt"
 	"strings"
 	"reflect"
-	_ "unsafe"
 
 	"test/main/imported"
 
 	"rsc.io/quote"
 )
 
-//go:linkname linkedPrintln fmt.Println
-func linkedPrintln(a ...interface{}) (n int, err error)
-
 func main() {
 	fmt.Println(imported.ImportedVar)
 	fmt.Println(imported.ImportedConst)
@@ -93,7 +89,6 @@ func main() {
 		fmt.Println("method not found")
 	}
 
-	linkedPrintln(nil)
 	fmt.Println(quote.Go())
 }
 
@@ -180,5 +175,4 @@ ReflectTypeOf
 ReflectTypeOfIndirect
 ReflectValueOf{ExportedField:"abc", unexportedField:""}
 [method: abc]
-<nil>
 Don't communicate by sharing memory, share memory by communicating.

testdata/scripts/linkname.txt

@@ -0,0 +1,64 @@
+env GOPRIVATE=test/main
+
+garble build
+exec ./main
+cmp stderr main.stderr
+
+! binsubstr main$exe 'garbledFunc' 'GarbledFunc'
+
+[short] stop # no need to verify this with -short
+
+go build
+exec ./main
+cmp stderr main.stderr
+
+-- go.mod --
+module test/main
+
+go 1.15
+-- main.go --
+package main
+
+import (
+	_ "strings"
+	_ "unsafe"
+
+	_ "test/main/imported"
+)
+
+// A linkname to an external non-garbled func.
+//go:linkname byteIndex strings.IndexByte
+func byteIndex(s string, c byte) int
+
+// A linkname to an external garbled func.
+//go:linkname garbledFunc test/main/imported.GarbledFuncImpl
+func garbledFunc() string
+
+// A linkname to an entirely made up name, implemented elsewhere.
+//go:linkname renamedFunc madeup.newName
+func renamedFunc() string
+
+func main() {
+	println(byteIndex("01234", '3'))
+	println(garbledFunc())
+	println(renamedFunc())
+}
+-- imported/imported.go --
+package imported
+
+import (
+	_ "unsafe"
+)
+
+func GarbledFuncImpl() string {
+	return "garbled func"
+}
+
+//go:linkname renamedFunc madeup.newName
+func renamedFunc() string {
+	return "renamed func"
+}
+-- main.stderr --
+3
+garbled func
+renamed func

testdata/scripts/syntax.txt

@@ -1,16 +1,15 @@
 env GOPRIVATE='test/main,private.source/*'
 
-garble build -tags directives
+garble build
 exec ./main$exe
 cmp stderr main.stderr
 
-! binsubstr main$exe 'localName' 'globalConst' 'globalVar' 'globalType' 'valuable information' 'private.source' 'remoteIntReturn' 'intReturn'
-binsubstr main$exe 'magicFunc'
+! binsubstr main$exe 'localName' 'globalConst' 'globalVar' 'globalType' 'valuable information' 'private.source' 'remoteIntReturn' 'intReturn' 'neverInlined'
 
 [short] stop # no need to verify this with -short
 
 # Check that the program works as expected without garble.
-go build -tags directives
+go build
 exec ./main$exe
 cmp stderr main.stderr
 
@@ -80,6 +79,13 @@ type EmbeddingUniverseScope struct {
 	string
 }
 
+// TODO: test that go:noinline still works without using debugdir
+
+//go:noinline
+func neverInlined() {
+	println("This func is never inlined.")
+}
+
 func main() {
 	switch V := V.(type) {
 	case int:
@@ -94,7 +100,7 @@ func main() {
 	scopesTest()
 	println(extra.Func())
 	sub.Test()
-	sub.TestDirectives()
+	neverInlined()
 }
 
 -- scopes.go --
@@ -143,43 +149,10 @@ func Test() {
 }
 
 func noop(...interface{}) {}
-
--- sub/directives.go --
-// +build directives
-
-package sub
-
-import (
-	_ "unsafe"
-	_ "test/main/sub/a"
-)
-
-//go:linkname remoteIntReturn a.magicFunc
-
-func remoteIntReturn() int
-
-//go:noinline
-func TestDirectives() {
-	if remoteIntReturn() != 42 {
-		panic("invalid result")
-	}
-}
--- sub/a/directives.go --
-// +build directives
-
-package a
-
-import _ "unsafe"
-
-//go:linkname intReturn a.magicFunc
-
-//go:noinline
-func intReturn() int {
-	return 42
-}
 -- main.stderr --
 nil case
 {"Foo":3}
 1 1 1
 1 4 5 1 input
 This is a separate module to obfuscate.
+This func is never inlined.