-
-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
plan9 builds broken on go1.18-b69b2f63d6 with garble @ 64cbbbaa #417
Comments
Thanks for reporting. We should probably have an integration test that ensures cross-builds to platforms we're not testing on still work. |
#418 prevents the misplaced compiler directive failure in the first case, now falling through to the shadowing of the |
I think these plan9 failures are not fully deterministic; I've seen different errors with different runs. Probably given that package compiles happen concurrently. In any case, I'm aware #418 won't fix this issue. The append build failure is a bug with literal obfuscation, for example. |
In the added test case, "garble -literals build" would fail: --- FAIL: TestScripts/literals (8.29s) testscript.go:397: > env GOPRIVATE=test/main > garble -literals build [stderr] # test/main Usz1FmFm.go:1: cannot call non-function string (type int), declared at Usz1FmFm.go:1 Usz1FmFm.go:1: string is not a type Usz1FmFm.go:1: cannot call non-function append (type int), declared at Usz1FmFm.go:1 That is, for input code such as: var append int println("foo") _ = append We'd end up with obfuscated code like: var append int println(func() string { // obfuscation... x = append(x, ...) // obfuscation... return string(x) }) _ = append Which would then break, as the code is shadowing the "append" builtin. To work around this, always obfuscate variable names, so we end up with: var mwu1xuNz int println(func() string { // obfuscation... x = append(x, ...) // obfuscation... return string(x) }) _ = mwu1xuNz This change shouldn't make the quality of our obfuscation stronger, as local variable names do not currently end up in Go binaries. However, this does make garble more consistent in treating identifiers, and it completely avoids any issues related to shadowing builtins. Moreover, this also paves the way for publishing obfuscated source code, such as burrowers#369. Fixes burrowers#417.
#420 fixes the immediate |
In the added test case, "garble -literals build" would fail: --- FAIL: TestScripts/literals (8.29s) testscript.go:397: > env GOPRIVATE=test/main > garble -literals build [stderr] # test/main Usz1FmFm.go:1: cannot call non-function string (type int), declared at Usz1FmFm.go:1 Usz1FmFm.go:1: string is not a type Usz1FmFm.go:1: cannot call non-function append (type int), declared at Usz1FmFm.go:1 That is, for input code such as: var append int println("foo") _ = append We'd end up with obfuscated code like: var append int println(func() string { // obfuscation... x = append(x, ...) // obfuscation... return string(x) }) _ = append Which would then break, as the code is shadowing the "append" builtin. To work around this, always obfuscate variable names, so we end up with: var mwu1xuNz int println(func() string { // obfuscation... x = append(x, ...) // obfuscation... return string(x) }) _ = mwu1xuNz This change shouldn't make the quality of our obfuscation stronger, as local variable names do not currently end up in Go binaries. However, this does make garble more consistent in treating identifiers, and it completely avoids any issues related to shadowing builtins. Moreover, this also paves the way for publishing obfuscated source code, such as burrowers#369. Fixes burrowers#417.
I'm still seeing the append shadowing in my test (using go1.18-b69b2f63d6 and ea2e0bd).
|
Oops, thanks for flagging. I was pretty sure that my fix was complete, but I didn't double-check with the full plan9 build. |
Add a regression test in gogarble.txt, as that test is already set up with packages to not obfuscate. This bug manifested in the form of a build failure for GOOS=plan9 with -literals turned on: [...]/os/file_plan9.go:151:12: invalid operation: cannot call non-function append (variable of type bool) In this case, the "os" package is not to be obfuscated, but we would still obfuscate its literals as per the bug. But, since the package's identifiers were not obfuscated, names like "append" were not replaced as per ea2e0bd, meaning that the shadowing would still affect us. Fixes burrowers#417.
Add a regression test in gogarble.txt, as that test is already set up with packages to not obfuscate. This bug manifested in the form of a build failure for GOOS=plan9 with -literals turned on: [...]/os/file_plan9.go:151:12: invalid operation: cannot call non-function append (variable of type bool) In this case, the "os" package is not to be obfuscated, but we would still obfuscate its literals as per the bug. But, since the package's identifiers were not obfuscated, names like "append" were not replaced as per ea2e0bd, meaning that the shadowing would still affect us. Fixes #417.
What version of Garble and Go are you using?
What environment are you running Garble on?
go env
OutputWhat did you do?
Run
go test
in github.com/kortschak/toutoumomoma. This involves cross-compiled builds including for plan9, which fails. Minimised reproducer not requiring running this test suite listed below.What did you expect to see?
Successful build.
What did you see instead?
exec_plan9.go:
//go:norace
is placed above empty text having been moved from the correct position above theforkAndExecInChild
function definition belowdupdev
. Reproduced byGOOS=plan9 garble -literals build -o ./testdata/executable ./testdata
in github.com/kortschak/toutoumomoma.file_plan9.go:
append
is a bool here, and so shadows theappend
built-in and breaks string obfuscation. Reproduced byGOOS=plan9 garble -literals -tiny build -o ./testdata/executable ./testdata
in github.com/kortschak/toutoumomoma.The text was updated successfully, but these errors were encountered: