Commit Graph

5 Commits (bb69facbd87dbcd62cb8e762a6d2c27be925e881)

Author SHA1 Message Date
Daniel Martí 98466a1f64 scripts: use -trimpath for "go build" in check-third-party
Since "garble build" first performs a Go build with -trimpath,
it helps to also use the flag with the "go build" step.
This way we get more build cache hits, as building a Go package with and
without the flag results in two separate builds.

Before:

	$ go clean -cache && time ./scripts/check-third-party.sh

	real  0m41.844s
	user  2m17.791s
	sys   0m35.440s

After:

	$ go clean -cache && time ./scripts/check-third-party.sh

	real  0m33.983s
	user  1m50.596s
	sys   0m28.499s
2 years ago
Daniel Martí 352d2fa73d scripts: follow shfmt 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í 60dbece24f work around another go/printer bug to fix andybalholm/brotli
When obfuscating the following piece of code:

	func issue_573(s struct{ f int }) {
		var _ *int = &s.f
		/*x*/
	}

the function body would roughly end up printed as:
we would roughly end up with:

	var _ *int = &dZ4xYx3N
	/*x*/.rbg1IM3V

Note that the /*x*/ comment got moved earlier in the source code.
This happens because the new identifiers are longer, so the printer
thinks that the selector now ends past the comment.

That would be fine - we don't really mind where comments end up,
because these non-directive comments end up being removed anyway.

However, the resulting syntax is wrong, as the period for the selector
must be on the first line rather than the second.
This is a go/printer bug that we should fix upstream,
but until then, we must work around it in Go 1.18.x and 1.19.x.

The fix is somewhat obvious in hindsight. To reduce the chances that
go/printer will trip over comments and produce invalid syntax,
get rid of most comments before we use the printer.
We still keep the removal of comments after printing,
since go/printer consumes some comments in ast.Node Doc fields.

Add the minimized unit test case above, and add the upstream project
that found this bug to check-third-party.
andybalholm/brotli helps cover a compression algorithm and ccgo code
generation from C to Go, and it's also a fairly popular module,
particular with HTTP implementations which want pure-Go brotli.

While here, fix the check-third-party script: it was setting GOFLAGS
a bit too late, so it may run `go get` on the wrong mod file.

Fixes #573.
2 years ago
Daniel Martí 201d890430 start checking some third party builds for regressions
Our tests should already be pretty extensive,
and any bug fixes should result in more regression test cases,
but testing against a few diverse and popular third party modules
will help prevent unintended regressions while developing garble.

The list is short for now. More can be added later.
This adds protobuf and wireguard from the original issue,
but not cobra and logrus, as they aren't particularly complex nor add
significant variety on top of protobuf and wireguard.

While here, we remove the job that only runs crlf-test.sh,
as we don't really need a separate job for a tiny script.

Fixes #240.
2 years ago