Mips arch broken #646
Comments
|
Thanks for spotting. This does not appear to be a recent regression, but we should still fix it. |
mvdan
added a commit
to mvdan/garble-fork
that referenced
this issue
Jan 25, 2023
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 burrowers#646.
mvdan
added a commit
that referenced
this issue
Jan 26, 2023
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What version of Garble and Go are you using?
$ garble version mvdan.cc/garble v0.0.0-20230113184234-5b2193351fdf Build settings: -compiler gc CGO_ENABLED 1 GOARCH amd64 GOOS linux GOAMD64 v1 vcs git vcs.revision 5b2193351fdfee3611bf01ac4215cf0a41172dcb vcs.time 2023-01-13T18:42:34Z vcs.modified true $ go version go version go1.19.4 linux/amd64What environment are you running Garble on?
go envOutputWhat did you do?
While building file size change statistics between v0.8.0 and v0.9.0, I found that
mipsis not building. Here full list (marked as FAILED)platform = GOOS/GOARCHWhat did you expect to see?
Success build
What did you see instead?
mips:
mips64:
The text was updated successfully, but these errors were encountered: