From 6b1a062c6f2cfb165dc924c7b43db9fe1054b917 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 5 Apr 2021 22:15:33 +0100
Subject: [PATCH] make -literals succeed on all of std

Two bugs were remaining which made the build with -literals of std fail.

First, we were ignoring too many objects in constant expressions,
including type names. This resulted in type names declared in
dependencies which were incorrectly not obfuscated in the current
package:

	# go/constant
	O1ku7TCe.go:1: undefined: alzLJ5Fd.Word
	b0ieEGVQ.go:1: undefined: alzLJ5Fd.Word
	LEpgYKdb.go:4: undefined: alzLJ5Fd.Word
	FkhHJCfm.go:1: undefined: alzLJ5Fd.Word

This edge case is easy to reproduce, so a test case is added to
literals.txt.

The second issue is trickier; in some packages like os/user, we would
get syntax errors because of comments printed out of place:

	../tip/os/user/getgrouplist_unix.go:35:130: syntax error: unexpected newline, expecting comma or )

This is a similar kind of error that we tried to fix with e2f06cce94. In
particular, it's fixed by also setting CallExpr.Rparen in withPos. We
also add many other missing Pos fields for good measure, even though
we're not sure they help just yet.

Unfortunately, all my attempts to minimize this into a reproducible
failure have failed. We can't just copy the failing file from os/user,
as it only builds on some OSs. It seems like it was the perfect mix of
cgo (which adds line directive comments) plus unlucky positioning of
literals.

For that last reason, as well as for ensuring that -literals works well
with a wide variety of software, we add a build of all of std with
-literals when not testing with -short. This is akin to what we do in
goprivate.txt, but with the -literals flag. This does make "go test"
more expensive, but also more thorough.

Fixes #285, hopefully for good this time.
---
 internal/literals/literals.go | 22 +++++++++++++++++-----
 testdata/scripts/literals.txt | 24 ++++++++++++++++++------
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/internal/literals/literals.go b/internal/literals/literals.go
index 07dee56..335a75a 100644
--- a/internal/literals/literals.go
+++ b/internal/literals/literals.go
@@ -191,14 +191,21 @@ func withPos(node ast.Node, pos token.Pos) ast.Node {
 			node.ValuePos = pos
 		case *ast.Ident:
 			node.NamePos = pos
-		case *ast.StarExpr:
-			node.Star = pos
 		case *ast.CompositeLit:
 			node.Lbrace = pos
+			node.Rbrace = pos
 		case *ast.ArrayType:
 			node.Lbrack = pos
 		case *ast.FuncType:
 			node.Func = pos
+		case *ast.BinaryExpr:
+			node.OpPos = pos
+		case *ast.StarExpr:
+			node.Star = pos
+		case *ast.CallExpr:
+			node.Lparen = pos
+			node.Rparen = pos
+
 		case *ast.GenDecl:
 			node.TokPos = pos
 		case *ast.ReturnStmt:
@@ -207,8 +214,8 @@ func withPos(node ast.Node, pos token.Pos) ast.Node {
 			node.For = pos
 		case *ast.RangeStmt:
 			node.For = pos
-		case *ast.CallExpr:
-			node.Lparen = pos
+		case *ast.BranchStmt:
+			node.TokPos = pos
 		}
 		return true
 	})
@@ -278,8 +285,13 @@ func RecordUsedAsConstants(node ast.Node, info *types.Info, ignoreObj map[types.
 			return true
 		}
 
+		// Only record *types.Const objects.
+		// Other objects, such as builtins or type names,
+		// must not be recorded as they would be false positives.
 		obj := info.ObjectOf(ident)
-		ignoreObj[obj] = true
+		if _, ok := obj.(*types.Const); ok {
+			ignoreObj[obj] = true
+		}
 
 		return true
 	}
diff --git a/testdata/scripts/literals.txt b/testdata/scripts/literals.txt
index d097b24..8054e1d 100644
--- a/testdata/scripts/literals.txt
+++ b/testdata/scripts/literals.txt
@@ -27,15 +27,11 @@ binsubstr main$exe 'Lorem' 'dolor' 'second assign' 'First Line' 'Second Line' 'm
 # seconds, it means we're trying to obfuscate them.
 generate-literals extra_literals.go
 
-# Also check that the binary is different from previous builds.
-rm main$exe
-garble -literals -debugdir=debug1 -seed=8J+Ri/Cfh6fwn4e+ build
-! bincmp main$exe main_old$exe
-
+garble -literals -debugdir=debug1 build
 exec ./main$exe
 cmp stderr main.stderr
 
-# Check obfuscators
+# Check obfuscators.
 
 # Xor obfuscator. Detect a[i] = a[i] (^|-|+) b[i]
 grep '^\s+\w+\[\w+\] = \w+\[\w+\] [\^\-+] \w+$' debug1/test/main/extra_literals.go
@@ -53,6 +49,11 @@ grep '^\s+\w+ = .*\bappend\(\w+,(\s+\w+\[\d+\][\^\-+]\w+\[\d+\],?)+\)$' debug1/t
 # XorSeed obfuscator. Detect type decFunc func(byte) decFunc
 grep '^\s+type \w+ func\(byte\) \w+$' debug1/test/main/extra_literals.go
 
+# Finally, sanity check that we can build all of std with -literals.
+# Analogous to goprivate.txt.
+env GOPRIVATE='*'
+garble -literals build std
+
 -- go.mod --
 module test/main
 
@@ -60,6 +61,8 @@ go 1.16
 -- main.go --
 package main
 
+import "test/main/imported"
+
 type strucTest struct {
 	field        string
 	anotherfield string
@@ -203,6 +206,11 @@ func constantTest() {
 	const f = "foo" // skip
 	const i = length + len(f)
 	println(length, i)
+
+	// We should still obfuscate ImportedType here.
+	// Otherwise, the build will fail,
+	// as the name was obfuscated in the original package.
+	const impType = imported.ImportedType(3)
 }
 
 func byteTest() {
@@ -242,6 +250,10 @@ const (
 	iota0 uint8 = iota
 	iota1
 )
+-- imported/imported.go --
+package imported
+
+type ImportedType int
 -- directives.go --
 // If we misplace any of the directives below,
 // cmd/compile will complain with "misplaced compiler directive".