Commit Graph

4 Commits (9dbbb518d19a4067ab28b1aaff4a5c536340a746)

Author SHA1 Message Date
Daniel Martí c26734c668 simplify our handling of "go list" errors
First, teach scripts/gen-go-std-tables.sh to omit test packages,
since runtime/metrics_test would always result in an error.
Instead, make transformLinkname explicitly skip that package,
leaving a comment about a potential improvement if needed.

Second, the only remaining "not found" error we had was "maps" on 1.20,
so rewrite that check based on ImportPath and GoVersionSemver.

Third, detect packages with the "exclude all Go files" error
by looking at CompiledGoFiles and IgnoredGoFiles, which is less brittle.
This means that we are no longer doing any filtering on pkg.Error.Err,
which means we are less likely to break with Go error message changes.

Fourth, the check on pkg.Incomplete is now obsolete given the above,
meaning that the CompiledGoFiles length check is plenty.

Finally, stop trying to be clever about how we print errors.
Now that we're no longer skipping packages based on pkg.Error values,
printing pkg.DepsErrors was causing duplicate messages in the output.
Simply print pkg.Error values with only minimal tweaks:
including the position if there is any, and avoiding double newlines.

Overall, this makes our logic a lot less complicated,
and garble still works the way we want it to.
1 year ago
lu4p 1526ce7fd2
rework reflection detection with ssa (#732)
This is significantly more robust, than the ast based detection and can
record very complex cases of indirect parameter reflection.

Fixes #554
1 year ago
Daniel Martí 0ec363d9c8 avoid breaking intrinsics when obfuscating names
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.
1 year ago
Daniel Martí 0b096c9e75 generate go_std_tables.go in its entirety
No more having to manually run the script and adapting it to Go code.
1 year ago