From 78b12dd427036caa717593f89bb63aeb0f2ce119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Fri, 28 Aug 2026 19:55:25 +0200 Subject: [PATCH] fix(asm): match go tool asm encodings and strictness --- asm/assemble_test.go | 2 +- asm/encode.go | 7 ++++++ asm/encode_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++- asm/instrs.go | 51 +++++++++++++++++++++++++++++++++++++++---- 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/asm/assemble_test.go b/asm/assemble_test.go index e658cdd..f4cff9c 100644 --- a/asm/assemble_test.go +++ b/asm/assemble_test.go @@ -62,7 +62,7 @@ TEXT ·f(SB), NOSPLIT, $0 XORQ AX, AX loop: ADDQ $1, AX - CMPQ $10, AX + CMPQ AX, $10 JLT loop RET `) diff --git a/asm/encode.go b/asm/encode.go index 1e89640..333d87e 100644 --- a/asm/encode.go +++ b/asm/encode.go @@ -314,6 +314,13 @@ func memComponents(regField int, m Mem) (modrm, sib int, disp []byte, xBit, bBit return regField<<3 | 0x05, -1, le32(m.Disp), 0, 0, nil // mod=00, rm=101 } + // The SIB scale field only encodes 1/2/4/8; the Go assembler rejects + // anything else ("bad scale: 16"), so a silent fallback to scale 1 here + // would mis-assemble the operand instead of reporting it. + if m.HasIndex && m.Scale != 1 && m.Scale != 2 && m.Scale != 4 && m.Scale != 8 { + return 0, -1, nil, 0, 0, fmt.Errorf("bad scale: %d", m.Scale) + } + needSIB := m.HasIndex || (m.HasBase && m.Base.idx&7 == 4) var mod int diff --git a/asm/encode_test.go b/asm/encode_test.go index c041a75..73bb9a4 100644 --- a/asm/encode_test.go +++ b/asm/encode_test.go @@ -78,13 +78,63 @@ func TestALU(t *testing.T) { checkSyntax(t, "cmp rsi, r10", "CMPQ", SI, Reg{idx: 10, size: 8}) checkSyntax(t, "add rbx, qword ptr [rax]", "ADDQ", Ptr(AX, 0, 8), BX) checkSyntax(t, "add qword ptr [rax], rbx", "ADDQ", BX, Ptr(AX, 0, 8)) - checkSyntax(t, "cmp rbx, -0x20", "CMPQ", Imm(-32), BX) + // The Go assembler rejects the immediate-first CMP spelling outright, + // so Encode errors instead of silently emitting the swapped form. + if _, err := Encode("CMPQ", Imm(-32), BX); err == nil { + t.Errorf("Encode(CMPQ imm-first) should error, got success") + } // The Go assembler's own spelling: immediate second. checkSyntax(t, "cmp ecx, 0x1f", "CMPL", CX, Imm(31)) checkSyntax(t, "cmp ecx, -0x80000000", "CMPL", CX, Imm(-2147483648)) checkSyntax(t, "cmp r9, -0x80000000", "CMPQ", Reg{idx: 9, size: 8}, Imm(-2147483648)) } +// TestScalarXmmRegMoves pins the Go-assembler byte forms of scalar +// MOVQ/MOVL between GPRs and XMM registers (66 REX.W 0F 6E/0F 7E) and the +// memory forms (F3 0F 7E load, 66 0F D6 store), all byte-for-byte. +func TestScalarXmmRegMoves(t *testing.T) { + cases := []struct { + name string + mnem string + ops []Operand + want string + }{ + {"MOVQ AX,X1", "MOVQ", []Operand{AX, vreg(t, "X1")}, "66480f6ec8"}, + {"MOVQ DX,X2", "MOVQ", []Operand{DX, vreg(t, "X2")}, "66480f6ed2"}, + {"MOVQ X1,AX", "MOVQ", []Operand{vreg(t, "X1"), AX}, "66480f7ec8"}, + {"MOVQ X0,DX", "MOVQ", []Operand{vreg(t, "X0"), DX}, "66480f7ec2"}, + {"MOVL AX,X1", "MOVL", []Operand{AX, vreg(t, "X1")}, "660f6ec8"}, + {"MOVL X1,AX", "MOVL", []Operand{vreg(t, "X1"), AX}, "660f7ec8"}, + {"MOVQ (SI),X1", "MOVQ", []Operand{Ptr(SI, 0, 8), vreg(t, "X1")}, "f30f7e0e"}, + {"MOVQ X3,(DI)", "MOVQ", []Operand{vreg(t, "X3"), Ptr(DI, 0, 8)}, "660fd61f"}, + } + for _, c := range cases { + code, err := Encode(c.mnem, c.ops...) + if err != nil { + t.Errorf("%s: %v", c.name, err) + continue + } + if got := fmt.Sprintf("%x", code); got != c.want { + t.Errorf("%s: got %s, want %s", c.name, got, c.want) + } + } +} + +// TestBadScale pins the go-tool-asm parity of rejecting SIB scales the +// hardware cannot encode. +func TestBadScale(t *testing.T) { + for _, sc := range []int{3, 5, 16, 32} { + if _, err := Encode("LEAQ", Idx(SI, BX, sc, 0, 8), AX); err == nil { + t.Errorf("LEAQ scale %d: expected error, got success", sc) + } + } + for _, sc := range []int{1, 2, 4, 8} { + if _, err := Encode("LEAQ", Idx(SI, BX, sc, 0, 8), AX); err != nil { + t.Errorf("LEAQ scale %d: %v", sc, err) + } + } +} + func TestLea(t *testing.T) { checkSyntax(t, "lea r9, ptr [rsi+4*rbx]", "LEAQ", Idx(SI, BX, 4, 0, 8), Reg{idx: 9, size: 8}) checkSyntax(t, "lea rax, ptr [rbx+0x8]", "LEAQ", Ptr(BX, 0x8, 8), AX) diff --git a/asm/instrs.go b/asm/instrs.go index 26256a9..57bcc61 100644 --- a/asm/instrs.go +++ b/asm/instrs.go @@ -51,13 +51,23 @@ func (e *enc) encodeMov(ops []Operand, size int) error { // Integer scalar XMM moves: MOVQ with an XMM operand is the SSE2 // packed-quadword move, NOT a GPR move: mem→xmm encodes as F3 0F 7E // (reg = dst, no REX.W — the Go assembler's form), xmm→mem as - // 66 0F D6 (rm = xmm). MOVL is the packed-dword move instead: - // 66 0F 6E load, 66 0F 7E store. A GPR-move fallback would silently - // emit REX.W 8B with the wrong operand meaning. + // 66 0F D6 (rm = xmm). Register forms against a GPR use the MOVD + // opcodes with REX.W instead: 66 REX.W 0F 6E (gpr→xmm) and + // 66 REX.W 0F 7E (xmm→gpr); the memory opcodes with a register r/m + // would be undefined forms. MOVL is the packed-dword move: + // 66 0F 6E load, 66 0F 7E store, no REX.W. A GPR-move fallback would + // silently emit REX.W 8B with the wrong operand meaning. _, srcVec := vecReg(src) dstReg, dstVec := vecReg(dst) if srcVec || dstVec { if dstVec { + if g, ok := src.(Reg); ok && !g.isVec() { + i := &instr{prefix: 0x66, opcode: []byte{0x0F, 0x6E}, modrm: -1, sib: -1, rexW: size == 8} + if err := setRM(i, dstReg, src, 8); err != nil { + return err + } + return e.emit(i) + } i := &instr{prefix: 0xF3, opcode: []byte{0x0F, 0x7E}, modrm: -1, sib: -1} if size == 4 { i.prefix = 0x66 @@ -72,6 +82,13 @@ func (e *enc) encodeMov(ops []Operand, size int) error { if !srcIsXMM || !srcXMM.isVec() { return fmt.Errorf("MOV: store needs an XMM source") } + if g, ok := dst.(Reg); ok && !g.isVec() { + i := &instr{prefix: 0x66, opcode: []byte{0x0F, 0x7E}, modrm: -1, sib: -1, rexW: size == 8} + if err := setRM(i, srcXMM, dst, 8); err != nil { + return err + } + return e.emit(i) + } i := &instr{prefix: 0x66, opcode: []byte{0x0F, 0xD6}, modrm: -1, sib: -1} if size == 4 { i.opcode = []byte{0x0F, 0x7E} @@ -199,7 +216,13 @@ func (e *enc) encodeALU(op struct { } src, dst := ops[0], ops[1] + // CMP never takes its immediate first: the Go assembler rejects + // CMPL $0, AX outright (only CMPL AX, $0 is legal, unlike TEST and the + // writing ALU ops whose immediate is naturally the source). if imm, ok := src.(Imm); ok { + if op.digit == 7 { + return fmt.Errorf("CMP immediate must be the second operand (reg, $imm)") + } return e.encodeALUImm(op.digit, dst, int64(imm), size) } @@ -290,6 +313,15 @@ func (e *enc) encodeALUImm(digit int, dst Operand, imm int64, size int) error { i.imm = []byte{byte(int8(imm))} return e.emit(i) } + // 0x81 /digit, imm16/imm32 — or the Go assembler's accumulator short + // form (opcode+5, no ModR/M) when the destination is AX/AL, which it + // prefers over the generic form exactly here. + if r, ok := dst.(Reg); ok && r.idx == 0 { + accOp := map[int]byte{0: 0x05, 1: 0x0D, 2: 0x15, 3: 0x1D, 4: 0x25, 5: 0x2D, 6: 0x35, 7: 0x3D}[digit] + i := newInstr(size, []byte{accOp}) + i.imm = immediate(imm, size, false) + return e.emit(i) + } // 0x81 /digit, imm16/imm32. i := newInstr(size, []byte{0x81}) if err := setRMDigit(i, digit, dst, size); err != nil { @@ -307,7 +339,18 @@ func (e *enc) encodeTest(ops []Operand, size int) error { } src, dst := ops[0], ops[1] if imm, ok := src.(Imm); ok { - // TEST r/m, imm: 0xF6 (8-bit) / 0xF7 /0. + // TEST r/m, imm: 0xF6 (8-bit) / 0xF7 /0 — but the Go assembler + // always uses the accumulator forms (A8/A9, no ModR/M) when the + // register operand is AL/AX, whatever the immediate's width. + if r, ok := dst.(Reg); ok && r.idx == 0 { + op := byte(0xA9) + if size == 1 { + op = 0xA8 + } + i := newInstr(size, []byte{op}) + i.imm = immediate(int64(imm), size, false) + return e.emit(i) + } op := byte(0xF7) if size == 1 { op = 0xF6