Commit Graph

285 Commits (598d5182fbbc0bd65b3b742e4ab424786fb732f1)

Author SHA1 Message Date
Daniel Martí 598d5182fb update x/tools version used in go:generate
Fixes running this go:generate line with Go tip.
1 year ago
pagran d3c6ed6729
add support for obfuscation of dot-imported string constants 1 year ago
Daniel Martí 5dd6b2dc43 support assembly references to package names
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.
1 year ago
Daniel Martí 7e95fcf1cb detect which objects are global in a simpler way
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)
1 year ago
Daniel Martí 1c4fe53fc1 skip over comments when obfuscating assembly
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.
1 year ago
pagran 22c177f088
remove all unexported func names with -tiny via the linker
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
2 years ago
Daniel Martí 417bcf27bb support referencing package paths with periods in assembly
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.
2 years 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.
2 years ago
Daniel Martí 05d9b4ed26 avoid call to loadSharedCache for toolexec calls we skip
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)
2 years ago
Daniel Martí 21a5eb1720 simplify types.TypeName panic logic
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.
2 years ago
Daniel Martí 9a0d48c27e use errors.Is for listPackage errors 2 years 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.
2 years ago
Daniel Martí 4a0c2d0274 remove duplicate err!=nil check 2 years ago
Daniel Martí 9c0cdc61ef avoid reflect method call panics with GOGARBLE=*
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.
2 years ago
Daniel Martí 481e3a1f09 default to GOGARBLE=*, stop using GOPRIVATE
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.
2 years ago
Daniel Martí ffb4987756
clarify that "garble version" does more now (#612)
Since earlier this year, it has also printed build settings.
2 years ago
Daniel Martí 7d591830cd teach -debugdir to produce assembly files too
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.
2 years ago
Daniel Martí e61317e7ae fix garble with newer Go tip versions
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.
2 years ago
Daniel Martí ff521782f1 obfuscate all assembly filenames
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.
2 years ago
Daniel Martí 416782340f support `garble test` in main packages
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
2 years ago
Daniel Martí 7c2866356f support obfuscating the syscall package
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.
2 years ago
Daniel Martí e71cb69dd8 support obfuscating the time package
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.
2 years ago
Daniel Martí 58b2d64784 drop support for Go 1.18.x
With Go 1.19 having been out for two months,
and Go 1.20's first beta coming out in two months,
it is now time to move forward again.
2 years ago
Daniel Martí fc91758b49 obfuscate Go names in asm header files
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.
2 years ago
Daniel Martí f9d99190d2 use -toolexec="garble toolexec"
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.
2 years ago
Daniel Martí 9d46fe917a avoid a type assertion panic with generic code
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.
2 years ago
Daniel Martí 8ad374d2fb start testing on Go 1.19.x
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.
2 years ago
Daniel Martí 4155854a2e go 1.18.x now sets -buildvcs=false for `go test`
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.
2 years ago
Daniel Martí 8d36e1d80e avoid go/printer from breaking imports
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.
2 years ago
Daniel Martí ec68fc6750 avoid `...any` allocs in debug logs in hot loops
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)
2 years ago
Daniel Martí 3f9d77d9b6 join runtimeAndDeps into cannotObfuscate
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.
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í 21bd89ff73 slight simplifications and alloc reductions
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.
2 years ago
Daniel Martí 3fbefbbacf fix TODOs about code which is now unused
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.
2 years ago
Daniel Martí d18dd73556 use go/parser.SkipObjectResolution
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.
2 years ago
Daniel Martí d6afdd08bb obfuscate net and runtime/debug
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.
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í f0d79a38d4 remove a couple of easy TODOs
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.
2 years ago
Daniel Martí 9fc19e8bdf support import paths ending with ".go"
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.
2 years ago
Daniel Martí 14d3803a7b fix hashing of generic field names
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.
2 years ago
Daniel Martí 1a3d7868d9 make "garble version" friendlier for devel builds
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.
2 years ago
Daniel Martí 06d2f2d561 avoid capturing groups in regular expressions
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.
2 years ago
Daniel Martí 79b6e4db3d skip unnecessary "refusing to list" test errors
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.
2 years ago
Daniel Martí e3a59eae07 add missing context to two unmarshal errors
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.
2 years ago
Daniel Martí 20ff64128e make KnownReflectAPIs aware of variadic funcs
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.
2 years ago
pagran c8e0abf9c9 Fix removing named imports and fix removing imports with init() methods 2 years ago
Daniel Martí 6a39ad2d81 make "garble version" include VCS information
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.
2 years ago
Daniel Martí 1c564ef091 slightly improve code thanks to Go 1.18 APIs
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.
2 years ago
lu4p 237e0b7b7c Detect unnecessary imports instead of hardcoding 2 years ago
lu4p 1a0b028db7 all: drop support for Go 1.17
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.
2 years ago