fix: handle struct offset constants in assembly files#986
fix: handle struct offset constants in assembly files#986goloroden wants to merge 1 commit intoburrowers:masterfrom
Conversation
When a Go package has assembly files, the compiler generates go_asm.h with constants like "syscall15Args_a1" (field offset) and "syscall15Args__size" (struct size). When garble obfuscates struct types and fields, go_asm.h gets regenerated with obfuscated names, but the assembly source files still reference the original names. This commit: - Computes mappings from original to obfuscated struct offset constants during the compile phase - Applies these mappings to assembly files in the second asm pass - Adds the original source directory to -I include path so local header files can still be found Fixes burrowers#948
|
Thanks for the contribution, will review soon. |
|
Thanks for the quick reply. I'm looking forward to your feedback 😊 |
mvdan
left a comment
There was a problem hiding this comment.
The way garble obfuscates names is entirely reproducible, so the way that this change stores a "mapping" of names seems weird to me. We don't need to do this at all; you can always cheaply re-compute how to obfuscate a name.
In other words, this change seems like it's in the right direction, but it feels like it's at least twice as much code as needed.
| MOVQ AX, ret+8(FP) | ||
| RET | ||
| -- main.stderr -- | ||
| 25 |
There was a problem hiding this comment.
I'd suggest adding a short test case to asm.txtar instead; if we added a test file for every fix we did, we'd already have 200 test scripts.
| // Add the original source directory to the include path so that | ||
| // local header files like "abi_amd64.h" can still be found. | ||
| if len(paths) > 0 { | ||
| flags = append(flags, "-I", filepath.Dir(paths[0])) |
There was a problem hiding this comment.
this feels dangerous; we want to limit the use of original non-obfuscated source files as much as possible. so I'd avoid doing this if possible. if we need files like abi_amd64.h in unobfuscated form, then we should explicitly carry those over.
|
Also, please note how the added test fails on Windows. Perhaps the test or fix are not entirely portable? |
When a Go package has assembly files, the compiler generates go_asm.h with constants like "syscall15Args_a1" (field offset) and "syscall15Args__size" (struct size). When garble obfuscates struct types and fields, go_asm.h gets regenerated with obfuscated names, but the assembly source files still reference the original names.
This commit:
Fixes #948