Skip to content
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

x/arch/x86/x86asm: GNUSyntax rendors relative pc jump targets wrong #18792

Open
minux opened this issue Jan 26, 2017 · 1 comment
Open

x/arch/x86/x86asm: GNUSyntax rendors relative pc jump targets wrong #18792

minux opened this issue Jan 26, 2017 · 1 comment
Milestone

Comments

@minux
Copy link
Member

@minux minux commented Jan 26, 2017

The GNUSyntax uses . to denote the address of the next instruction, but that doesn't match GNU as' definition. This makes it impossible to round trip the disassembly (GNU as will not be able to assemble je .-0x80 into two-byte short jump.)

package main
import "golang.org/x/arch/x86/x86asm"
func main() {
	text := []byte{0x74, 0x80, 0xeb, 0x80, 0x0f, 0x87, 0x80, 0xff, 0xff, 0xff, 0xe9, 0x80, 0xff, 0xff, 0xff}
	for len(text) > 0 {
		inst, err := x86asm.Decode(text[:], 64)
		if err != nil {
			panic(err)
		}
		println("GNU:", x86asm.GNUSyntax(inst))
		println("Go :", x86asm.GoSyntax(inst, 0, nil))
		text = text[inst.Len:]
	}
}

Output:

GNU: je .-0x80
Go : JE .-128
GNU: jmp .-0x80
Go : JMP .-128
GNU: ja .-0x80
Go : JA .-128
GNU: jmpq .-0x80
Go : JMP .-128

However, in AT&T syntax, . means the address of the current instruction, whereas the encoding actually means (conditional) jumps to the address of next instruction - 0x80.

I think to better test the disassembler, we need to do round-trip test (disassemble -> use GNU assembler to assemble the instruction -> disassemble again and verify the two Insts are deep equal.)

I want to fix this issue, but the current behavior is locked down the the current testdata/decode.txt.
Should I change the data?

/cc @rsc

@minux minux added this to the Unreleased milestone Jan 26, 2017
@minux
Copy link
Member Author

@minux minux commented Jan 26, 2017

The roundtrip test proved really effective at finding bugs:

Input: 41 80 3c 24 00          cmpb   $0x0,(%r12)
Output: cmpb $0x0,(%r12,%riz,1)

Actually, all 0(%r12) arguments are decoded wrong.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant