Skip to content

fix: handle struct offset constants in assembly files#986

Open
goloroden wants to merge 1 commit intoburrowers:masterfrom
goloroden:fix-asm-struct-offsets
Open

fix: handle struct offset constants in assembly files#986
goloroden wants to merge 1 commit intoburrowers:masterfrom
goloroden:fix-asm-struct-offsets

Conversation

@goloroden
Copy link

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 #948

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
@luantak
Copy link
Member

luantak commented Dec 23, 2025

Thanks for the contribution, will review soon.

@goloroden
Copy link
Author

Thanks for the quick reply.

I'm looking forward to your feedback 😊

Copy link
Member

@mvdan mvdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mvdan
Copy link
Member

mvdan commented Dec 25, 2025

Also, please note how the added test fails on Windows. Perhaps the test or fix are not entirely portable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

ebitengine/purego causes: expected pseudo-register; found R11

3 participants