Our recent work in fieldToAlias worked well when the embedded field
declaration (using an alias) was in the same package as the use of that
field. We would have the *ast.Ident for the field declaration, so
types.Info.Uses would give us the TypeName for the alias.
Unfortunately, if the declaration was in a dependency package, we did
not have that same *ast.Ident, as we weren't parsing the source code for
dependencies for type-checking. This resulted in us incorrectly
obfuscating the use of such an embedded field:
> garble build
[stderr]
# test/main
JtzmzxWf.go:4: unknown field 'ExternalForeignAlias' in struct literal of type _BdSNiEL.Vcs_smer
To fix this, look through the direct imports of the package defining the
field to find an alias under the exact same name. Not a foolproof
solution, as there's a TODO, but it should work for most cases.
Fixes the obfuscation of google.golang.org/grpc/internal/status, too.
Updates #349.
The added test case, which is obfuscating and linking os/user, would fail
before this fix:
> garble build
[stderr]
# test/main
/usr/lib/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/bin/ld: $WORK/.tmp/go-link-073246656/go.o: in function `Chz0Yfs2._cgo_cmalloc':
go.go:(.text+0x993cc): undefined reference to `Chz0Yfs2.runtime_throw'
/usr/bin/ld: $WORK/.tmp/go-link-073246656/go.o: in function `Chz0Yfs2.tDfhQ8uK':
go.go:(.text+0x99801): undefined reference to `Chz0Yfs2._cgo_runtime_gostring'
/usr/bin/ld: go.go:(.text+0x9982a): undefined reference to `Chz0Yfs2._cgo_runtime_gostring'
/usr/bin/ld: go.go:(.text+0x99853): undefined reference to `Chz0Yfs2._cgo_runtime_gostring'
collect2: error: ld returned 1 exit status
The reason is that we would alter the linkname directives of cgo-generated
code, but we would not obfuscate the code itself at all.
The generated code would end up being transformed into:
//go:linkname zh_oKZIy runtime.throw
func runtime_throw(string)
One can clearly see the error there; handleDirectives obfuscated the
local linkname name, but since transformGo didn't run, the actual Go
declaration was not obfuscated in the same way. Thus, the linker fails
to find a function body for runtime_throw, and fails.
The solution is simple: handleDirectives assumes that it's running on
code being obfuscated, so only run it when transformGo is running.
We can also remove the cgo skip check in handleDirectives, as it never
runs on cgo-generated code now.
Fixes a number of build errors that have been noticed since
907aebd770.
When such an alias name was used to define an embedded field, we handled
that case gracefully via the code using:
tf.info.Uses[node].(*types.TypeName)
Unfortunately, when the same field name was used elsewhere, such as a
composite literal, tf.Info.Uses gave us a *types.Var, not a
*types.TypeName, meaning we could no longer tell if this was an alias,
or what it pointed to.
Thus, we failed to obfuscate the name properly in the added test case:
> garble build
[stderr]
# test/main/sub
xxWZf66u.go:36: unknown field 'foreignAlias' in struct literal of type smhWelwn
It doesn't seem like any of the go/types APIs allows us to obtain the
*types.TypeName directly in this scenario. Thus, use a trick that we
used before: after typechecking, but before obfuscating, record all
embedded struct field *types.Var which are aliases via a map, where the
value holds the *types.TypeName for the alias.
Updates #349.
We already added support for "//go:embed" with string and []byte,
by not obfuscating the "embed" import path.
However, embed.FS was still failing:
> garble build
[stderr]
# test/main
:13: go:embed cannot apply to var of type embed.WtKNvwbN
The compiler detects the type by matching its name to exactly "embed.FS",
so don't obfuscate the name "FS" either.
While at it, ensure that the embed code behaves the same with "go build".
Updates #349.
See the added comment for the rationale. For that same reason, I always
build Go itself via "GOGC=off ./make.bash", as it's noticeably faster.
Before this change:
$ go clean -cache && go test -short
PASS
ok mvdan.cc/garble 35.298s
$ go test -short
PASS
ok mvdan.cc/garble 2.703s
With the change:
$ go clean -cache && go test -short
PASS
ok mvdan.cc/garble 25.323s
$ go test -short
PASS
ok mvdan.cc/garble 2.469s
Incremental test runs with a warm cache are largely unaffected, as those
would run very few of those short-lived and allocation-heavy programs.
However, when the build cache isn't warm (such as when garble itself is
modified), we easily see savings of 20-30%.
We might revisit this in the future if Go's GC gets better in these
situations, which should make "go build" faster. For now, we run our
tests very often, so having them burn a bit less CPU is nice.
Before, "garble build -h" would print the same as "garble -h", which is
too much and confusing, as it doesn't tell us much about "build".
Now it's far better, and includes the output of "go build -h":
$ garble build -h
usage: garble [garble flags] build [arguments]
This command wraps "go build". Below is its help:
usage: go build [-o output] [build flags] [packages]
Run 'go help build' for details.
We do the same for "garble reverse -h", since it doesn't wrap a Go tool
command.
I added a second one recently without noticing, since the original was
too far below and I had written commands above it.
Deduplicate them, and put the new env line as early as possible, to
prevent further issues and confusion.
Every now and then, a CI run would fail:
FAIL: testdata/scripts/reflect.txt:7: unexpected match for ["main.go"] in main
These were rare, and very hard to reproduce or debug.
My best guess is that, since "main.go" is a short string and we use
random eight-character obfuscated filenames ending with ".go", it was
possible that the random filename happened to end in "main" in some
cases.
Given the base64 encoding, the chances of a single suffix collision are
about 0.000006%. Note, however, that a single obfuscated build will most
likely obfuscate many filenames, especially for the tests obfuscating
multiple packages. For a single CI run with many tests across three OSs,
the chances of any collision are likely very low, but realistic.
All this has a simple fix: use longer filenames to match with. We choose
"garble_main.go" since it's long enough, but also because it's still
clear it's a "main" Go file, and it's very unlikely to cause conflicts
with filenames in upstream Go given the "garble_" prefix.
Remove some asthelper APIs that haven't been used for some time.
They can be recovered from the git history if needed again.
One type assertion in the literals package is always true.
Embedded field objects are handled near the top of transformGo, so the
extra !obj.Embedded() check was always true. Remove it.
We always obfuscate standalone funcs now, so the obfuscatedTypesPackage
check is no longer necessary. This was necessary when we used to not
obfuscate func names when they were used in linkname directives.
The workaround for test package imports in obfuscatedTypesPackage I had
to add a few commits ago no longer seems to be necessary. This might be
thanks to the simplification with functions in the paragraph just above.
It's impossible to run garble without -trimpath nowadays, as we error
before the build even starts:
$ go build -toolexec=garble
go tool compile: exit status 1
cannot open shared file, this is most likely due to not running "garble [command]"
When run as "garble build", the trimpath flag is always set. So the
check in alterTrimpath never triggers anymore, and couldn't be tested.
Finally, simplify the handling of comment syntax in printFile, and add a
few TODOs for other code paths not covered by our existing tests.
Total code coverage is up from 90.3% to 91.0%.
Before this change, we would try to never obfuscate alias names. That
was far from ideal, as they can end up in field names via anonymous
fields.
Even then, we would sometimes still fail to build, because we would
inconsistently obfuscate alias names. For example, in the added test
case:
--- FAIL: TestScripts/syntax (0.23s)
testscript.go:397:
> env GOPRIVATE='test/main,private.source'
> garble build
[stderr]
# test/main/sub
Lv_a8gRD.go:15: undefined: KCvSpxmQ
To fix this problem, we set obj to be the TypeName corresponding to the
alias when it is used as an embedded field. We can then make the right
choice when obfuscating the name.
Right now, all aliases will be obfuscated. A TODO exists about not
obfuscating alias names when they're used as embedded fields in a struct
type in the same package, and that package is used for reflection -
since then, the alias name ends up as the field name.
With these changes, the protobuf module now builds.
obfuscatedTypesPackage is used to figure out if a name in a dependency
package was obfuscated or not. For example, if that package used
reflection on a named type, it wasn't obfuscated, so we must have the
same information to not obfuscate the same name downstream.
obfuscatedTypesPackage could return nil if the package was indirectly
imported, though. This can happen if a direct import has a function that
returns an indirect type, or if a direct import exposes a name that's a
type alias to an indirect type.
We sort of dealt with this in two pieces of code by checking for
obfPkg!=nil, but a third one did not have this check and caused a panic
in the added test case:
--- FAIL: TestScripts/reflect (0.81s)
testscript.go:397:
> env GOPRIVATE=test/main
> garble build
[stderr]
# test/main
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x8a5e39]
More importantly though, the nil check only avoids panics. It doesn't
fix the root cause of the problem: that importcfg does not contain
indirectly imported packages. The added test case would still fail, as
we would obfuscate a type in the main package, but not in the indirectly
imported package where the type is defined.
To fix this, resurrect a bit of code from earlier garble versions, which
uses "go list -toolexec=garble" to fetch a package's export file. This
lets us fill the indirect import gaps in importcfg, working around the
problem entirely.
This solution is still not particularly great, so we add a TODO about
possibly rethinking this in the future. It does add some overhead and
complexity, though thankfully indirect imports should be uncommon.
This fixes a few panics while building the protobuf module.
Our allPkgs boolean logic was wrong, because it could still lead to
garble obfuscating a type when used in the main package, but not in its
defining package. The added test case shows such a case.
To fix that, use a package path to only record the named objects from
the target package, which is a narrower operation without this problem,
but still makes all our tests pass.
This makes the google.golang.org/protobuf/internal/filetype package
start building.
I've wanted this to more easily debug build failures.
To not force a build failure in a test script, as that would require
some trickery to remain stable, we use runtime.Caller without printing
the line number. Before this patch, those filenames without line numbers
would not be reversed at all.
If a package A imports package B, and uses reflect.TypeOf on an unnamed
struct type B.T (such as an alias), we don't want to record B.T's fields
as "do not obfuscate". This is for the same reason that we don't if B.T
is a named struct type: the detection only works for the package
defining the type, as otherwise it's inconsistent.
We failed to handle this case well, because we assumed all struct types
would be under some named type. This is not the case for type aliases.
Fortunately, struct fields are named, and as such they are objects.
Check their package too, just like we do for named types.
Fixes another build error when obfuscating the protobuf module.
We add a simplified version of the example above as a test case,
which originated from debugging the protobuf build failure.
In printFile, we print and re-parse the modified AST to be able to have
reliable position information.
The re-parsing step can fail if something goes very wrong, such as a bug
in -literals. It should generally not happen. However, in rare cases it
has happened, and it's confusing for the end user to see syntax errors
pointing at an existing file on disk, when the code doesn't align
- since we're on a modified copy.
To prevent such confusion, use an empty filename. Syntax errors will
still not be terribly helpful, but they should be extremely rare and
promptly fixed, so that's not a huge concern.
For that same reason, we can't really add a good test here. We could
perhaps add a test that forces garble to mess up the src slice in some
way, but that would be a weird test, and not particularly worth it.
Fixes#286.
The detection of reflection usage is tricky and there are plenty of edge
cases to test for. We definitely want one script for it, rather than
splitting those cases between other scripts like imports.txt and
syntax.txt.
Moreover, those two were rather generic and large, so this helps keep a
balance.
We were using temporary filenames for modified Go and assembly files.
For example, an obfuscated "encoding/json/encode.go" would end up as:
/tmp/garble-shared123/encode.go.456.go
where "123" and "456" are random numbers, usually longer.
This was usually fine for two reasons:
1) We would add "/tmp/garble-shared123/" to -trimpath, so the temporary
directory and its random number would be invisible.
2) We would add "//line" directives to the source files, replacing
the filename with obfuscated versions excluding any random number.
Unfortunately, this broke in multiple ways. Most notably, assembly files
do not have any line directives, and it's not clear that there's any
support for them. So the random number in their basename could end up in
the binary, breaking reproducibility.
Another issue is that the -trimpath addition described above was only
done for cmd/compile, not cmd/asm, so assembly filenames included the
randomized temporary directory.
To fix the issues above, the same "encoding/json/encode.go" would now
end up as:
/tmp/garble-shared123/encoding/json/encode.go
Such a path is still unique even though the "456" random number is gone,
as import paths are unique within a single build.
This fixes issues with the base name of each file, so we no longer rely
on line directives as the only way to remove the second original random
number.
We still rely on -trimpath to get rid of the temporary directory in
filenames. To fix its problem with assembly files, also amend the
-trimpath flag when running the assembler tool.
Finally, add a test that reproducible builds still work when a full
rebuild is done. We choose goprivate.txt for such a test as its
stdimporter package imports a number of std packages, including uses of
assembly and cgo.
For the time being, we don't use such a "full rebuild" reproducibility
test in other test scripts, as this step is expensive, rebuilding many
packages from scratch.
This issue went unnoticed for over a year because such random numbers
"123" and "456" were created when a package was obfuscated, and that
only happened once per package version as long as the build cache was
kept intact.
When clearing the build cache, or forcing a rebuild with -a, one gets
new random numbers, and thus a different binary resulting from the same
build input. That's not something that most users would do regularly,
and our tests did not cover that edge case either, until now.
Fixes#328.
I've tripped over this GODEBUG env var four or five times already.
Since it affects any Go program by making them print tons of runtime
information, it also affects "go env", breaking garble horribly.
To prevent further issues, unset the env var when done.
Packages P1 and P2 can define identical struct types T1 and T2, and one
can convert from type T1 to T2 or vice versa.
The spec defines two identical struct types as:
Two struct types are identical if they have the same sequence of
fields, and if corresponding fields have the same names, and
identical types, and identical tags. Non-exported field names
from different packages are always different.
Unfortunately, garble broke this: since we obfuscated field names
differently depending on the package, cross-package conversions like the
case above would result in typechecking errors.
To fix this, implement Joe Tsai's idea: hash struct field names with the
string representation of the entire struct. This way, identical struct
types will have their field names obfuscated in the same way in all
packages across a build.
Note that we had to refactor "reverse" a bit to start using transformer,
since now it needs to keep track of struct types as well.
This failure was affecting the build of google.golang.org/protobuf,
since it makes regular use of cross-package struct conversions.
Note that the protobuf module still fails to build, but for other
reasons. The package that used to fail now succeeds, so the build gets a
bit further than before. #240 tracks adding relevant third-party Go
modules to CI, so we'll track the other remaining failures there.
Fixes#310.
Previously, the local name part of linkname directives were never
obfuscated. Now that we can obfuscate function names in Go assembly,
that limitation is required no longer.
Fixes#309
Obfuscating some std packages for windows/arm triggered a bug; when
encountering a call to runtime·memmove, we'd hash "memmove" with the
current package's action ID.
This is wrong on two levels: First, we aren't obfuscating the runtime
package yet. And second, if we did, we would have to hash the symbol
appropriately, with that package's action ID.
For now, only hashing the local names does the trick. That's all that
the code currently supports, anyway.
Since 1a8e32227f we have been setting line numbers on call sites, and
that line number is always the minimum value for -tiny: 1.
Still not as zero-overhead as the old mechanism which entirely deleted
line numbers, but a useless and small line number is still pretty good.
THey don't show up in stack traces, and if they show up in regular
program output, we should in theory not obfuscate those names via the
detection of reflection.
However, there's one relatively common scenario where obfuscated field
names can appear: in build error messages, when obfuscation fails a
build.
Our previous logic only took care of fairly simple types, such as a
simple struct or a pointer to a struct. If we had a struct embedding
another struct, we'd fail to record the objects for the fields in the
inner struct, and that would lead to miscompilation:
> garble build
[stderr]
# test/main
LZmt64Nm.go:7: outer.InnerField undefined (type *CcUt1wkQ.EmbeddingOuter has no field or method InnerField)
To fix this issue, make the function that records all objects under a
types.Type smarter. Since it now does more than just dealing with
structs, it's also renamed.
Since the function now walks types properly, we get to remove the extra
ast.Inspect in recordReflectArgs, which is nice.
We also make it a method, to avoid the map parameter. A boolean
parameter is also added, since we need this feature to only look at the
current package when looking at reflect calls.
Finally, we add a test case, a simplified version of the original bug
report.
Fixes#315.
The docs for go/importer.ForCompiler say:
The lookup function is called each time the resulting importer
needs to resolve an import path. In this mode the importer can
only be invoked with canonical import paths (not relative or
absolute ones); it is assumed that the translation to canonical
import paths is being done by the client of the importer.
We use a lookup func for two reasons: first, to support modules, and
second, to be able to use our information from "go list -json -export".
However, go/types does not canonicalize import paths before calling
ImportFrom. This is somewhat understandable; it doesn't know whether an
importer was created with a lookup func, and ImportFrom only requires
the input path to be canonicalized in that scenario. When the lookup
func is nil, the importer canonicalizes by itself via go/build.Import.
Before this change, the added crossbuild test would fail:
> garble build net/http
[stderr]
# vendor/golang.org/x/crypto/chacha20
typecheck error: /usr/lib/go/src/vendor/golang.org/x/crypto/chacha20/chacha_generic.go:10:2: could not import crypto/cipher (can't find import: "crypto/cipher")
# vendor/golang.org/x/text/secure/bidirule
typecheck error: /usr/lib/go/src/vendor/golang.org/x/text/secure/bidirule/bidirule.go:12:2: could not import errors (can't find import: "errors")
# vendor/golang.org/x/crypto/cryptobyte
typecheck error: /usr/lib/go/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go:8:16: could not import encoding/asn1 (can't find import: "encoding/asn1")
# vendor/golang.org/x/text/unicode/norm
typecheck error: /usr/lib/go/src/vendor/golang.org/x/text/unicode/norm/composition.go:7:8: could not import unicode/utf8 (can't find import: "unicode/utf8")
This is because we'd fall back to importer.Default, which only knows how
to find packages in $GOROOT/pkg. Those are missing for cross-builds,
unsurprisingly, as those built archives end up in the build cache.
After this change, we properly support importing std-vendored packages,
so we can get rid of the importer.Default workaround. And, by extension,
cross-builds now work as well.
Note that, in the added test script, the full build of the binary fails,
as there seems to be some sort of linker problem:
> garble build
[stderr]
# test/main
d9rqJyxo.uoqIiDs5: relocation target runtime.os9A16A3 not defined
We leave that as a TODO for now, as this change is subtle enough as it
is.
In the worst case scenario, when GOPRIVATE isn't set at all, we would
run these three commands:
* "go env GOPRIVATE", to fetch GOPRIVATE itself
* "go list -m", for GOPRIVATE's fallback
* "go version", to check the version of Go being used
Now that we support Go 1.16 and later, all these three can be obtained
via "go env -json":
$ go env -json GOPRIVATE GOMOD GOVERSION
{
"GOMOD": "/home/mvdan/src/garble/go.mod",
"GOPRIVATE": "",
"GOVERSION": "go1.16.3"
}
Note that we don't get the module path directly, but we can use the
x/mod/modfile Go API to parse it from the GOMOD file cheaply.
Notably, this also simplifies our Go version checking logic, as now we
get just the version string without the "go version" prefix and
"GOOS/GOARCH" suffix we don't care about.
This makes our code a bit more maintainable and robust. When running a
short incremental build, we can also see a small speed-up, as saving two
"go" invocations can save a few milliseconds:
name old time/op new time/op delta
Build/Cache-8 168ms ± 0% 166ms ± 1% -1.26% (p=0.009 n=6+6)
name old bin-B new bin-B delta
Build/Cache-8 6.36M ± 0% 6.36M ± 0% +0.12% (p=0.002 n=6+6)
name old sys-time/op new sys-time/op delta
Build/Cache-8 222ms ± 2% 219ms ± 3% ~ (p=0.589 n=6+6)
name old user-time/op new user-time/op delta
Build/Cache-8 857ms ± 1% 846ms ± 1% -1.31% (p=0.041 n=6+6)
In ce2c45440a, we simplified the code a bit and removed one call to
obfuscatedTypesPackage.
Unfortunately, we introduced a regression; if an exported function is
linknamed to another symbol name, and it's called from an importer
package, we would have a build failure now:
> garble build
[stderr]
# test/main
ZiOACuw7.go:1: undefined: ODC0xN52.BaDqbhkj
This is because the imported package would not hash the original name,
via its ignoreObjects logic. And, since the importer package has no
access to that knowledge, it would hash the same name, and fail to find
it in the final build.
The regression happened because we used to have a types.Scope Lookup
that saved us in this scenario. Add the test, and re-add the Lookup,
this time only for this particular scenario with function names.
Thanks to Andrew LeFevre for reporting and describing the test case.
While at it, replace more uses of "garbled" to "obfuscated".
Mention two more bugfixes. Not the initial Go 1.17 support just yet,
because coincidentally it broke in the past 24h due to upstream changes.
Also tweak some of the other wording to be clearer.
Since it changes rapidly, especially during merge cycles, and we don't
want CI to surprisingly blow up in our faces from one day to another.
Pin this to a commit from yesterday which works, since some changes
merged today moved where the Go build version is recorded and broke
garble.
While at it, replace "git clone" with a wget of a source archive. This
is much, much faster, mainly because a tarball is significantly smaller.
We now download about 20MiB instead of over 350MiB.
One downside is that, without git, make.bash can't construct a devel
version on its own. For that reason, add a pretty basic manual version
via the VERSION file.
This means that we must not reject custom devel version strings. This is
a good thing anyway, because custom devel strings are already common
when building Go in custom ways. Those people tend to be advanced users,
such as CI, so fall back to assuming they know what they are doing and
don't error.
Plus, starting last week, devel versions in Go master now contain the
major Go version like in build tags, such as "go1.17-commit...", so we
will soon start relying on that instead of parsing dates:
$ go version
go version devel go1.17-a7e16abb22 Thu Apr 8 07:33:58 2021 +0000 linux/amd64
Just two minor tweaks were necessary to get "go test" to pass on:
go version devel go1.17-a25c584629 Tue Apr 6 04:48:09 2021 +0000 linux/amd64
Re-enable the CI for it, too. The config needed changing since the
set-env and add-path commands now use special files instead, due to some
security issues uncovered last winter.
It's possible that CI on master could suddenly break, if Go master
changes in some substantial way that requires more tweaks. If that turns
out to be an issue pretty often, we could always pin a specific git repo
commit and update it every few weeks.
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.
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.
This is easier to understand, since now the modification of the
*ast.File is all within a single chunk of code. We can also simplify
literals.Obfuscate to work on a single file, as transformGo runs in a
loop.
We also remove the "use receiver" TODOs, since the code is now in a
different package and it can't declare methods on a type here.
Walking the entire Go file again seems unnecessary, when we have an
astutil.Apply just next to it.
We could add to its "pre" function, but it's a bit easier to use the
"post" instead, which is empty.
Another advantage here is that astutil.Apply is more powerful than
ast.Inspect, which can come in handy in the future.
Reduces "go test -short" with a warm build cache from ~9s to ~4s. The
main offender was the use of "-a" in the "garble test" call; I think I
added that flag to rebuild all packages when debugging an error, and
forgot to remove it.
The sections are a bit more organized now. We add better docs for flags,
a new section on build speed, and we also remove the "withgo" helper as
it seems a bit over the top.
Fixes#275.
Daniel Martí
Damien Lespiau
Andrew LeFevre
lu4p