From 98a562d8b33a590baa23a1036bb0de48c52e7cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Wed, 7 Oct 2026 12:58:24 +0200 Subject: [PATCH] fix(asm): settle the byte-form width from the register operands The suffixed scalar families derived the operand width from the mnemonic alone, so a byte-spelled register under the L spelling or no suffix at all encoded the widened form: XADDL DL, DL emitted 0F C1 where the byte form is 0F C0, CMPL AL, $7 emitted the 32-bit immediate form where the AL form is 3C 07, and CRC32 DL, R11 widened past the F0 byte opcode. operandWidth now reconciles the suffix with the operands: a byte register (AL, DL, R8B, ...) forces the 8-bit form, which is the text the toolchain's own disassembly prints for those encodings, while the W and Q spellings never ride a byte register and are refused as go tool asm refuses them (MOVQ AL, AX). The shift count and the two- and three-operand IMUL forms stay out of the reconciliation, and the byte accumulator short forms now belong to the AL spelling alone, matching the toolchain's division (ADDB $3, AX is 80 c0 03, TESTB $7, AX is f6 c0 07). Assisted-by: GLM 5.3 --- asm/encode.go | 19 +++++++++++- asm/instrs.go | 13 +++++--- asm/operand.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/asm/encode.go b/asm/encode.go index 94da113..4221231 100644 --- a/asm/encode.go +++ b/asm/encode.go @@ -257,6 +257,19 @@ func (e *enc) encode(mnem string, ops []Operand) error { } base, size := splitSize(upper) + // The scalar families that own byte forms settle the width against the + // register operands here, before the unsuffixed 64-bit default applies, + // so a literal Q suffix stays distinguishable from no suffix at all + // (CRC32 DL, R11 is the byte form; CRC32Q DL, R11 is a conflict). + if byteFormBase[base] { + w, err := operandWidth(mnem, size, widthOperands(base, ops)) + if err != nil { + return err + } + if w != 0 { + size = w + } + } if size == 0 { size = 8 // default operand size in 64-bit mode (e.g. PUSHQ) } @@ -306,8 +319,12 @@ func (e *enc) encode(mnem string, ops []Operand) error { case "MOV": return e.encodeMov(ops, size) // MOVD is the Go assembler's alias of MOVQ: the same byte forms, 64-bit - // REX.W and all. + // REX.W and all. The alias takes no byte register either, the same + // conflict rule the Q-suffixed spelling answers to. case "MOVD": + if _, err := operandWidth(mnem, 8, ops); err != nil { + return err + } return e.encodeMov(ops, 8) case "ADD", "SUB", "AND", "OR", "XOR", "CMP", "ADC", "SBB": return e.encodeALU(aluOp[base], ops, size) diff --git a/asm/instrs.go b/asm/instrs.go index a223488..1c1a8ae 100644 --- a/asm/instrs.go +++ b/asm/instrs.go @@ -529,8 +529,10 @@ func (e *enc) encodeALUImm(digit int, dst Operand, imm int64, size int) error { return err } // The byte accumulator short form (0x04+digit*8, no ModR/M) when - // the destination is AL, the form the Go assembler prefers here. - if r, ok := dst.(Reg); ok && r.idx == 0 { + // the destination is spelled AL itself; the size-agnostic AX takes + // the generic 0x80 /digit form, the division go tool asm makes + // (ADDB $7, AL is 04 07 while ADDB $3, AX is 80 c0 03). + if r, ok := dst.(Reg); ok && r.idx == 0 && byteReg(r) { i := &instr{opcode: []byte{byte(0x04 + digit*8)}, modrm: -1, sib: -1} i.imm = immBytes return e.emit(i) @@ -587,8 +589,11 @@ func (e *enc) encodeTest(ops []Operand, size int) error { if imm, ok := src.(Imm); ok { // 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 { + // register operand is AL/AX, whatever the immediate's width. At + // byte width the short form belongs to the AL spelling alone; the + // size-agnostic AX takes the generic F6 /0 (TESTB $7, AX is + // f6 c0 07), the same division the ALU accumulator makes. + if r, ok := dst.(Reg); ok && r.idx == 0 && (size != 1 || byteReg(r)) { op := byte(0xA9) if size == 1 { op = 0xA8 diff --git a/asm/operand.go b/asm/operand.go index 9de5aa4..b6a90e8 100644 --- a/asm/operand.go +++ b/asm/operand.go @@ -3,6 +3,8 @@ package asm +import "fmt" + // Operand is an instruction operand: a Reg, a Mem reference or an Imm value. type Operand interface { isOperand() @@ -107,3 +109,84 @@ func isX86Mem(o Operand) bool { return false } } + +// byteReg reports whether r is a general-purpose register whose name spells +// a byte width (AL, CL, DL, BL, AH-DH, SPL-DIL, R8B-R15B): the name itself +// fixes an 8-bit access, unlike the size-agnostic spellings (AX, EAX, RAX, +// R11) whose width the mnemonic supplies. The vector, opmask, x87, MMX, +// control and segment registers never name a byte access. +func byteReg(r Reg) bool { + return r.size == 1 && !r.isVec() && !r.mask && !r.fp && !r.mmx && r.ctl == 0 && r.seg == 0 +} + +// byteFormBase lists the scalar families that own an 8-bit encoding beside +// the 16/32/64-bit ones: the arithmetic and logic group, TEST, the moves, +// the unary group, the shifts, the exchanges and atomics, the one-operand +// IMUL and CRC32. Families without a byte form (LEA, BT, the bit scans, +// PUSH/POP, ADCX/ADOX, BSWAP) stay out, so their width comes from the +// mnemonic alone and a byte register in them is the handler's own error to +// make. +var byteFormBase = map[string]bool{ + "MOV": true, + "ADD": true, "OR": true, "ADC": true, "SBB": true, + "AND": true, "SUB": true, "XOR": true, "CMP": true, + "TEST": true, + "INC": true, "DEC": true, "NEG": true, "NOT": true, + "MUL": true, "DIV": true, "IDIV": true, + "SHL": true, "SAL": true, "SHR": true, "SAR": true, + "ROL": true, "ROR": true, "RCL": true, "RCR": true, + "XCHG": true, "CMPXCHG": true, "XADD": true, + "CRC32": true, + "IMUL": true, +} + +// operandWidth settles the operand width of a suffixed scalar instruction +// against the widths its register operands spell. A byte-spelled register +// operand (AL, DL, R8B, ...) forces the 8-bit form whatever the mnemonic's +// L suffix or lack of one says: that is the text the toolchain's own +// disassembly prints for the byte encodings (the rendered suffix rides the +// operand-size attribute, so an L or no suffix at all can name a byte +// form), and every register operand joins the form at its low byte, the way +// go tool asm reads the classic names there (TESTL R11, DL encodes TESTB +// R11B, DL). A W or Q suffix never rides a byte form, so that mix is a +// conflict rejected the way the toolchain rejects it (MOVQ AL, AX). With +// no byte operand the mnemonic decides alone and 0 returns, keeping the +// caller's width: the classic names are size-agnostic at every width. +func operandWidth(mnem string, suffix int, ops []Operand) (int, error) { + for _, o := range ops { + r, ok := o.(Reg) + if !ok || !byteReg(r) { + continue + } + if suffix == 2 || suffix == 8 { + kind := "quad" + if suffix == 2 { + kind = "word" + } + return 0, fmt.Errorf("%s: byte register cannot take the %s form", mnem, kind) + } + return 1, nil + } + return 0, nil +} + +// widthOperands selects the operands that carry the instruction's data +// width for base: every operand of the scalar families but a shift's count, +// which names the CL register without narrowing the shifted value (RCLW CL, +// 0(R11) stays 16-bit), and nothing of IMUL's two- and three-operand forms, +// which have no byte encoding at all. +func widthOperands(base string, ops []Operand) []Operand { + switch base { + case "SHL", "SAL", "SHR", "SAR", "ROL", "ROR", "RCL", "RCR": + if len(ops) > 1 { + return ops[1:] + } + return nil + case "IMUL": + if len(ops) == 1 { + return ops + } + return nil + } + return ops +}