set positions when using cursor.Replace

The regular obfuscation process simply modifies some simple nodes, such
as identifiers and strings. In those cases, we modify the nodes
in-place, meaning that their positions remain the same. This hasn't
caused any problems.

Literal obfuscation is trickier. Since we replace one expression with an
entirely different one, we use cursor.Replace. The new expression is
entirely made up on the spot, so it lacks position information.

This was causing problems. For example, in the added test input:

	> garble -literals build
	[stderr]
	# test/main
	dgcm4t6w.go:3: misplaced compiler directive
	dgcm4t6w.go:4: misplaced compiler directive
	dgcm4t6w.go:3: misplaced compiler directive
	dgcm4t6w.go:6: misplaced compiler directive
	dgcm4t6w.go:7: misplaced compiler directive
	dgcm4t6w.go:3: misplaced compiler directive
	dgcm4t6w.go:9: misplaced compiler directive
	dgcm4t6w.go:3: misplaced compiler directive
	dgcm4t6w.go:3: too many errors

The build errors are because we'd move the compiler directives, which
makes the compiler unhappy as they must be directly followed by a
function declaration.

The root cause there seems to be that, since the replacement nodes lack
position information, go/printer would try to estimate its printing
position by adding to the last known position. Since -literals adds
code, this would result in the printer position increasing rapidly, and
potentially printing directive comments earlier than needed.

For now, making the replacement nodes have the same position as the
original node seems to stop go/printer from making this mistake.

It's possible that this workaround won't be bulletproof forever, but it
works well for now, and I don't see a simpler workaround right now.
It would be possible to use fancier mechanisms like go/ast.CommentMap or
dave/dst, but those are a significant amount of added complexity as well.

Fixes #285.

internal/literals/literals.go

@@ -113,7 +113,7 @@ func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, ignoreObj
 
 					data[i] = byte(value)
 				}
-				cursor.Replace(obfuscateByteArray(data, y.Len()))
+				cursor.Replace(withPos(obfuscateByteArray(data, y.Len()), x.Pos()))
 
 			case *types.Slice:
 				if y.Elem() != byteType {
@@ -138,7 +138,7 @@ func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, ignoreObj
 
 					data = append(data, byte(value))
 				}
-				cursor.Replace(obfuscateByteSlice(data))
+				cursor.Replace(withPos(obfuscateByteSlice(data), x.Pos()))
 
 			}
 
@@ -168,7 +168,7 @@ func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, ignoreObj
 				return true
 			}
 
-			cursor.Replace(obfuscateString(value))
+			cursor.Replace(withPos(obfuscateString(value), x.Pos()))
 		}
 
 		return true
@@ -177,6 +177,44 @@ func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, ignoreObj
 	return astutil.Apply(file, pre, post).(*ast.File)
 }
 
+// withPos sets any token.Pos fields under node which affect printing to pos.
+// Note that we can't set all token.Pos fields, since some affect the semantics.
+//
+// This function is useful so that go/printer doesn't try to estimate position
+// offsets, which can end up in printing comment directives too early.
+//
+// We don't set any "end" or middle positions, because they seem irrelevant.
+func withPos(node ast.Node, pos token.Pos) ast.Node {
+	ast.Inspect(node, func(node ast.Node) bool {
+		switch node := node.(type) {
+		case *ast.BasicLit:
+			node.ValuePos = pos
+		case *ast.Ident:
+			node.NamePos = pos
+		case *ast.StarExpr:
+			node.Star = pos
+		case *ast.CompositeLit:
+			node.Lbrace = pos
+		case *ast.ArrayType:
+			node.Lbrack = pos
+		case *ast.FuncType:
+			node.Func = pos
+		case *ast.GenDecl:
+			node.TokPos = pos
+		case *ast.ReturnStmt:
+			node.Return = pos
+		case *ast.ForStmt:
+			node.For = pos
+		case *ast.RangeStmt:
+			node.For = pos
+		case *ast.CallExpr:
+			node.Lparen = pos
+		}
+		return true
+	})
+	return node
+}
+
 func obfuscateString(data string) *ast.CallExpr {
 	obfuscator := randObfuscator()
 	block := obfuscator.obfuscate([]byte(data))

position.go

@@ -51,7 +51,7 @@ func printFile(file1 *ast.File) ([]byte, error) {
 	// The only way we can get the final ones is to parse again.
 	file2, err := parser.ParseFile(fset, absFilename, src, parser.ParseComments)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("re-parse error: %w", err)
 	}
 
 	// Keep the compiler directives, and change position info.

testdata/scripts/literals.txt

@@ -242,6 +242,55 @@ const (
 	iota0 uint8 = iota
 	iota1
 )
+-- directives.go --
+// If we misplace any of the directives below,
+// cmd/compile will complain with "misplaced compiler directive".
+//
+// We use many literals and functions, mixing different types,
+// so that it's more likely that bugs will be caught.
+package main
+
+//go:noinline
+func str0() { println("foo") }
+
+//go:noinline
+func str1() { println("foo") }
+
+//go:noinline
+func str2() { println("foo") }
+
+//go:noinline
+func str3() { println("foo") }
+
+//go:noinline
+func str4() { println("foo") }
+
+//go:noinline
+func arr0() { println(len([...]byte{0, 1, 2})) }
+
+//go:noinline
+func arr1() { println(len([...]byte{0, 1, 2})) }
+
+//go:noinline
+func slc0() { println([]byte{0, 1, 2}) }
+
+//go:noinline
+func slc1() { println([]byte{0, 1, 2}) }
+
+//go:noinline
+func str5() { println("foo") }
+
+//go:noinline
+func str6() { println("foo") }
+
+//go:noinline
+func str7() { println("foo") }
+
+//go:noinline
+func str8() { println("foo") }
+
+//go:noinline
+func str9() { println("foo") }
 
 -- main.stderr --
 Lorem true