This is in preparation for the switch to Go's cache package,
whose ActionID type is also a full sha256 hash with 32 bytes.
We were using "short" hashes as shown by `go tool buildid`,
since that was consistent and 15 bytes was generally enough.
First, rename "component" to "hash", since it's shorter and more useful.
A full build ID is two or four hashes joined with slashes.
Second, add sanity checks that buildIDHashLength is being followed.
Otherwise the use of []byte could lead to human error.
Third, move all the hash encoding and decoding logic together.
We first called the typecheck method, which starts filling cachedOutput
with information from the current package, and later we would load the
gob files for all dependencies via loadCachedOutputs.
This was a bit confusing; instead, load the cached gob files first,
and then do all the operations which fill information for curPkg.
Similarly, we were waiting until the very end of transformCompile to
write curPkg's cachedOutput gob file to the disk cache.
We can write the file at an earlier point, before we have obfuscated and
re-printed all Go files for the current package.
We can also write the file before other work like processImportCfg.
None of these changes should affect garble's behavior,
but they will make the cache redesign for #708 easier.
See the added comment and test, which failed before the fix when
the encoded certificate was decoded.
It took a while to find the culprit in asn1, but in hindsight it's easy:
case reflect.Slice:
if t.Elem().Kind() == reflect.Uint8 {
return false, TagOctetString, false, true
}
if strings.HasSuffix(t.Name(), "SET") {
return false, TagSet, true, true
}
return false, TagSequence, true, true
Fixes#711.
printOneCgoTraceback now returns a boolean rather than an int.
Since we need to have different logic based on the Go version,
and toolchainVersionSemver was only set for the main process,
move the string to the shared cache global.
This is a nice thing to do anyway, to reduce the number of globals.
While here, update actions/setup-go to v4, which starts caching
GOMODCACHE and GOCACHE by default now.
Disable it, because it still doesn't help in our case,
and GitHub's Actions caching is still really inefficient.
And update staticcheck too.
The seedFlag.random field had never worked,
as my refactor in December 2021 never set it to true.
Even if the boolean was working, we only printed the random seed
when we failed. It's still useful to see it when a build succeeds,
for example when wanting to reproduce the same binary
or when wanting to reverse a panic from the produced binary.
Add a test this time.
Fixes#696.
Added in Go 1.19, types like sync/atomic.Uint64 are handy,
because they ensure proper alignment even on 32-bit GOOSes.
However, this was done via a magic `type align64 struct{}`,
which the compiler spotted by name.
To keep that magic working, do not obfuscate the name.
Neither package path was being obfuscated,
as both packages contain compiler intrinsics already.
Fixes#686.
We're building the linker binary for the host GOOS,
not the target GOOS that we happen to be building for.
I noticed that, after running `go test`, my garble cache
would contain both link and link.exe, which made no sense
as I run linux and not windows.
`go env` has GOHOSTOS to mirror GOOS, but there is no
GOHOSTEXE to mirror GOEXE, so we reconstruct it from
runtime.GOOS, which is equivalent to GOHOSTOS.
Add a regression test as well.
When we use `go list` on the standard library, we need to be careful
about what flags are passed from the top-level build command,
because some flags are not going to be appropriate.
In particular, GOFLAGS=-modfile=... resulted in a failure,
reproduced via the GOFLAGS variable added to linker.txtar:
go: inconsistent vendoring in /home/mvdan/tip/src:
golang.org/x/crypto@v0.5.1-0.20230203195927-310bfa40f1e4: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
golang.org/x/net@v0.7.0: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
golang.org/x/sys@v0.5.1-0.20230208141308-4fee21c92339: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
golang.org/x/text@v0.7.1-0.20230207171107-30dadde3188b: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
To ignore the vendor directory, use -mod=readonly or -mod=mod.
To sync the vendor directory, run:
go mod vendor
To work around this problem, reset the -mod and -modfile flags when
calling "go list" on the standard library, as those are the only two
flags which alter how we load the main module in a build.
The code which builds a modified cmd/link has a similar problem;
it already reset GOOS and GOARCH, but it could similarly run into
problems if other env vars like GOFLAGS were set.
To be on the safe side, we also disable GOENV and GOEXPERIMENT,
which we borrow from Go's bootstrapping commands.
At linker stage, we now encrypt funcInfo.entryoff value with a simple algorithm (1 xor + 1 mul).
This makes it harder to relate function metadata (e.g. name) to function itself in binary, almost without affecting performance.
The test package hack still appears to be needed as of Go 1.20.
Interestingly, the cgo filename check does not trigger at all anymore.
Presumably this means that it's not needed at all, and obfuscating code
generated by cgo appears to be fine. Go with that for now.
It assumes that reflect.Value has a field named "flag",
which wasn't the case with obfuscated builds as we obfuscated it.
We already treated the reflect package as special,
for instance when not obfuscating Method or MethodByName.
In a similar fashion, mark reflect's rtype and Value types to not be
obfuscated alongside their field names. Note that rtype is the
implementation behind the reflect.Type interface.
This fix is fairly manual and repetitive.
transformCompile, transformLinkname, and transformAsm should all
use the same mechanism to tell if names should be obfuscated.
However, they do not do that right now, and that refactor feels too
risky for a bugfix release. We add more TODOs instead.
We're not adding go-spew to scripts/check-third-party.sh since the
project is largely abandoned. It's not even a Go module yet.
The only broken bit from it is what we've added to our tests.
Fixes#676.
Go contains such inline comments, like:
crypto/aes/gcm_ppc64x.s:129: VPMSUMD IN, HL, XL // H.lo·H.lo
We didn't notice since these comments were somewhat rare.
While here, "//" does not need to be followed by a space.
The code turns out to be pretty easy with strings.Cut.
Fixes#672.
`go help build` now has -C, -cover, -coverpkg, and -pgo.
There are no new boolean flags, but note that -cover is now a common
build flag rather than just a test flag.
While here, sort the lists so that they are easier to skim for missing
items in the future.
Go master, the upcoming Go 1.21, has had its merge window open for over
two weeks at this point, and it seems calmer at this point.
ALso update staticcheck to its latest release, which supports Go 1.20.
This is not common, but it is done by a few projects.
Namely, github.com/goccy/go-json reached into reflect's guts,
which included a number of methods:
internal/runtime/rtype.go
11://go:linkname rtype_Align reflect.(*rtype).Align
19://go:linkname rtype_FieldAlign reflect.(*rtype).FieldAlign
27://go:linkname rtype_Method reflect.(*rtype).Method
35://go:linkname rtype_MethodByName reflect.(*rtype).MethodByName
[...]
Add tests for such go:linkname directives pointing at methods.
Note that there are two possible symbol string variants;
"pkg/path.(*Receiver).method" for methods with pointer receivers,
and "pkg/path.Receiver.method" for the rest.
We can't assume that the presence of two dots means a method either.
For example, a package path may be "pkg/path.with.dots",
and so "pkg/path.with.dots.SomeFunc" is the function "SomeFunc"
rather than the method "SomeFunc" on a type "dots".
To account for this ambiguity, rather than splitting on the last dot
like we used to, try to find a package path prefix by splitting on an
increasing number of first dots.
This can in theory still be ambiguous. For example,
we could have the package "pkg/path" expose the method "foo.bar",
and the package "pkg/path.foo" expose the func "bar".
Then, the symbol string "pkg/path.foo.bar" could mean either of them.
However, this seems extremely unlikely to happen in practice,
and I'm not sure that Go's toolchain would support it either.
I also noticed that goccy/go-json still failed to build after the fix.
The reason was that the type reflect.rtype wasn't being obfuscated.
We could, and likely should, teach our assembly and linkname
transformers about which names we chose not to obfuscate due to the use
of reflection. However, in this particular case, reflect's own types
can be obfuscated safely, so just do that.
Fixes#656.
Declare the ast.GenDecl node once,
which allows us to deduplicate and inline code.
resolveImportSpec doesn't need to be separate either.
We also don't need explicit panics for unexpected nils;
we can rely on nil checks inserted by the compiler.
Finally, move the bulk of the code outside of the scope.Names loop,
which unindents most of the logic.
While here, add a few more inline docs.
garble's -literals flag and its patching of the runtime may leave unused imports.
We used to try to detect those and remove the imports,
but that was still buggy with edge cases like dot imports or renamed imports.
Moreover, it was potentially incorrect.
Completely removing an import from a package means we don't run its init funcs,
which could have side effects changing the behavior of a program.
As an example, database/sql drivers are registered at init time.
Instead, for each import in an obfuscated Go file,
add an unnamed declaration which references the imported package.
This may not be necessary for all imported packages,
as only a minority become unused due to garble,
but it's also relatively harmless to do so.
Fixes#658.
We obfuscate import paths as well as their declared names.
The compiler treats some packages and APIs in special ways,
and the way it detects those is by looking at import paths and names.
In the past, we have avoided obfuscating some names like embed.FS or
reflect.Value.MethodByName for this reason. Otherwise,
go:embed or the linker's deadcode elimination might be broken.
This matching by path and name also happens with compiler intrinsics.
Intrinsics allow the compiler to rewrite some standard library calls
with small and efficient assembly, depending on the target GOARCH.
For example, math/bits.TrailingZeros32 gets replaced with ssa.OpCtz32,
which on amd64 may result in using the TZCNTL instruction.
We never noticed that we were breaking many of these intrinsics.
The intrinsics for funcs declared in the runtime and its dependencies
still worked properly, as we do not obfuscate those packages yet.
However, for other packages like math/bits and sync/atomic,
the intrinsics were being entirely disabled due to obfuscated names.
Skipping intrinsics is particularly bad for performance,
and it also leads to slightly larger binaries:
│ old │ new │
│ bin-B │ bin-B vs base │
Build-16 5.450Mi ± ∞ ¹ 5.333Mi ± ∞ ¹ -2.15% (p=0.029 n=4)
Finally, the main reason we noticed that intrinsics were broken
is that apparently GOARCH=mips fails to link without them,
as some symbols end up being not defined at all.
This patch fixes builds for the MIPS family of architectures.
Rather than building and linking all of std for every GOARCH,
test that intrinsics work by asking the compiler to print which
intrinsics are being applied, and checking that math/bits gets them.
This fix is relatively unfortunate, as it means we stop obfuscating
about 120 function names and a handful of package paths.
However, fixing builds and intrinsics is much more important.
We can figure out better ways to deal with intrinsics in the future.
Fixes#646.
Go's package runtime/internal/atomic contains references to functions
which refer to the current package by its package name alone:
src/runtime/internal/atomic/atomic_loong64.s: JMP atomic·Load(SB)
src/runtime/internal/atomic/atomic_loong64.s: JMP atomic·Load64(SB)
src/runtime/internal/atomic/atomic_loong64.s: JMP atomic·Load64(SB)
src/runtime/internal/atomic/atomic_mips64x.s: JMP atomic·Load(SB)
src/runtime/internal/atomic/atomic_mips64x.s: JMP atomic·Load64(SB)
src/runtime/internal/atomic/atomic_mips64x.s: JMP atomic·Load64(SB)
We could only handle unqualified or fully qualified references, like:
JMP ·Load64(SB)
JMP runtime∕internal∕atomic·Load64(SB)
Apparently, all three forms are equally valid.
Add a test case and fix it.
I checked whether referencing an imported package by its name worked;
it does not seem to be the case.
This feature appears to be restricted to the current package alone.
While here, we only need goPkgPath when we need to call listPackage.
Fixes#619.
To detect if an object is global, our previous code did:
obj.Pkg().Scope().Lookup(obj.Name()) == obj
However, that Lookup call is unnecessary.
It is a map lookup, which is cheap, but not free.
Instead, we can compare the scopes directly:
obj.Pkg().Scope() == obj.Parent()
While here, reuse the calls to obj.Pkg(),
and avoid using fmt.Sprintf in this hot path.
Looked here since perf on Linux with a garble build
reports that we spend about 2.6% of CPU time in recordedObjectString.
Overall provides a very minor improvement:
name old time/op new time/op delta
Build-16 21.8s ± 1% 21.7s ± 0% ~ (p=0.200 n=4+4)
name old bin-B new bin-B delta
Build-16 5.67M ± 0% 5.67M ± 0% ~ (all equal)
name old cached-time/op new cached-time/op delta
Build-16 734ms ± 3% 722ms ± 3% ~ (p=0.486 n=4+4)
name old mallocs/op new mallocs/op delta
Build-16 25.0M ± 0% 24.7M ± 0% -1.28% (p=0.029 n=4+4)
name old sys-time/op new sys-time/op delta
Build-16 17.0s ± 1% 16.8s ± 1% ~ (p=0.200 n=4+4)
github.com/bytedance/sonic has many comments in its assembly files,
and one in particular caused us trouble:
internal/rt/asm_amd64.s:2:// Code generated by asm2asm, DO NOT EDIT·
Since we looked for "middle dot" characters anywhere,
we would try to load the package "EDIT", which clearly does not exist.
To fix that, start skipping over comments. Thankfully,
Go's assembly syntax only supports "//" line comments,
so it's enough to read one line at a time and skip them quickly.
We also longer need a regular expression to find #include lines.
While here, two other minor bits of cleanup.
First, rename transformGo to transformGoFile, for clarity.
Second, use ".s" extensions for obfuscated assembly filenames,
just like we do with the comiler and ".go" files.
Updates #621.
The patch to the linker does this when generating the pclntab,
which is the binary section containing func names.
When `-tiny` is being used, we look for unexported funcs,
and set their names to the offset `0` - a shared empty string.
We also avoid including the original name in the binary,
which saves a significant amount of space.
The following stats were collected on GOOS=linux,
which show that `-tiny` is now about 4% smaller:
go build 1203067
garble build 782336
(old) garble -tiny build 688128
(new) garble -tiny build 659456
The test case is lifted from github.com/bytedance/sonic/internal/native,
which had the following line:
JMP github·com∕bytedance∕sonic∕internal∕native∕avx2·__quote(SB)
Updates #621.
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.
We were doing too much work for tools we don't need to wrap at all,
such as `go tool pack` or `go tool buildid`, which get run about as
often as the compiler does.
The overhead is small per call, but it adds up.
name old time/op new time/op delta
Build-16 20.7s ± 0% 20.5s ± 1% ~ (p=0.057 n=4+4)
name old bin-B new bin-B delta
Build-16 5.67M ± 0% 5.66M ± 0% -0.07% (p=0.029 n=4+4)
name old cached-time/op new cached-time/op delta
Build-16 707ms ± 0% 705ms ± 2% ~ (p=0.886 n=4+4)
name old mallocs/op new mallocs/op delta
Build-16 25.0M ± 0% 24.9M ± 0% -0.26% (p=0.029 n=4+4)
name old sys-time/op new sys-time/op delta
Build-16 8.32s ± 2% 7.90s ± 3% -5.05% (p=0.029 n=4+4)
The logic
if X || Y {
if X {
<X panic>
}
<Y panic>
}
is equivalent to, and easier to read as:
if X {
<X panic>
}
if Y {
<Y panic>
}
It probably evolved unintentionally to be this way.
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.
We were obfuscating reflect's package path and its declared names,
but the toolchain wants to detect the presence of method reflection
to turn down the aggressiveness of dead code elimination.
Given that the obfuscation broke the detection,
we could easily end up in crashes when making reflect calls:
fatal error: unreachable method called. linker bug?
goroutine 1 [running]:
runtime.throw({0x50c9b3?, 0x2?})
runtime/panic.go:1047 +0x5d fp=0xc000063660 sp=0xc000063630 pc=0x43245d
runtime.unreachableMethod()
runtime/iface.go:532 +0x25 fp=0xc000063680 sp=0xc000063660 pc=0x40a845
runtime.call16(0xc00010a360, 0xc00000e0a8, 0x0, 0x0, 0x0, 0x8, 0xc000063bb0)
runtime/wcS9OpRFL:728 +0x49 fp=0xc0000636a0 sp=0xc000063680 pc=0x45eae9
runtime.reflectcall(0xc00001c120?, 0x1?, 0x1?, 0x18110?, 0xc0?, 0x1?, 0x1?)
<autogenerated>:1 +0x3c fp=0xc0000636e0 sp=0xc0000636a0 pc=0x462e9c
Avoid obfuscating the three names which cause problems: "reflect",
"Method", and "MethodByName".
While here, we also teach obfuscatedImportPath to skip "runtime",
as I also saw that the toolchain detects it for many reasons.
That wasn't a problem yet, as we do not obfuscate the runtime,
but it was likely going to become a problem in the future.
We can drop the code that kicked in when GOGARBLE was empty.
We can also add the value in addGarbleToHash unconditionally,
as we never allow it to be empty.
In the tests, remove all GOGARBLE lines where it just meant "obfuscate
everything" or "obfuscate the entire main module".
cgo.txtar had "obfuscate everything" as a separate step,
so remove it entirely.
linkname.txtar started failing because the imported package did not
import strings, so listPackage errored out. This wasn't a problem when
strings itself wasn't obfuscated, as transformLinkname silently left
strings.IndexByte untouched. It is a problem when IndexByte does get
obfuscated. Make that kind of listPackage error visible, and fix it.
reflect.txtar started failing with "unreachable method" runtime throws.
It's not clear to me why; it appears that GOGARBLE=* makes the linker
think that ExportedMethodName is suddenly unreachable.
Work around the problem by making the method explicitly reachable,
and leave a TODO as a reminder to investigate.
Finally, gogarble.txtar no longer needs to test for GOPRIVATE.
The rest of the test is left the same, as we still want the various
values for GOGARBLE to continue to work just like before.
Fixes#594.
Up to this point, -debugdir only included obfuscated Go files.
Include assembly files and their headers as well.
While here, ensure that -debugdir supports the standard library too,
and that it behaves correctly with build tags as well.
Also use more consistent names for path strings, to clarify which are
only the basename, and which are the obfuscated version.
Some big changes landed in Go for the upcoming 1.20.
While here, remove the use of GOGC=off with make.bash,
as https://go.dev/cl/436235 makes that unnecessary now.
We were still leaking the filenames for assembly files.
In our existing asm.txtar test's output binary,
the string `test/main/garble_main_amd64.s` was present.
This leaked full import paths on one hand,
and the filenames of each assembly file on the other.
We avoid this in Go files by using `/*line` directives,
but those are not supported in assembly files.
Instead, obfuscate the paths in the temporary directory.
Note that we still need a separate temporary directory per package,
because otherwise any included header files might collide.
We must remove the `main` package panic in obfuscatedImportPath,
as we now need to use that function for all packages.
While here, remove the outdated comment about `-trimpath`.
Fixes#605.
A main package can be imported in one edge case we weren't testing:
when the same main package has any tests, which implicitly import main.
Before the fix, we would panic:
> garble test -v ./...
# test/bar/somemaintest.test
panic: main packages should never need to obfuscate their import paths
One more package that further unblocks obfuscating the runtime.
The issue was the TODO we already had about go:linkname directives with
just one argument, which are used in the syscall package.
While here, factor out the obfuscation of linkname directives into
transformLinkname, as it was starting to get a bit complex.
We now support debug logging as well, while still being able to use
"early returns" for some cases where we bail out.
We also need listPackage to treat all runtime sub-packages like it does
runtime itself, as `runtime/internal/syscall` linknames into `syscall`
without it being a dependency as well.
Finally, add a regression test that, without the fix,
properly spots that the syscall package was not obfuscated:
FAIL: testdata/script/gogarble.txtar:41: unexpected match for ["syscall.RawSyscall6"] in out
Updates #193.
This failed at link time because transformAsm did not know how to handle
the fact that the runtime package's assembly code implements the
`time.now` function via:
TEXT time·now<ABIInternal>(SB),NOSPLIT,$16-24
First, we need transformAsm to happen for all packages, not just the
ones that we are obfuscating. This is because the runtime can implement
APIs in other packages which are themselves obfuscated, whereas runtime
may not itself be getting obfuscated. This is currently the case with
`GOGARBLE=*` as we do not yet support obfuscating the runtime.
Second, we need to teach replaceAsmNames to handle qualified names with
import paths. Not just to look up the right package information for the
name, but also to obfuscate the package path if necessary.
Third, we need to relax the Deps requirement on listPackage, since the
runtime package and its dependencies are always implicit dependencies.
This is a big step towards being able to obfuscate the runtime, as there
is now just one package left that we cannot obfuscate outside the runtime.
Updates #193.
Assembly files can include header files within the same Go module,
and those header files can include "defines" which refer to Go names.
Since those Go names are likely being obfuscated,
we need to replace them just like we do in assembly files.
The added mechanism is rather basic; we add two TODOs to improve it.
This should help when building projects like go-ethereum.
Fixes#553.
This way, the child process knows that it's running a toolchain command
via -toolexec without having to guess via filepath.IsAbs.
While here, improve the docs and tests a bit.
I was wrongly assumed that, if `used` has an `Elem` method,
then `origin` must too. But it does not if it's a type parameter.
Add a test case too, which panicked before the fix.
Fixes#577.
While here, start the changelog for the upcoming release,
which will likely be a bugfix release as it's a bit early to drop 1.18.
We also bump staticcheck to get a version that supports 1.19.
I also noticed the "Go version X or newer" messages were slightly weird
and inconsistent. Our policy, per the README, is "Go version X or newer",
so the errors given to the user were unnecessarily confusing.
For example, now that Go 1.19 is out, we shouldn't simply recommend that
they upgrade to 1.18; we should recommend 1.18 or later.
That is, since Go 1.18.1, released back in April 2022.
We no longer need to worry about the buggy Go 1.18.0.
While here, use a clearer env var name; the settings are build settings.
We obfuscate import paths in import declarations like:
"domain.com/somepkg"
by replacing them with the obfuscated package path:
somepkg "HPS4Mskq"
Note how we add a name to the import if there wasn't one,
so that references like somepkg.Foo keep working in the code.
This could break in some edge cases involving comments between imports
in the Go code, because go/printer is somewhat brittle with positions:
> garble build -tags buildtag
[stderr]
# test/main/importedpkg
:16: syntax error: missing import path
exit status 2
exit status 2
To prevent that, ensure the name has a reasonable position.
This was preventing github.com/gorilla/websocket from being obufscated.
It is a fairly popular library in Go, but we don't add it to
scripts/check-third-party.sh for now as wireguard already gives us
coverage over networking and cryptography.
These lines get executed for every identifier in every package in each
Go build, so one allocation per log.Printf call can quickly add up to
millions of allocations across a build.
Until https://go.dev/issue/53465 is fixed, the best way to avoid the
escaping due to `...any` is to not perform the function call at all.
name old time/op new time/op delta
Build-16 10.5s ± 1% 10.5s ± 2% ~ (p=0.604 n=9+10)
name old bin-B new bin-B delta
Build-16 5.52M ± 0% 5.52M ± 0% ~ (all equal)
name old cached-time/op new cached-time/op delta
Build-16 506ms ±13% 500ms ± 7% ~ (p=0.739 n=10+10)
name old mallocs/op new mallocs/op delta
Build-16 31.7M ± 0% 30.1M ± 0% -5.33% (p=0.000 n=10+9)
name old sys-time/op new sys-time/op delta
Build-16 5.70s ± 5% 5.78s ± 6% ~ (p=0.278 n=9+10)
There used to be a reason to keep these maps separate, but ever since we
became better at obfuscating the standard library, that has gone away.
It's still a good idea to keep `go list -deps runtime` as a group,
but we can do that via a comment inside a joint map literal.
I also noticed that one comment still referred to cannotObfuscateNames,
which hasn't existed for some time. Fix that up.
It's also not documented how cachedOutput contains info for all deps,
so clarify that while we're improving the docs.
Finally, the reason we cannot obfuscate the syscall package was out of
date; it's not part of the runtime. It is a go:linkname bug.
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.
Reuse a buffer and a map across loop iterations, because we can.
Make recordTypeDone only track named types, as that is enough to detect
type cycles. Without named types, there can be no cycles.
These two reduce allocs by a fraction of a percent:
name old time/op new time/op delta
Build-16 10.4s ± 2% 10.4s ± 1% ~ (p=0.739 n=10+10)
name old bin-B new bin-B delta
Build-16 5.51M ± 0% 5.51M ± 0% ~ (all equal)
name old cached-time/op new cached-time/op delta
Build-16 391ms ± 9% 407ms ± 7% ~ (p=0.095 n=10+9)
name old mallocs/op new mallocs/op delta
Build-16 34.5M ± 0% 34.4M ± 0% -0.12% (p=0.000 n=10+10)
name old sys-time/op new sys-time/op delta
Build-16 5.87s ± 5% 5.82s ± 5% ~ (p=0.182 n=10+9)
It doesn't seem like much, but remember that these stats are for the
entire set of processes, where garble only accounts for about 10% of the
total wall time when compared to the compiler or linker. So a ~0.1%
decrease globally is still significant.
linkerVariableStrings is also indexed by *types.Var rather than types.Object,
since -ldflags=-X only supports setting the string value of variables.
This shouldn't make a significant difference in terms of allocs,
but at least the map is less prone to confusion with other object types.
To ensure the new code doesn't trip up on non-variables, we add test cases.
Finally, for the sake of clarity, index into the types.Info maps like
Defs and Uses rather than calling ObjectOf if we know whether the
identifier we have is a definition of a name or the use of a defined name.
This isn't better in terms of performance, as ObjectOf is a tiny method,
but just like with linkerVariableStrings before, the new code is clearer.
The _gomod_.go file inserted by the Go toolchain no longer shows up;
it's likely that either the -trimpath or -buildvcs=false flags are
preventing that extra bit of work from happening entirely.
The modinfo.txt test ensures that we're not breaking,
and the inner lines of code weren't hit as part of `go test`.
It also appears that we don't need to avoid obfuscating functions
defined with an `//export` directive. This is likely because cgo runs as
a pre-process step compared to the compiler, so us removing the
directive later does not make a difference.
We might need to revisit this in the future if we implement obfuscating
Go code instead of builds, e.g. `garble export`.
Just in case, I've expanded the cgo.txt test to also include one more
kind of cgo integration: an "import C" block including a C header file.
Either of these changes are slightly risky, as our tests don't cover all
edge cases. We've just done a release, so now is the time to try them.
We don't use go/ast.Objects, as we use go/types instead.
Avoiding this work saves a bit of CPU and memory allocs.
name old time/op new time/op delta
Build-16 10.2s ± 1% 10.2s ± 1% ~ (p=0.937 n=6+6)
name old bin-B new bin-B delta
Build-16 5.47M ± 0% 5.47M ± 0% ~ (all equal)
name old cached-time/op new cached-time/op delta
Build-16 328ms ±14% 321ms ± 6% ~ (p=0.589 n=6+6)
name old mallocs/op new mallocs/op delta
Build-16 34.8M ± 0% 34.0M ± 0% -2.26% (p=0.010 n=6+4)
name old sys-time/op new sys-time/op delta
Build-16 5.89s ± 3% 5.89s ± 3% ~ (p=0.937 n=6+6)
See golang/go#52463.
It appears that we already support obfuscating them,
and nothing seems to break when they are pulled in.
While here, add runtime/internal/syscall to runtimeAndDeps.
It first appeared in Go 1.18, but we missed adding it.
It seems like not having it there didn't cause any issues,
which makes sense given it's got almost zero Go code.
We also teach garble about the -work boolean build flag,
which has existed for multiple years but we forgot about.
It's likely that noone noticed as it's a rarely used flag.
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.
First, I tried to follow my own past advice to only set GarbleActionID
if ToObfuscate is true. However, that broke at least three parts of
transformCompile, as the hash is used for more than I recalled.
Give up on that idea, because the current code is working as intended.
Better document what GarbleActionID is and what we use it for.
Second, now that https://go.dev/cl/348741 was shipped with Go 1.18,
using the logger when its output is io.Discard is already a no-op.
So we no longer need our debugf wrapper to apply the no-op logic.
When splitFlagsFromFiles saw "-p foo/bar.go",
it thought that was the first Go file, when in fact it's not.
We didn't notice because such import paths are pretty rare,
but they do exist, like github.com/nats-io/nats.go.
Before the fix, the added test case fails as expected:
> garble build -tags buildtag
[stderr]
# test/main/goextension.go
open test/main/goextension.go: no such file or directory
We could go through the trouble of teaching splitFlagsFromFiles about
all of the flags understood by the compiler and linker, but that feels
like far more code than the small alternative we went with.
And I'm pretty sure the alternative will work pretty reliably for now.
Fixes#539.
Trying to make Go master work, I noticed that crypto/tls still failed to
build. The reason was generic structs; we would badly obfuscate their
field names when the types are instantiated:
> garble build
[stderr]
# test/main
Z4ZpcbMj.go:4: unknown field 'FOpszkrN' in struct literal of type SYdpWfK5[string]
Z4ZpcbMj.go:5: m8hLTotb.FypXrbTd undefined (type SYdpWfK5[string] has no field or method FypXrbTd)
exit status 2
See the added comment for what happened and how we fixed it. And add tests.
The proposal at https://go.dev/issue/50603 has been approved,
so Go will at some point start producing module pseudo-versions
even if the main module was built from a VCS clone.
To not wait until a future release like Go 1.20,
implement that ourselves with the help of module.PseudoVersion.
The result is a friendlier version output; what used to be
$ go install && garble version
mvdan.cc/garble (devel)
Build settings:
[...]
will now look like
$ go install && garble version
mvdan.cc/garble v0.0.0-20220505210747-22e3d30216be
Build settings:
[...]
Note that we don't use VCS tags in any way, so the prefix is hard-coded
as v0.0.0. That seems fine for development builds, and Go doesn't embed
VCS tag information in binaries anyway.
Finally, note that we start printing the module sum, as it's redundant.
The VCS commit hash, at least in git, should be unique enough.
We don't use sub-matches captured by these groups,
so avoiding that extra work will save some CPU cycles.
It is likely insignificant compared to the rest of a Go build,
but it's a very easy little win.
The added test case reproduces the failure if we uncomment the added
"continue" line in processImportCfg:
# test/bar/exporttest [test/bar/exporttest.test]
panic: refusing to list non-dependency package: test/bar/exporttest
goroutine 1 [running]:
mvdan.cc/garble.processImportCfg({0xc000166780?, 0xc0001f4a70?, 0x2?})
/home/mvdan/src/garble/main.go:983 +0x58b
mvdan.cc/garble.transformCompile({0xc000124020?, 0x11?, 0x12?})
/home/mvdan/src/garble/main.go:736 +0x338
It seems like a quirk of cmd/go that it includes a redundant packagefile
line in this particular edge case, but it's generally harmless for "go
build". For "garble build" it's also harmless in principle, but in
practice we had sanity checks that got upset by the unexpected line.
For now, notice the edge case and ignore it.
Fixes#522.
Returning a json or gob error directly to the user is generally not
helpful, as it lacks any form of context.
For example, from the json unmarshal of "go env -json":
$ garble build
invalid character 'w' looking for beginning of value
Also improve the error when the user ran garble in the wrong way,
resulting in no shared gob file. The context is now shorter, and we also
include the os.Open error in case it contains any useful details.
While here, apply Go tip's gofmt, which reformatted a godoc list.
For #523.
When a function definition is variadic,
the number of parameters may not match the number of calling arguments.
Our existing code was a bit naive with this edge case,
leading to a panic when indexing call.Args.
Keep track of that information, and properly handle all variadic
arguments that may be used indirectly with reflection.
We add a test case that used to panic, where 0 arguments are used for a
variadic parameter, as well as a case where we need to disable
obfuscation for a Go type used as the second variadic argument rather
than the first.
Finally, the old code in findReflectFunctions looked at Go function
types from the point of view of go/ast.FuncType.
Use go/types.Signature instead, where we don't need to deal with the
grouping of variables in the original syntax, and which is more
consistent with the rest of the garble codebase.
Fixes#524.
When someone builds garble from a git clone,
the resulting binary used to not contain any information:
$ garble version
(devel)
Since Go 1.18, VCS information is stamped by default into binaries.
We now print it, alongside any other available build settings:
$ garble version
mvdan.cc/garble (devel)
Build settings:
-compiler gc
CGO_ENABLED 1
GOARCH amd64
GOOS linux
GOAMD64 v3
vcs git
vcs.revision 91ea246349
vcs.time 2022-03-18T13:45:11Z
vcs.modified true
Note that it's still possible for a garble build to contain no useful
version information, such as when built via "go build -buildvcs=false".
However, if a user opts into omitting the information, it's on them to
figure out what version of garble they actually built.
While here, bump test-gotip.
Fixes#491.
strings.Cut makes some string handling code more intuitive.
Note that we can't use it everywhere, as some places need LastIndexByte.
Start using x/exp/slices, too, which is our first use of generics.
Note that its API is experimental and may still change,
but since we are not a library, we can control its version updates.
I also noticed that we were using TrimSpace for importcfg files.
It's actually unnecessary if we swap strings.SplitAfter for Split,
as the only whitespace present was the trailing newline.
While here, I noticed an unused copy of printfWithoutPackage.
Now that we've released v0.6.0, that will be the last feature release to
feature support for Go 1.17. The upcoming v0.7.0 will be Go 1.18+.
Code-wise, the cleanup here isn't super noticeable,
but it will be easier to work on features like VCS-aware version
information and generics support without worrying about Go 1.17.
Plus, now CI is back to being much faster.
Note how "go 1.18" in go.mod makes "go mod tidy" more aggressive.
I hadn't noticed that cmd/bundle prints its own go:generate directive.
I guess that makes sense for the average user running it directly,
but that doesn't apply to us, and we end up with duplicate directives.
Before:
$ go generate -n
bundle -o cmdgo_quoted.go -prefix cmdgoQuoted cmd/internal/quoted
go run golang.org/x/tools/cmd/bundle@v0.1.9 -o cmdgo_quoted.go -prefix cmdgoQuoted cmd/internal/quoted
After:
$ go generate -n
go run golang.org/x/tools/cmd/bundle@v0.1.9 -o cmdgo_quoted.go -prefix cmdgoQuoted cmd/internal/quoted
sed -i /go:generate/d cmdgo_quoted.go
While here, I made a typo in the last release notes, because of course.
I already edited that out in the GitHub release.
For instance, Go 1.18 added support for generics, so its compiler output
files changed format to accomodate for the new language feature.
If garble is built with Go 1.17 and then used to perform builds on Go
1.18, it will fail in a very confusing way, because garble's go/types
and go/importer packages will not know how to deal with that.
As already discussed in #269, require the version that built the garble
binary to be equal or newer. In that thread we discussed only comparing
the major version, so for example garble built on go1.18 could be used
on the toolchain go1.18.5. However, that could still fail in confusing
ways if a fix to go/types or go/importer happened in a point release.
While here, I noticed that we were still using Go 1.17 for some CI
checks. Fix that, except for staticcheck.
Fixes#269.
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%.
We don't need to nest the RemoveAll and MkdirAll calls for debugdir.
Fill the string for -toolexec= when we actually append it to goArgs.
Consistently use the objectString type alias.
Remove unnecessary parameter names in transformFuncs.
Unindent the code for switch variables by reversing the conditional.
This will hopefully give new contributors a place to start.
Some of these, like verifying that the "help" commands work,
will be relatively simple additions to our test scripts.
The added comment in main.go explains the situation in detail.
The added test is a minimization of the scenario, which failed:
> cd mod1
> garble -seed=${SEED1} build -v gopkg.in/garbletest.v2
> cd ../mod2
> garble -seed=${SEED1} build -v
[stderr]
test/main/mod2
# test/main/mod2
cannot load garble export file for gopkg.in/garbletest.v2: open […]/go-build/ed/[…]-garble-ZV[…]-d: no such file or directory
To work around the problem, we'll always add each package's
GarbleActionID to its build artifact, even when not using -seed.
This will get us the previous behavior with regards to the build cache,
meaning that we fix the recent regression.
The added variable doesn't make it to the final binary either.
While here, improve the cached file loading error's context,
and add an extra sanity check for duplicates on ListedPackages.
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.
This code initially used os.Stat, where it made sense to use
os.IsNotExist to catch whether the directory didn't exist.
However, we later transitioned to os.RemoveAll, which will never return
neither ErrExist nor ErrNotExist:
If the path does not exist, RemoveAll returns nil (no error).
Simplify the code.
I spent some time trying to reproduce the bug but couldn't,
so for now, make a detailed note for it.
We can come back to it if we actually run into it in the future.
Fixes#492.
In particular, using -ldflags with -
In particular, a command like:
garble -literals build -ldflags='-X "main.foo=foo bar"'
would fail, because we would try to use "\"main" as the package name for
the -X qualified name, with the leading quote character.
This is because we used strings.Split(ldflags, " ").
Instead, use the same quoted.Split that cmd/go uses,
copied over thanks to x/tools/cmd/bundle and go:generate.
Updates #492.
We used to record all objects in cannotObfuscateNames,
and then we'd add the exported ones to KnownCannotObfuscate.
Instead, teach recordAsNotObfuscated to store each object in either
knownCannotObfuscateUnexported or KnownCannotObfuscate, but not both.
The former isn't cached so it uses in-memory pointers as keys,
and the latter uses the cross-process objectStrings like before.
Functionally, this is all the same, but with the difference that the map
indexed by types.Object will not contain objects already recorded in
KnownCannotObfuscate, reducing the amount of duplicate memory use.
While here, give recordIgnore a less ambiguous name,
and remove the second parameter as it was always tf.pkg.Path().
This also means we can compare *types.Package pointers directly.
Finally, add more TODOs for further improvement ideas.
It does mean that we end up with more TODOs than before,
even though I'm fixing one, but I reckon that's a good thing.
Recording these ideas can give first-time contributors ways to help,
and it ensures I don't forget about ideas just in my head.
Back in the day, we used to call toObfuscate anytime we needed to know
whether a package should be obfuscated.
More recently, we started computing via the ToObfuscate field,
which then gets shared with all sub-processes via sharedCache.
We still had two places that directly called toObfuscate.
Replace those with ToObfuscate, and inline toObfuscate into shared.go.
obfuscatedImportPath is also a potential footgun for main packages.
Some use cases always want the original "main" package name,
such as for use in the compiler's "-p main" flag,
while other cases want the obfuscated package import path,
such as the entries in importcfg files.
Since each of these call sites handles the edge case well,
obfuscatedImportPath now panics on main packages to avoid any misuse.
Finally, test that we never leak main package paths via ldflags.txt.
We never did, but it's good to make sure.
Overall, this avoids confusion and trims the size of main.go a bit.
I tried to fix the TODO, but ran into the problem described by the added
documentation - that some packages in the import graph are incomplete,
as go/types was clever and didn't fully load them.
While here, also make the panics a bit more descriptive,
which helped me debug what was going wrong after the attempted refactor.
Fix a staticcheck warning about unused code,
as well as an unparam warning and a missing copyright header.
We also bump the action versions to their latest releases,
and drop unnecessary "name" fields for self-describing steps.
Note that we drop the "go env" commands, as setup-go does that now.
Finally, I did briefly try to add caching,
but then realised it didn't help us at all. Document why.
There are two scenarios when it comes to embedding fields.
The first is easy, and we always handled it well:
type Named struct { Foo int }
type T struct { Named }
In this scenario, T ends up with an embedded field named "Named",
and a promoted field named "Foo".
Then there's the form with a type alias:
type Named struct { Foo int }
type Alias = Named
type T struct { Alias }
This case is different: T ends up with an embedded field named "Alias",
and a promoted field named "Foo".
Note how the field gets its name from the referenced type,
even if said type is just an alias to another type.
This poses two problems.
First, we must obfuscate the field T.Alias as the name "Alias",
and not as the name "Named" that the alias points to.
Second, we must be careful of cases where Named and Alias are declared
in different packages, as they will obfuscate the same name differently.
Both of those problems compounded in the reported issue.
The actual reason is that quic-go has a type alias in the form of:
type ConnectionState = qtls.ConnectionState
In other words, the entire problem boils down to a type alias which
points to a named type in a different package, where both types share
the same name. For example:
package parent
import "parent/p1"
type T struct { p1.SameName }
[...]
package p1
import "parent/p2"
type SameName = p2.SameName
[...]
package p2
type SameName struct { Foo int }
This broke garble because we had a heuristic to detect when an embedded
field was a type alias:
// Instead, detect such a "foreign alias embed".
// If we embed a final named type,
// but the field name does not match its name,
// then it must have been done via an alias.
// We dig out the alias's TypeName via locateForeignAlias.
if named.Obj().Name() != node.Name {
As the reader can deduce, this heuristic would incorrectly assume that
the snippet above does not embed a type alias, when in fact it does.
When obfuscating the field T.SameName, which uses a type alias,
we would correctly obfuscate the name "SameName",
but we would incorrectly obfuscate it with the package p2, not p1.
This would then result in build errors.
To fix this problem for good, we need to get rid of the heuristic.
Instead, we now mimic what was done for KnownCannotObfuscate,
but for embedded fields which use type aliases.
KnownEmbeddedAliasFields is now filled for each package
and stored in the cache as part of cachedOutput.
We can then detect the "embedded alias" case reliably,
even when the field is declared in an imported package.
On the plus side, we get to remove locateForeignAlias.
We also add a couple of TODOs to record further improvements.
Finally, add a test.
Fixes#466.
This is like allocs/op by testing.B.ReportAllocs,
but it combines all allocations by garble sub-processes.
As we currently generate quite a bit of garbage,
and reductions in it may only reduce time/op very slowly,
this new metric will help us visualize small improvements.
The regular ReportAllocs would not help us at all,
as the main process simply executes "garble build".
We remove user-ns/op to make space for mallocs/op,
and also since it's a bit redundant given sys-ns/op and time-ns/op.
name time/op
Build-16 9.20s ± 1%
name bin-B
Build-16 5.16M ± 0%
name cached-time/op
Build-16 304ms ± 4%
name mallocs/op
Build-16 30.7M ± 0%
name sys-time/op
Build-16 4.78s ± 4%
If package P1 imports package P2, P1 needs to know which names from P2
weren't obfuscated. For instance, if P2 declares T2 and does
"reflect.TypeOf(T2{...})", then P2 won't obfuscate the name T2, and
neither should P1.
This information should flow from P2 to P1, as P2 builds before
P1. We do this via obfuscatedTypesPackage; P1 loads the type information
of the obfuscated version of P2, and does a lookup for T2. If T2 exists,
then it wasn't obfuscated.
This mechanism has served us well, but it has downsides:
1) It wastes CPU; we load the type information for the entire package.
2) It's complex; for instance, we need KnownObjectFiles as an extra.
3) It makes our code harder to understand, as we load both the original
and obfuscated type informaiton.
Instead, we now have each package record what names were not obfuscated
as part of its cachedOuput file. Much like KnownObjectFiles, the map
records incrementally through the import graph, to avoid having to load
cachedOutput files for indirect dependencies.
We shouldn't need to worry about those maps getting large;
we only skip obfuscating declared names in a few uncommon scenarios,
such as the use of reflection or cgo's "//export".
Since go/types is relatively allocation-heavy, and the export files
contain a lot of data, we get a nice speed-up:
name old time/op new time/op delta
Build-16 11.5s ± 2% 11.1s ± 3% -3.77% (p=0.008 n=5+5)
name old bin-B new bin-B delta
Build-16 5.15M ± 0% 5.15M ± 0% ~ (all equal)
name old cached-time/op new cached-time/op delta
Build-16 375ms ± 3% 341ms ± 6% -8.96% (p=0.008 n=5+5)
name old sys-time/op new sys-time/op delta
Build-16 283ms ±17% 289ms ±13% ~ (p=0.841 n=5+5)
name old user-time/op new user-time/op delta
Build-16 687ms ± 6% 664ms ± 7% ~ (p=0.548 n=5+5)
Fixes#456.
Updates #475.
Obfuscating the runtime only needs to list the linknamed packages,
and doesn't need to know about their dependencies directly.
Refactor the script to return a "flat" list that includes all packages
we need, except those that we know the runtime already pulled in.
This allows us to simplify the script and avoid passing -deps to cmd/go.
Performance is unaffected, but I reckon it's worthwhile given how much
we simplified the script.
Longer term, it's also best to avoid using -deps when we don't need it,
as cmd/go could avoid computing information we don't need.
name old time/op new time/op delta
Build/NoCache-16 1.68s ± 1% 1.68s ± 0% ~ (p=1.000 n=5+5)
name old bin-B new bin-B delta
Build/NoCache-16 6.72M ± 0% 6.72M ± 0% +0.01% (p=0.008 n=5+5)
name old sys-time/op new sys-time/op delta
Build/NoCache-16 1.88s ± 1% 1.89s ± 2% ~ (p=0.548 n=5+5)
name old user-time/op new user-time/op delta
Build/NoCache-16 19.9s ± 1% 19.8s ± 0% ~ (p=0.421 n=5+5)
Currently exported names in the main package are not hashed, because they
might be a Go plugin API. We don't currently support Go plugins though,
so hash them anyway.
Updates #87.
The -X linker flag sets a string variable to a given value,
which is often used to inject strings such as versions.
The way garble's literal obfuscation works,
we replace string literals with anonymous functions which,
when evaluated, result in the original string.
Both of these features work fine separately,
but when intersecting, they break. For example, given:
var myVar = "original"
[...]
-ldflags=-X=main.myVar=replaced
The -X flag effectively replaces the initial value,
and -literals adds code to be run at init time:
var myVar = "replaced"
func init() { myVar = func() string { ... } }
Since the init func runs later, -literals breaks -X.
To avoid that problem,
don't obfuscate literals whose variables are set via -ldflags=-X.
We also leave TODOs about obfuscating those in the future,
but we're also leaving regression tests to ensure we get it right.
Fixes#323.
We recently made an important change when obfuscating the runtime,
so that if it's missing any linkname packages in ListedPackages,
it does an extra "go list" call to obtain their information.
This works very well, but we missed an edge case.
In main.go, we disable flagLiterals for the runtime package,
but not for other packages like sync/atomic.
And, since the runtime's extra "go list" has to compute GarbleActionIDs,
it uses the list of garble flags via appendFlags.
Unfortunately, it thinks "-literals" isn't set, when it is,
and the other packages see it as being set.
This discrepancy results in link time errors,
as each end of the linkname obfuscates with a different hash:
> garble -literals build
[stderr]
# test/main
jccGkbFG.(*yijmzGHo).String: relocation target jccGkbFG.e_77sflf not defined
jQg9GEkg.(*NLxfRPAP).pB5p2ZP0: relocation target jQg9GEkg.ce66Fmzl not defined
jQg9GEkg.(*NLxfRPAP).pB5p2ZP0: relocation target jQg9GEkg.e5kPa1qY not defined
jQg9GEkg.(*NLxfRPAP).pB5p2ZP0: relocation target jQg9GEkg.aQ_3sL3Q not defined
jQg9GEkg.(*NLxfRPAP).pB5p2ZP0: relocation target jQg9GEkg.zls3wmws not defined
jQg9GEkg.(*NLxfRPAP).pB5p2ZP0: relocation target jQg9GEkg.g69WgKIS not defined
To fix the problem, treat flagLiterals as read-only after flag.Parse,
just like we already do with the other flags except flagDebugDir.
The code that turned flagLiterals to false is no longer needed,
as literals.Obfuscate is only called when ToObfuscate is true,
and ToObfuscate is false for runtimeAndDeps already.
Literal obfuscation uses constant folding now,
so it no longer needs to record identifiers to ignore.
Remove the parameter and the outdated bit of docs.
Daniel Martí
lu4p
pagran
pagran
Andrew LeFevre