diff mbox series

[3/3] x86: widen applicability and use of CheckRegSize

Message ID 0168ba4a-766c-7cfe-7917-53259f846da0@suse.com
State Accepted
Headers
Series x86: correct checking of matching operand sizes |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jan Beulich Nov. 23, 2022, 10:35 a.m. UTC 
  First of all make operand_type_register_match() apply to all sized
operands, i.e. in Intel Syntax also to respective memory ones. This
addresses gas wrongly accepting certain SIMD insns where register and
memory operand sizes should match but don't. This apparently has
affected all templates with one memory-only operand and one or more
register ones, both permitting at least two sizes, due to CheckRegSize
not taking effect.

Then also add CheckRegSize to a couple of non-SIMD templates matching
that same pattern of memory-only vs register operands. This replaces
bogus (for Intel Syntax) diagnostics referring to a wrong suffix (when
none was used at all) by "type mismatch" ones, just like already emitted
for insns where the template allows a register operand alongside a
memory one at any particular position.

This also is a prereq to limiting (ideally eliminating in the long run)
suffix "derivation" in Intel Syntax mode.

While making the code adjustment also flip order of checks to do the
cheaper one first in both cases.
---
CheckRegSize now firmly isn't an appropriate name anymore - perhaps we
want to rename it to e.g. CheckSizes or CheckOperandSize (and then
better in a prereq patch)?

Comments

H.J. Lu Nov. 29, 2022, 11:57 p.m. UTC | #1 
On Wed, Nov 23, 2022 at 2:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> First of all make operand_type_register_match() apply to all sized
> operands, i.e. in Intel Syntax also to respective memory ones. This
> addresses gas wrongly accepting certain SIMD insns where register and
> memory operand sizes should match but don't. This apparently has
> affected all templates with one memory-only operand and one or more
> register ones, both permitting at least two sizes, due to CheckRegSize
> not taking effect.
>
> Then also add CheckRegSize to a couple of non-SIMD templates matching
> that same pattern of memory-only vs register operands. This replaces
> bogus (for Intel Syntax) diagnostics referring to a wrong suffix (when
> none was used at all) by "type mismatch" ones, just like already emitted
> for insns where the template allows a register operand alongside a
> memory one at any particular position.
>
> This also is a prereq to limiting (ideally eliminating in the long run)
> suffix "derivation" in Intel Syntax mode.
>
> While making the code adjustment also flip order of checks to do the
> cheaper one first in both cases.
> ---
> CheckRegSize now firmly isn't an appropriate name anymore - perhaps we
> want to rename it to e.g. CheckSizes or CheckOperandSize (and then
> better in a prereq patch)?
>

CheckOperandSize sounds better.
diff mbox series

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2192,7 +2192,7 @@  operand_type_match (i386_operand_type ov
 
 /* If given types g0 and g1 are registers they must be of the same type
    unless the expected operand type register overlap is null.
-   Some Intel syntax memory operand size checking also happens here.  */
+   Intel syntax sized memory operands are also checked here.  */
 
 static INLINE int
 operand_type_register_match (i386_operand_type g0,
@@ -2202,18 +2202,14 @@  operand_type_register_match (i386_operan
 {
   if (g0.bitfield.class != Reg
       && g0.bitfield.class != RegSIMD
-      && (!operand_type_check (g0, anymem)
-	  || g0.bitfield.unspecified
-	  || (t0.bitfield.class != Reg
-	      && t0.bitfield.class != RegSIMD)))
+      && (g0.bitfield.unspecified
+	  || !operand_type_check (g0, anymem)))
     return 1;
 
   if (g1.bitfield.class != Reg
       && g1.bitfield.class != RegSIMD
-      && (!operand_type_check (g1, anymem)
-	  || g1.bitfield.unspecified
-	  || (t1.bitfield.class != Reg
-	      && t1.bitfield.class != RegSIMD)))
+      && (g1.bitfield.unspecified
+	  || !operand_type_check (g1, anymem)))
     return 1;
 
   if (g0.bitfield.byte == g1.bitfield.byte
--- a/gas/testsuite/gas/i386/intelbad.l
+++ b/gas/testsuite/gas/i386/intelbad.l
@@ -169,3 +169,27 @@ 
 .*:192: Error: .*
 .*:193: Error: .*
 .*:194: Error: .*
+.*:196: Error: .*
+.*:197: Error: .*
+.*:199: Error: .*
+.*:200: Error: .*
+.*:202: Error: .*
+.*:203: Error: .*
+.*:205: Error: .*
+.*:206: Error: .*
+.*:208: Error: .*
+.*:209: Error: .*
+.*:211: Error: .*
+.*:212: Error: .*
+.*:214: Error: .*
+.*:215: Error: .*
+.*:217: Error: .*
+.*:218: Error: .*
+.*:220: Error: .*
+.*:221: Error: .*
+.*:223: Error: .*
+.*:224: Error: .*
+.*:226: Error: .*
+.*:227: Error: .*
+.*:229: Error: .*
+.*:230: Error: .*
--- a/gas/testsuite/gas/i386/intelbad.s
+++ b/gas/testsuite/gas/i386/intelbad.s
@@ -192,3 +192,39 @@  start:
 	lsl	ax, eax
 	lsl	eax, dword ptr [eax]
 	lsl	ax, dword ptr [eax]
+
+	mov	eax, word ptr [eax]
+	mov	eax, qword ptr [eax]
+
+	mov	eax, word ptr [0x12345678]
+	mov	eax, qword ptr [0x12345678]
+
+	xchg	eax, word ptr [eax]
+	xchg	eax, qword ptr [eax]
+
+	add	eax, word ptr [eax]
+	add	eax, qword ptr [eax]
+
+	test	eax, word ptr [eax]
+	test	eax, qword ptr [eax]
+
+	test	word ptr [eax], eax
+	test	qword ptr [eax], eax
+
+	movnti	word ptr [eax], eax
+	movnti	qword ptr [eax], eax
+
+	movbe	eax, word ptr [eax]
+	movbe	eax, qword ptr [eax]
+
+	vmovntdq xmmword ptr [eax], ymm0
+	vmovntdq zmmword ptr [eax], ymm0
+
+	vmovntdqa ymm0, xmmword ptr [eax]
+	vmovntdqa ymm0, zmmword ptr [eax]
+
+	movdiri	word ptr [eax], eax
+	movdiri	qword ptr [eax], eax
+
+	aadd	word ptr [eax], eax
+	aadd	qword ptr [eax], eax
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -142,9 +142,9 @@ 
 ### MARKER ###
 
 // Move instructions.
-mov, 0xa0, None, CpuNo64, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Unspecified|Byte|Word|Dword, Acc|Byte|Word|Dword }
-mov, 0xa0, None, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
-movabs, 0xa0, None, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
+mov, 0xa0, None, CpuNo64, D|W|CheckRegSize|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Unspecified|Byte|Word|Dword, Acc|Byte|Word|Dword }
+mov, 0xa0, None, Cpu64, D|W|CheckRegSize|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
+movabs, 0xa0, None, Cpu64, D|W|CheckRegSize|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
 movq, 0xa1, None, Cpu64, D|Size64|NoSuf, { Disp64|Unspecified|Qword, Acc|Qword }
 mov, 0x88, None, 0, D|W|CheckRegSize|Modrm|No_sSuf|No_ldSuf|HLEPrefixRelease, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 movq, 0x89, None, Cpu64, D|Modrm|Size64|NoSuf|HLEPrefixRelease, { Reg64, Reg64|Unspecified|Qword|BaseIndex }
@@ -176,7 +176,7 @@  movq, 0xf21, None, Cpu64, D|RegMem|Size6
 mov, 0xf24, None, Cpu386|CpuNo64, D|RegMem|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf, { Test, Reg32 }
 
 // Move after swapping the bytes
-movbe, 0x0f38f0, None, CpuMovbe, D|Modrm|No_bSuf|No_sSuf|No_ldSuf, { Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+movbe, 0x0f38f0, None, CpuMovbe, D|Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 
 // Move with sign extend.
 // "movsbl" & "movsbw" must not be unified into "movsb" to avoid
@@ -302,7 +302,7 @@  cmp, 0x3c, None, 0, W|No_sSuf|No_ldSuf,
 cmp, 0x80, 7, 0, W|Modrm|No_sSuf|No_ldSuf, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 
 test, 0x84, None, 0, W|CheckRegSize|Modrm|No_sSuf|No_ldSuf, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Unspecified|Byte|Word|Dword|Qword|BaseIndex }
-test, 0x84, None, 0, W|Modrm|No_sSuf|No_ldSuf, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
+test, 0x84, None, 0, W|CheckRegSize|Modrm|No_sSuf|No_ldSuf, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
 test, 0xa8, None, 0, W|No_sSuf|No_ldSuf|Optimize, { Imm8|Imm16|Imm32|Imm32S, Acc|Byte|Word|Dword|Qword }
 test, 0xf6, 0, 0, W|Modrm|No_sSuf|No_ldSuf|Optimize, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 
@@ -970,7 +970,7 @@  fucompi, 0xdfe8, None, Cpu687, NoSuf, {
 
 // Pentium4 extensions.
 
-movnti, 0xfc3, None, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf, { Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex }
+movnti, 0xfc3, None, CpuSSE2, Modrm|CheckRegSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf, { Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex }
 clflush, 0xfae, 7, CpuClflush, Modrm|Anysize|IgnoreSize|NoSuf, { BaseIndex }
 lfence, 0xfaee8, None, CpuSSE2, NoSuf, {}
 mfence, 0xfaef0, None, CpuSSE2, NoSuf, {}
@@ -3053,7 +3053,7 @@  cldemote, 0x0f1c, 0, CpuCLDEMOTE, Modrm|
 
 // MOVDIR[I,64B] instructions.
 
-movdiri, 0xf38f9, None, CpuMOVDIRI, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf, { Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex }
+movdiri, 0xf38f9, None, CpuMOVDIRI, Modrm|CheckRegSize|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf, { Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex }
 movdir64b, 0x660f38f8, None, CpuMOVDIR64B, Modrm|AddrPrefixOpReg|NoSuf, { Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 
 // MOVEDIR instructions end.