From e2f06cce944b1e2b58cfd0181bf450ba528a72fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Mon, 5 Apr 2021 11:28:52 +0100 Subject: [PATCH] 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 | 44 ++++++++++++++++++++++++++++--- position.go | 2 +- testdata/scripts/literals.txt | 49 +++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/internal/literals/literals.go b/internal/literals/literals.go index c5ebae1..07dee56 100644 --- a/internal/literals/literals.go +++ b/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)) diff --git a/position.go b/position.go index e255236..a045b22 100644 --- a/position.go +++ b/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. diff --git a/testdata/scripts/literals.txt b/testdata/scripts/literals.txt index 25d8b1a..d097b24 100644 --- a/testdata/scripts/literals.txt +++ b/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