Commit Graph

59 Commits (master)

Author SHA1 Message Date
Daniel Martí 69bc62c56c start using some Go 1.22 features
We no longer need to worry about the scope of range variables,
we can iterate over integers directly, and we can use cmp.Or too.

I haven't paid close attention to using these everywhere.
This is mainly testing out the new features where I saw some benefit.
3 months ago
Daniel Martí 344cdd5e7b make `go test -race` fast again on Go 1.21
On my laptop, `go test -short -race` on Go 1.20 used to take about 62s.
Jumping to Go 1.21, I was surprised to see an increase to 152s,
more than double - which was weird given how often the CPU was idle.

This manifested when updating our CI to start testing on Go 1.21.
Where Go 1.20 on Linux took about 10m to run `go test -race`,
Go 1.21 hit the 20m timeout every single time.

After a bit of googling, I was reminded of https://go.dev/issues/20364
as well as https://go.dev/doc/articles/race_detector#Options:

    atexit_sleep_ms (default 1000): Amount of milliseconds to sleep in the main goroutine before exiting.

This default is a bit aggressive for Go, but usually harmless,
having each test binary sleep for 1s after the package has been tested.

However, this 1s sleep after main runs is horrendous for garble's tests;
the testscripts run `garble build` many times, running the test binary.
It then runs `go build -toolexec=garble`, which runs the test binary
many more times: for every compiler, linker, etc invocation.

This means that our testscripts would include dozens of 1s sleeps,
in many cases blocking the continuation of the entire test.
This seemed to not be happening on earlier Go versions due to a bug;
Go 1.21's race mode started obeying this default properly.

The added change sets atexit_sleep_ms to something more reasonable
if GORACE isn't set at all; 10ms doesn't disable this check entirely,
but its overhead is orders of magnitude less noticeable than 1000ms.
`go test -short -race` on Go 1.21 drops back down to 68s for me.
8 months ago
Daniel Martí 6dd5c53a91 internal/linker: place files under GARBLE_CACHE
This means we now have a unified cache directory for garble,
which is now documented in the README.

I considered using the same hash-based cache used for pkgCache,
but decided against it since that cache implementation only stores
regular files without any executable bits set.
We could force the cache package to do what we want here,
but I'm leaning against it for now given that single files work OK.
12 months ago
Daniel Martí 7d1bd13778 replace our caching inside GOCACHE with GARBLE_CACHE
For each Go package we obfuscate, we need to store information about
how we obfuscated it, which is needed when obfuscating its dependents.
For example, if A depends on B to use the type B.Foo, A needs to know
whether or not B.Foo was obfuscated; it depends on B's use of reflect.

We record this information in a gob file, which is cached on disk.
To avoid rolling our own custom cache, and since garble is so closely
connected with cmd/go already, we piggybacked off of Go's GOCACHE.
In particular, for each build cache entry per `go list`'s Export field,
we would store a "garble" sibling file with that gob content.

However, this was brittle for two reasons:

1) We were doing this without cmd/go's permission or knowledge.
   We were careful to use filename suffixes similar to Export files,
   meaning that `go clean` and other commands would treat them the same.
   However, this could confuse cmd/go at any point in the future.

2) cmd/go trims cache entries in GOCACHE regularly, to keep the size of
   the build and test caches under control. Right now, this means that
   every 24h, any file not accessed in the last five days is deleted.
   However, that trimming heuristic is done per-file.
   If the trimming removed Garble's sibling file but not the original
   Export file, this could cause errors such as
   "cannot load garble export file" which users already ran into.

Instead, start using github.com/rogpeppe/go-internal/cache,
an exported copy of cmd/go's own cache implementation for GOCACHE.
Since we need an entirely separate directory, we introduce GARBLE_CACHE,
defaulting to the "garble" directory inside the user's cache directory.
For example, on Linux this would be ~/.cache/garble.

Inside GARBLE_CACHE, our gob file cache will be under "build",
which helps clarify that this cache is used when obfuscating Go builds,
and allows placing other kinds of caches inside GARBLE_CACHE.
For example, we already have a need for storing linker binaries,
which for now still use their own caching mechanism.

This commit does not make our cache properly resistant to removed files.
The proof is that our seed.txtar testscript still fails the second case.
However, we do rewrite all of our caching logic away from Export files,
which in itself is a considerable refactor, and we add a few TODOs.

One notable change is how we load gob files from dependencies
when building the cache entry for the current package.
We used to load the gob files from all packages in the Deps field.
However, that is the list of all _transitive_ dependencies.
Since these gob files are already flat, meaning they contain information
about all of their transitive dependencies as well, we need only load
the gob files from the direct dependencies, the Imports field.

Performance is largely unchanged, since the behavior is similar.
However, the change from Deps to Imports saves us some work,
which can be seen in the reduced mallocs per obfuscated build.

It's unclear why the binary size isn't stable.
When reverting the Deps to Imports change, it then settles at 5.386Mi,
which is almost exactly in between the two measurements below.
I'm not sure why, but that metric appears to be slightly unstable.

    goos: linux
    goarch: amd64
    pkg: mvdan.cc/garble
    cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
            │    old     │             new              │
            │   sec/op   │   sec/op    vs base          │
    Build-8   11.09 ± 1%   11.08 ± 1%  ~ (p=0.796 n=10)

            │     old      │                 new                 │
            │    bin-B     │    bin-B      vs base               │
    Build-8   5.390Mi ± 0%   5.382Mi ± 0%  -0.14% (p=0.000 n=10)

            │      old      │               new               │
            │ cached-sec/op │ cached-sec/op  vs base          │
    Build-8     415.5m ± 4%     421.6m ± 1%  ~ (p=0.190 n=10)

            │     old     │                new                 │
            │ mallocs/op  │ mallocs/op   vs base               │
    Build-8   35.43M ± 0%   34.05M ± 0%  -3.89% (p=0.000 n=10)

            │    old     │             new              │
            │ sys-sec/op │ sys-sec/op  vs base          │
    Build-8   5.662 ± 1%   5.701 ± 2%  ~ (p=0.280 n=10)
12 months ago
Daniel Martí 9044b1d31c add a test to reproduce our caching bugs
The test passes, because it expects garble to fail the same way that
users are seeing in practice.

For #708.
1 year ago
Daniel Martí e6fc593f68 set testscript's RequireExplicitExec and RequireUniqueNames
The first makes our test scripts more consistent, as all external
program executions happen via "exec" and are not as easily confused
with custom builtin commands like our "generate-literals".

The second catches mistakes if any of our txtar files have duplicate
files, where all but one of the contents would be ignored before.
1 year ago
Dominic Breuker b587d8c01a
use the "simple" obfuscator for large literals
Changes literal obfuscation such that literals of any size will be obfuscated,
but beyond `maxSize` we only use the `simple` obfuscator.
This one seems to apply AND, OR, or XOR operators byte-wise and should be safe to use,
unlike some of the other obfuscators which are quadratic on the literal size or worse.

The test for literals is changed a bit to verify that obfuscation is applied.
The code written to the `extra_literals.go` file by the test helper now ensures
that Go does not optimize the literals away when we build the binary.
We also append a unique string to all literals so that we can test that
an unobfuscated build contains this string while an obfuscated build does not.
1 year ago
Daniel Martí a186419d3d avoid rebuilding garble in the main benchmark
Similar to what testscript does, we can reuse the test binary by telling
TestMain to run the main function rather than the Go tests.

This saves a few hundred milliseconds out of each benchmark run.
1 year ago
Daniel Martí bb69facbd8 cut test time with coverage by half
Per the inline comment, we want to build every package in the test suite
at least once, otherwise we won't get proper code coverage information.
For example, some code only triggers when obfuscating the runtime,
and if I run "go test" twice in a row, the second might reuse the
first's obfuscation of the runtime and skip the code entirely.

However, per the TODO, we used to solve this in a rather wasteful way,
by making each test use its own separate GOCACHE directory.
Instead, use a new GOCACHE directory, but share it between tests.
This is still enough, as we still build each package at least once.

Running the command below twice in a row with Go 1.20:

	go test -cover

took about 1m55s on my laptop before, and now takes about 1m10s.
Not great, but a noticeable improvement.
Both report a total coverage of 88.7%.

While here, do the same for GARBLE_CACHE_DIR,
and use testscript.Env.Setenv for simplicity.
1 year ago
Daniel Martí e37f39054d update testscript to support code coverage with Go 1.20
See https://github.com/rogpeppe/go-internal/pull/201.
1 year ago
Daniel Martí 658060851d drop bits of code to support Go 1.19 1 year ago
pagran 6ace03322f
patch and rebuild cmd/link to modify the magic value in pclntab
This value is hard-coded in the linker and written in a header.
We could rewrite the final binary, like we used to do with import paths,
but that would require once again maintaining libraries to do so.

Instead, we're now modifying the linker to do what we want.
It's not particularly hard, as every Go install has its source code,
and rebuilding a slightly modified linker only takes a few seconds at most.

Thanks to `go build -overlay`, we only need to copy the files we modify,
and right now we're just modifying one file in the toolchain.
We use a git patch, as the change is fairly static and small,
and the patch is easier to understand and maintain.

The other side of this change is in the runtime,
as it also hard-codes the magic value when loading information.
We modify the code via syntax trees in that case, like `-tiny` does,
because the change is tiny (one literal) and the affected lines of code
are modified regularly between major Go releases.

Since rebuilding a slightly modified linker can take a few seconds,
and Go's build cache does not cache linked binaries,
we keep our own cached version of the rebuilt binary in `os.UserCacheDir`.

The feature isn't perfect, and will be improved in the future.
See the TODOs about the added dependency on `git`,
or how we are currently only able to cache one linker binary at once.

Fixes #622.
1 year ago
Daniel Martí d955196470 avoid using math/rand's global funcs like Seed and Intn
Go 1.20 is starting to deprecate the use of math/rand's global state,
per https://go.dev/issue/56319 and https://go.dev/issue/20661.
The reasoning is sound:

	Deprecated: Programs that call Seed and then expect a specific sequence
	of results from the global random source (using functions such as Int)
	can be broken when a dependency changes how much it consumes from the
	global random source. To avoid such breakages, programs that need a
	specific result sequence should use NewRand(NewSource(seed)) to obtain a
	random generator that other packages cannot access.

Aside from the tests, we used math/rand only for obfuscating literals,
which caused a deterministic series of calls like Intn. Our call to Seed
was also deterministic, per either GarbleActionID or the -seed flag.

However, our determinism was fragile. If any of our dependencies or
other packages made any calls to math/rand's global funcs, then our
determinism could be broken entirely, and it's hard to notice.

Start using separate math/rand.Rand objects for each use case.
Also make uses of crypto/rand use "cryptorand" for consistency.

Note that this requires a bit of a refactor in internal/literals
to start passing around Rand objects. We also do away with unnecessary
short funcs, especially since math/rand's Read never errors,
and we can obtain a byte via math/rand's Uint32.
1 year ago
Daniel Martí 12bc0349e6 make bincmp keep binaries around when it fails
Even if diffoscope is installed, because further investigation might be
needed, and some failures are rare or hard to reproduce.

Make GitHub Actions upload those artifacts,
so that a failed CI run on Windows or Mac due to bincmp
allows us to download and inspect those binaries locally.
2 years ago
Daniel Martí 3c7141e801 update the state of a few TODOs related to upstream Go
The generics issue has been fixed for the upcoming Go 1.20.
Include that version as a reminder for when we can drop Go 1.19.

The fs.SkipAll proposal is also implemented for Go 1.20.

The BinaryContentID comment was a little bit trickier.
We did get stamped VCS information some time ago,
but it only provides us with the current commit info and a dirty bit.
That is not enough for our use of the build cache,
because we want any uncommitted changes to garble to cause rebuilds.

I don't think we'll get any better than using garble's own build ID.
Reword the quasi-TODO to instead explain what we're doing and why.
2 years ago
Daniel Martí 99c12e396a replace testdata/scripts/*.txt with testdata/script/*.txtar
Following the best practices from upstream.
In particular, the "txt" extension is somewhat ambiguous.

This may cause some conflicts due to the git diff noise,
but hopefully we won't ever do this again.
2 years ago
Daniel Martí d0c6ccd63d sleep between cp and exec in test scripts
Every now and then, I get test failures in the goenv test like:

	> [!windows] cp $EXEC_PATH $NAME/garble$exe
	> [!windows] exec $NAME/garble$exe build
	[fork/exec with"double"quotes/garble: text file busy]
	FAIL: testdata/scripts/goenv.txt:21: unexpected command failure

The root cause is https://go.dev/issue/22315, which isn't going to be
fixed anytime soon, as it is a race condition in Linux itself, triggered
by how heavily concurrent Go tends to be.

For now, try to make the race much less likely to happen.
2 years ago
Daniel Martí 2d12f41e71 actually remove temporary directories after obfuscation
Back in February 2021, we changed the obfuscation logic so that the
entire `garble build` process would use one shared temporary directory
across all package builds, reducing the amount of files we created in
the top-level system temporary directory.

However, we made one mistake: we didn't swap os.Remove for os.RemoveAll.
Ever since then, we've been leaving temporary files behind.

Add regression tests, which failed before the fix, and fix the bug.
Note that we need to test `garble reverse` as well, as it calls
toolexecCmd separately, so it needs its own cleanup as well.

The cleanup happens via the env var, which doesn't feel worse than
having toolexecCmd return an extra string or cleanup func.

While here, also test that we support TMPDIRs with special characters.
2 years ago
Daniel Martí f37561589b properly quote the path to garble in -toolexec
If we don't quote it, paths containing spaces or quote characters will
fail. For instance, the added test without the fix fails:

        > env NAME='with spaces'
        > mkdir $NAME
        > cp $EXEC_PATH $NAME/garble$exe
        > exec $NAME/garble$exe build main.go
        [stderr]
        go tool compile: fork/exec $WORK/with: no such file or directory
        exit status 1

Luckily, the fix is easy: we bundle Go's cmd/internal/quoted package,
which implements a QuotedJoin API for this very purpose.

Fixes #544.
2 years ago
Daniel Martí 434de2e472 make early errors count towards code coverage
I recently added TODOs for bits of code we should cover in the tests.
I was looking at that again just now, and was puzzled;
we do indeed have test cases for many of them already.

We just weren't counting them towards code coverage due to a bug.
errJustExit works as expected, except that it calls os.Exit directly,
whereas testscript wants a non-zero return to run its "after" code.
Part of that code is what handles joining code coverage files.

The total code coverage jumps from 86.2% to 87.6%.
2 years ago
Daniel Martí c1c90fee13 make obfuscation fully deterministic with -seed
The default behavior of garble is to seed via the build inputs,
including the build IDs of the entire Go build of each package.
This works well as a default, and does give us determinism,
but it means that building for different platforms
will result in different obfuscation per platform.

Instead, when -seed is provided, don't use any other hash seed or salt.
This means that a particular Go name will be obfuscated the same way
as long as the seed, package path, and name itself remain constant.

In other words, when the user supplies a custom -seed,
we assume they know what they're doing in terms of storage and rotation.

Expand the README docs with more examples and detail.

Fixes #449.
2 years ago
Daniel Martí 29ea99fc5f CI: test on GOARCH=386
Note that this cross-compilation disables cgo by default,
and so the cgo.txt test script isn't run on GOARCH=386.
That seems fine for now, as the test isn't arch-specific.

This testing uncovered one build failure in internal/literals;
the comparison between int and math.MaxUint32 is invalid on 32-bit.
To fix that build failure, use int64 consistently.

One test also incorrectly assumed amd64; it now supports 386 too.
For any other architecture, it's being skipped for now.

I also had to increase the -race test timeout,
as it usually takes 8-9m on GitHub Actions,
and the timeout would sometimes trigger.

Finally, use "go env" rather than "go version" on CI,
which gives us much more useful information,
and also includes Go's own version now via GOVERSION.

Fixes #426.
2 years ago
Daniel Martí caa9831a63
fail if we are unexpectedly overwriting files (#418)
While investigating a bug report,
I noticed that garble was writing to the same temp file twice.
At best, writing to the same path on disk twice is wasteful,
as the design is careful to be deterministic and use unique paths.
At worst, the two writes could cause races at the filesystem level.

To prevent either of those situations,
we now create files with os.OpenFile and os.O_EXCL,
meaning that we will error if the file already exists.
That change uncovered a number of such unintended cases.

First, transformAsm would write obfuscated Go files twice.
This is because the Go toolchain actually runs:

	[...]/asm -gensymabis [...] foo.s bar.s
	[...]/asm [...] foo.s bar.s

That is, the first run is only meant to generate symbol ABIs,
which are then used by the compiler.
We need to obfuscate at that first stage,
because the symbol ABI descriptions need to use obfuscated names.

However, having already obfuscated the assembly on the first stage,
there is no need to do so again on the second stage.
If we detect gensymabis is missing, we simply reuse the previous files.

This first situation doesn't seem racy,
but obfuscating the Go assembly files twice is certainly unnecessary.

Second, saveKnownReflectAPIs wrote a gob file to the build cache.
Since the build cache can be kept between builds,
and since the build cache uses reproducible paths for each build,
running the same "garble build" twice could overwrite those files.

This could actually cause races at the filesystem level;
if two concurrent builds write to the same gob file on disk,
one of them could end up using a partially-written file.

Note that this is the only of the three cases not using temporary files.
As such, it is expected that the file may already exist.
In such a case, we simply avoid overwriting it rather than failing.

Third, when "garble build -a" was used,
and when we needed an export file not listed in importcfg,
we would end up calling roughly:

	go list -export -toolexec=garble -a <dependency>

This meant we would re-build and re-obfuscate those packages.
Which is unfortunate, because the parent process already did via:

	go build -toolexec=garble -a <main>

The repeated dependency builds tripped the new os.O_EXCL check,
as we would try to overwrite the same obfuscated Go files.
Beyond being wasteful, this could again cause subtle filesystem races.
To fix the problem, avoid passing flags like "-a" to nested go commands.

Overall, we should likely be using safer ways to write to disk,
be it via either atomic writes or locked files.
However, for now, catching duplicate writes is a big step.
I have left a self-assigned TODO for further improvements.

CI on the pull request found a failure on test-gotip.
The failure reproduces on master, so it seems to be related to gotip,
and not a regression introduced by this change.
For now, disable test-gotip until we can investigate.
3 years ago
Daniel Martí 64cbbbaa0f update modinfo.txt test for 1.18's build and VCS info
That is, use a very specific build tag and git commit,
and ensure that neither ends up in the binary.

Luckily, we have nothing to do here.
We were already removing _gomod_.go from the build entirely,
and that is still the mechanism that "go build" uses to bundle the data.

Note that the test will still work if git is not installed,
but it will simply not check the VCS side.

Finally, we use "go version -m" to check the existing fields,
which is easier than calling the Go APIs directly.

It seems like "go test" passes on yesterday's Go master, now.
So, enable test-gotip again with that commit hash.

Fixes #385.
3 years ago
Daniel Martí 29356f30f7 update runtimeAndDeps for Go 1.18
In particular, internal/abi now has some actual code,
so obfuscating those literals was breaking as expected.
Document how to update the list in the future as well.

The change above gets "go test" to just one test failure on:

	go version devel go1.18-578ada410d Tue Nov 9 22:58:24 2021 +0000 linux/amd64

We also move the doc about why we disable GarbleLiterals,
so that it's next to where the disabling happens.

While here, we also rename GarbleLiterals to ObfuscateLiterals,
as we have been trying to move away from "to garble" as a verb.

Finally, limit the verbosity of diffoscope.
One test was failing for me, and diffoscope printed thousands of lines.
Not particularly useful when I'm trying to skim test results.
Usually, seeing a few dozen lines of output is enough.

Updates #385.
3 years ago
Daniel Martí 5e1e4d710b
fix a data race with the global cachedBinary mechanism (#413)
Spotted by our friend "go test -race":

	WARNING: DATA RACE
	Write at 0x0000010522d8 by goroutine 69:
	  mvdan.cc/garble.readFile()
		  garble/main_test.go:124 +0x23a
	  mvdan.cc/garble.binsubstr()
		  garble/main_test.go:141 +0xc4
	  github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
		  github.com/rogpeppe/go-internal@v1.8.1-0.20211023094830-115ce09fd6b4/testscript/testscript.go:496 +0x9e8
	  [...]

	Previous write at 0x0000010522d8 by goroutine 60:
	  mvdan.cc/garble.readFile()
		  garble/main_test.go:124 +0x23a
	  mvdan.cc/garble.binsubstr()
		  garble/main_test.go:141 +0xc4
	  github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
		  github.com/rogpeppe/go-internal@v1.8.1-0.20211023094830-115ce09fd6b4/testscript/testscript.go:496 +0x9e8
	  [...]

This wasn't a data race that we spotted via failures in practice,
as it only affected test code since July.

The race is due to the fact that each test script runs as a parallel
sub-test within the same Go program, sharing all globals.
As such, a single "cached binary" global is read and written with races.

Moreover, note that the caching always missed.
I briefly rewrote the code to avoid the race via a sync.Map keyed by
absolute filenames, and while that removed the data race,
the caching never actually hit.

To have a cache hit, we need an absolute path to already be in the cache
and for it to not have been modified since it was last cached. That is:

	modify-bin-1 foo
	binsubstr foo 'abc' # miss
	binsubstr foo 'def' # hit; use the cached "/tmp/[...]/foo" entry

	modify-bin-2 foo
	binsubstr foo 'abc' # miss

However, the test scripts don't do contiguous binsubstr calls like
these. Instead, they join repeated binsubstr calls:

	modify-bin-1 foo
	binsubstr foo 'abc' 'def' # miss

	modify-bin-2 foo
	binsubstr foo 'abc' # miss

For that reason, remove the extra code entirely.
I didn't notice any change to the performance of "go test -short"
with a warm build cache, with:

	go test -c
	./garble.test -test.short #warm cache
	benchcmd -n 5 TestShort ./garble.test -test.short

	name       old time/op         new time/op         delta
	TestShort          4.62s ±12%          4.35s ±12%   ~     (p=0.310 n=5+5)

	name       old user-time/op    new user-time/op    delta
	TestShort          16.8s ± 3%          16.7s ± 3%   ~     (p=0.690 n=5+5)

	name       old sys-time/op     new sys-time/op     delta
	TestShort          7.28s ± 1%          7.26s ± 2%   ~     (p=0.841 n=5+5)

	name       old peak-RSS-bytes  new peak-RSS-bytes  delta
	TestShort          305MB ± 0%          306MB ± 0%   ~     (p=0.421 n=5+5)

Finally, start using "go test -race" on Linux on CI,
which should have made the PR back in July red before merging.
3 years ago
Daniel Martí 680e5624e9 speed up tests by 20-30% by using GOGC=off
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.
3 years ago
Daniel Martí fe095ef132
handle unknown flags in reverse (#290)
While at it, expand the tests for build and test too.
3 years ago
Daniel Martí ff0bea73b5
all: drop support for Go 1.15.x (#265)
This mainly cleans up the few bits of code where we explicitly kept
support for Go 1.15.x. With v0.1.0 released, we can drop support now,
since the next v0.2.0 release will only support Go 1.16.x.

Also updates all modules, including test ones, to 'go 1.16'.

Note that the TOOLEXEC_IMPORTPATH refactor is not done here, despite all
the TODOs about doing so when we drop 1.15 support. This is because that
refactor needs to be done carefully and might have side effects, so it's
best to keep it to a separate commit.

Finally, update the deps.
3 years ago
Daniel Martí d33faabb94
remove unused test cmds (#226)
binsubint and binsubfloat haven't been used since 388ff7d1a4 over half a
year ago, and they're a significant amount of code.

Remove them for now; we can always re-add them from the git history if
needed.
3 years ago
Daniel Martí 1db6e1e230
make -coverprofile include toolexec processes (#216)
testscript already included magic to also account for commands in the
total code coverage. That does not happen with plain tests, since those
only include coverage from the main test process.

The main problem was that, before, indirectly executed commands did not
properly save their coverage profile anywhere for testscript to collect
it at the end. In other words, we only collected coverage from direct
garble executions like "garble -help", but not indirect ones like "go
build -toolexec=garble".

	$ go test -coverprofile=cover.out
	PASS
	coverage: 3.6% of statements
	total coverage: 16.6% of statements
	ok  	mvdan.cc/garble	6.453s

After the delicate changes to testscript, any direct or indirect
executions of commands all go through $PATH and properly count towards
the total coverage:

	$ go test -coverprofile=cover.out
	PASS
	coverage: 3.6% of statements
	total coverage: 90.5% of statements
	ok  	mvdan.cc/garble	33.258s

Note that we can also get rid of our code to set up $PATH, since
testscript now does it for us.

goversion.txt needed minor tweaks, since we no longer set up $WORK/.bin.

Finally, note that we disable the reuse of $GOCACHE when collecting
coverage information. This is to do "full builds", as otherwise the
cached package builds would result in lower coverage.

Fixes #35.
3 years ago
Daniel Martí ba19a1d49c
do not try to obfuscate huge literals (#204)
It's common for asset bundling code generators to produce huge literals,
for example in strings. Our literal obfuscators are meant for relatively
small string-like literals that a human would write, such as URLs, file
paths, and English text.

I ran some quick experiments, and it seems like "garble build -literals"
appears to hang trying to obfuscate literals starting at 5-20KiB. It's
not really hung; it's just doing a lot of busy work obfuscating those
literals. The code it produces is also far from ideal, so it also takes
some time to finally compile.

The generated code also led to crashes. For example, using "garble build
-literals -tiny" on a package containing literals of over a megabyte,
our use of asthelper to remove comments and shuffle line numbers could
run out of stack memory.

This all points in one direction: we never designed "-literals" to deal
with large sizes. Set a source-code-size limit of 2KiB.

We alter the literals.txt test as well, to include a few 128KiB string
literals. Before this fix, "go test" would seemingly hang on that test
for over a minute (I did not wait any longer). With the fix, those large
literals are not obfuscated, so the test ends in its usual 1-3s.

As said in the const comment, I don't believe any of this is a big
problem. Come Go 1.16, most developers should stop using asset-bundling
code generators and use go:embed instead. If we wanted to somehow
obfuscate those, it would be an entirely separate feature.

And, if someone wants to work on obfuscating truly large literals for
any reason, we need good tests and benchmarks to ensure garble does not
consume CPU for minutes or run out of memory.

I also simplified the generate-literals test command. The only argument
that matters to the script is the filename, since it's used later on.

Fixes #178.
3 years ago
Daniel Martí 39372a8c9b testdata: don't let tests rely on rewriting mod files
In Go 1.15, if a dependency is required but not listed in go.mod/go.sum,
it's resolved and added automatically.

This is changing in 1.16. From that release, one will have to explicitly
update the mod files via 'go mod tidy' or 'go get'.

To get ahead of the curve, start using -mod=readonly to get the same
behavior in 1.15, and fix all existing tests.

The only tests that failed were imports.txt and syntax.txt, the only
ones to require other modules. But since we're here, let's add the 'go'
line to all go.mod files as well.
4 years ago
Nick d4eee0c9bc Replaced asthelper.Ident with ast.NewIdent
No point in having around a helper method that has been implemented for
us by `go/ast`
4 years ago
Daniel Martí 805c895d59 set up an AUTHORS file to attribute copyright
Many files were missing copyright, so also add a short script to add the
missing lines with the current year, and run it.

The AUTHORS file is also self-explanatory. Contributors can add
themselves there, or we can simply update it from time to time via
git-shortlog.

Since we have two scripts now, set up a directory for them.
4 years ago
lu4p 388ff7d1a4
remove buggy number literal obfuscation
Also remove boolean literal obfuscation.
4 years ago
Daniel Martí 98113d0124 properly skip non-build flags for 'go list'
If the flags list included ["-o" "binary"], we would properly skip "-o",
but we wouldn't skip "binary".

Thus, 'go list' would receive "binary" as the first argument, and assume
that's the first parameter and the end of the flags.

And add a unit test case.

Fixes #82, again.
4 years ago
Daniel Martí d0e01478f0 keep build flags when calling 'go list'
Otherwise any build flags like -tags won't be used, and we might easily
end up with errors or incorrect packages.

The common case with -tags is covered by one of the integration test
scripts. On top of that, we add a table-driven unit test to cover all
edge cases, since there are many we can do quickly in a unit test.

Fixes #82.
4 years ago
Daniel Martí 65461aabce reuse a single 'go list -json -export -deps' call
Instead of doing a 'go list' call every time we need to fetch a
dependency's export file, we now do a single 'go list' call before the
build begins. With the '-deps' flag, it gives us all the dependency
packages recursively.

We store that data in the gob format in a temporary file, and share it
with the future garble sub-processes via an env var.

This required lazy parsing of flags for the 'build' and 'test' commands,
since now we need to run 'go list' with the same package pattern
arguments.

Fixes #63.
4 years ago
pagran c2079ac0a1
Add test for literal obfuscators (#80)
* Combine literals-all-obfuscators.txt nad literals.txt
Rewrite literals.txt logic

* Remove unused \s

* Refactoring and add float ast helpers
4 years ago
Daniel Martí 846ddb4097
internal/literals: minor adjustments to the last commits (#77)
First, unindent some of the AST code.

Second, genRandInt is unused; delete it.

Third, genRandIntn is really just mathrand.Intn. Just use it directly.

Fourth, don't use inline comments if they result in super long lines.
4 years ago
lu4p 50d24cdf51 Add float, int, and boolean literal obfuscation.
Add ast helper functions to reduce ast footprint.

Add binsubfloat and binsubint functions for testing.

Fixes #55.
4 years ago
Pagran 0c5e0a8944 Fix 'A required privilege is not held by the client' on Windows 4 years ago
Daniel Martí 3e4f3821ea don't leak build version information via a const either
This requires a bit of extra magic to replace one constant in
runtime/internal/sys, but that was simple enough given that we can reuse
a lot of the code to parse the files and write them to a temporary dir.

We can also drop the -X flags, as runtime.buildVersion is based on the
constant that we replace here.

Fixes #44, again.
4 years ago
Daniel Martí ccd46404c0 improve binsubstr error messages a bit
By printing all the strings that failed at once, not just the first.
4 years ago
lu4p 0cf8d4e7a6
add seed flag to control how builds are reproducible
Fixes #26.
4 years ago
Daniel Martí c6643d37f9 simplify and tidy up the string obfuscation code
Mainly removing unnecessary indentation and newlines, but also other
minor things like making error handling a bit more consistent.
4 years ago
Daniel Martí 19e4c098cd make selection of packages configurable via GOPRIVATE
Carefully select a default that will do the right thing when inside a
module, as well as when building ad-hoc packages.

This means we no longer need to look at the compiler's -std flag, which
is nice.

Also replace foo.com/ with test/, as per golang/go#37641.

Fixes #7.
4 years ago
Daniel Martí d72c00eafd support building modules which require other modules
We use 'go list -json -export' to locate required modules. This works
fine to locate direct module dependencies; since we're building in the
current module, we run 'go list' in the correct directory.

However, if we're building one of those module dependencies, and it has
other module dependencies of its own, we would fail with cryptic errors
like:

	typecheck error: [...] go list error: updates to go.sum needed, disabled by -mod=readonly

This is because we would try to run 'go list' outside of the main
module, probably inside the module cache. Instead, use a $GARBLE_DIR env
var from the top-level 'garble build' call to always run 'go list' in
the original directory.

We add a few small modules to properly test this.

Updates #9.
4 years ago
Daniel Martí 308e984293 don't use regexes when searching binaries for strings
This is a bit simpler, and saves us a small amount of CPU work in the
tests.
4 years ago