public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] x86: correct checking of matching operand sizes
@ 2022-11-23 10:32 Jan Beulich
  2022-11-23 10:33 ` [PATCH 1/3] x86: correct handling of LAR and LSL Jan Beulich
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jan Beulich @ 2022-11-23 10:32 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

I've spotted a few cases where operand sizes matching wasn't really
checked, leading to malformed insn/operand combinations to be
accepted (in the first patch another anomaly is also taken care of).
This mainly, but not only affects Intel Syntax.

1: correct handling of LAR and LSL
2: add missing CheckRegSize
3: widen applicability and use of CheckRegSize

Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] x86: correct handling of LAR and LSL
  2022-11-23 10:32 [PATCH 0/3] x86: correct checking of matching operand sizes Jan Beulich
@ 2022-11-23 10:33 ` Jan Beulich
  2022-11-23 10:34 ` [PATCH 2/3] x86: add missing CheckRegSize Jan Beulich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2022-11-23 10:33 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

Both uniformly only ever take 16-bit memory operands while at the same
time requiring matching (in size) register operands, which then also
should disassemble that way. This in particular requires splitting each
of the templates for the assembler and separating decode of the
register and memory forms in the disassembler.

--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -697,7 +697,9 @@ i386_intel_operand (char *operand_string
 	  i.types[this_operand].bitfield.word = 1;
 	  if (got_a_float == 2)	/* "fi..." */
 	    suffix = SHORT_MNEM_SUFFIX;
-	  else
+	  else if ((current_templates->start->base_opcode | 1) != 0x03
+		   || (current_templates->start->opcode_modifier.opcodespace
+		       != SPACE_0F)) /* lar, lsl */
 	    suffix = WORD_MNEM_SUFFIX;
 	  break;
 
--- a/gas/testsuite/gas/i386/intel.d
+++ b/gas/testsuite/gas/i386/intel.d
@@ -698,6 +698,14 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	0f 4b 90 90 90 90 90 	cmovnp -0x6f6f6f70\(%eax\),%edx
 [ 	]*[a-f0-9]+:	66 0f 4a 90 90 90 90 90 	cmovp  -0x6f6f6f70\(%eax\),%dx
 [ 	]*[a-f0-9]+:	66 0f 4b 90 90 90 90 90 	cmovnp -0x6f6f6f70\(%eax\),%dx
+[ 	]*[a-f0-9]+:	0f 02 c0             	lar    %eax,%eax
+[ 	]*[a-f0-9]+:	66 0f 02 c0          	lar    %ax,%ax
+[ 	]*[a-f0-9]+:	0f 02 00             	lar    \(%eax\),%eax
+[ 	]*[a-f0-9]+:	66 0f 02 00          	lar    \(%eax\),%ax
+[ 	]*[a-f0-9]+:	0f 03 c0             	lsl    %eax,%eax
+[ 	]*[a-f0-9]+:	66 0f 03 c0          	lsl    %ax,%ax
+[ 	]*[a-f0-9]+:	0f 03 00             	lsl    \(%eax\),%eax
+[ 	]*[a-f0-9]+:	66 0f 03 00          	lsl    \(%eax\),%ax
 [ 	]*[a-f0-9]+:	8b 04 04             	mov    \(%esp,%eax(,1)?\),%eax
 [ 	]*[a-f0-9]+:	8b 04 20             	mov    \(%eax(,%eiz)?(,1)?\),%eax
 [ 	]*[a-f0-9]+:	c4 e2 69 92 04 08    	vgatherdps %xmm2,\(%eax,%xmm1(,1)?\),%xmm0
--- a/gas/testsuite/gas/i386/intel.s
+++ b/gas/testsuite/gas/i386/intel.s
@@ -699,6 +699,16 @@ fidivr  dword ptr [ebx]
  cmovpe  dx, 0x90909090[eax]
  cmovpo dx, 0x90909090[eax]
 
+	lar	eax, eax
+	lar	ax, ax
+	lar	eax, word ptr [eax]
+	lar	ax, word ptr [eax]
+
+	lsl	eax, eax
+	lsl	ax, ax
+	lsl	eax, word ptr [eax]
+	lsl	ax, word ptr [eax]
+
  # Check base/index swapping
 	.allow_index_reg
  mov    eax, [eax+esp]
--- a/gas/testsuite/gas/i386/intel-intel.d
+++ b/gas/testsuite/gas/i386/intel-intel.d
@@ -232,8 +232,8 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	e5 90 +	in     eax,0x90
 [ 	]*[a-f0-9]+:	e6 90 +	out    0x90,al
 [ 	]*[a-f0-9]+:	e7 90 +	out    0x90,eax
-[ 	]*[a-f0-9]+:	e8 90 90 90 90 +	call   90909373 <barn\+0x90908831>
-[ 	]*[a-f0-9]+:	e9 90 90 90 90 +	jmp    90909378 <barn\+0x90908836>
+[ 	]*[a-f0-9]+:	e8 90 90 90 90 +	call   90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	e9 90 90 90 90 +	jmp    90909... <barn\+0x90908...>
 [ 	]*[a-f0-9]+:	ea 90 90 90 90 90 90 	jmp    0x9090:0x90909090
 [ 	]*[a-f0-9]+:	eb 90 +	jmp    281 <foo\+0x281>
 [ 	]*[a-f0-9]+:	ec +	in     al,dx
@@ -308,22 +308,22 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	0f 77 +	emms
 [ 	]*[a-f0-9]+:	0f 7e 90 90 90 90 90 	movd   DWORD PTR \[eax-0x6f6f6f70\],mm2
 [ 	]*[a-f0-9]+:	0f 7f 90 90 90 90 90 	movq   QWORD PTR \[eax-0x6f6f6f70\],mm2
-[ 	]*[a-f0-9]+:	0f 80 90 90 90 90 +	jo     909094e6 <barn\+0x909089a4>
-[ 	]*[a-f0-9]+:	0f 81 90 90 90 90 +	jno    909094ec <barn\+0x909089aa>
-[ 	]*[a-f0-9]+:	0f 82 90 90 90 90 +	jb     909094f2 <barn\+0x909089b0>
-[ 	]*[a-f0-9]+:	0f 83 90 90 90 90 +	jae    909094f8 <barn\+0x909089b6>
-[ 	]*[a-f0-9]+:	0f 84 90 90 90 90 +	je     909094fe <barn\+0x909089bc>
-[ 	]*[a-f0-9]+:	0f 85 90 90 90 90 +	jne    90909504 <barn\+0x909089c2>
-[ 	]*[a-f0-9]+:	0f 86 90 90 90 90 +	jbe    9090950a <barn\+0x909089c8>
-[ 	]*[a-f0-9]+:	0f 87 90 90 90 90 +	ja     90909510 <barn\+0x909089ce>
-[ 	]*[a-f0-9]+:	0f 88 90 90 90 90 +	js     90909516 <barn\+0x909089d4>
-[ 	]*[a-f0-9]+:	0f 89 90 90 90 90 +	jns    9090951c <barn\+0x909089da>
-[ 	]*[a-f0-9]+:	0f 8a 90 90 90 90 +	jp     90909522 <barn\+0x909089e0>
-[ 	]*[a-f0-9]+:	0f 8b 90 90 90 90 +	jnp    90909528 <barn\+0x909089e6>
-[ 	]*[a-f0-9]+:	0f 8c 90 90 90 90 +	jl     9090952e <barn\+0x909089ec>
-[ 	]*[a-f0-9]+:	0f 8d 90 90 90 90 +	jge    90909534 <barn\+0x909089f2>
-[ 	]*[a-f0-9]+:	0f 8e 90 90 90 90 +	jle    9090953a <barn\+0x909089f8>
-[ 	]*[a-f0-9]+:	0f 8f 90 90 90 90 +	jg     90909540 <barn\+0x909089fe>
+[ 	]*[a-f0-9]+:	0f 80 90 90 90 90 +	jo     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 81 90 90 90 90 +	jno    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 82 90 90 90 90 +	jb     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 83 90 90 90 90 +	jae    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 84 90 90 90 90 +	je     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 85 90 90 90 90 +	jne    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 86 90 90 90 90 +	jbe    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 87 90 90 90 90 +	ja     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 88 90 90 90 90 +	js     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 89 90 90 90 90 +	jns    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 8a 90 90 90 90 +	jp     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 8b 90 90 90 90 +	jnp    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 8c 90 90 90 90 +	jl     90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 8d 90 90 90 90 +	jge    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 8e 90 90 90 90 +	jle    90909... <barn\+0x90908...>
+[ 	]*[a-f0-9]+:	0f 8f 90 90 90 90 +	jg     90909... <barn\+0x90908...>
 [ 	]*[a-f0-9]+:	0f 90 80 90 90 90 90 	seto   BYTE PTR \[eax-0x6f6f6f70\]
 [ 	]*[a-f0-9]+:	0f 91 80 90 90 90 90 	setno  BYTE PTR \[eax-0x6f6f6f70\]
 [ 	]*[a-f0-9]+:	0f 92 80 90 90 90 90 	setb   BYTE PTR \[eax-0x6f6f6f70\]
@@ -532,7 +532,7 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	66 d3 90 90 90 90 90 	rcl    WORD PTR \[eax-0x6f6f6f70\],cl
 [ 	]*[a-f0-9]+:	66 e5 90 +	in     ax,0x90
 [ 	]*[a-f0-9]+:	66 e7 90 +	out    0x90,ax
-[ 	]*[a-f0-9]+:	66 e8 8f 90 +	callw  9922 <barn\+0x8de0>
+[ 	]*[a-f0-9]+:	66 e8 8f 90 +	callw  9... <barn\+0x8...>
 [ 	]*[a-f0-9]+:	66 ea 90 90 90 90 +	jmp    0x9090:0x9090
 [ 	]*[a-f0-9]+:	66 ed +	in     ax,dx
 [ 	]*[a-f0-9]+:	66 ef +	out    dx,ax
@@ -699,6 +699,14 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	0f 4b 90 90 90 90 90 	cmovnp edx,DWORD PTR \[eax-0x6f6f6f70\]
 [ 	]*[a-f0-9]+:	66 0f 4a 90 90 90 90 90 	cmovp  dx,WORD PTR \[eax-0x6f6f6f70\]
 [ 	]*[a-f0-9]+:	66 0f 4b 90 90 90 90 90 	cmovnp dx,WORD PTR \[eax-0x6f6f6f70\]
+[ 	]*[a-f0-9]+:	0f 02 c0 +	lar    eax,eax
+[ 	]*[a-f0-9]+:	66 0f 02 c0 +	lar    ax,ax
+[ 	]*[a-f0-9]+:	0f 02 00 +	lar    eax,WORD PTR \[eax\]
+[ 	]*[a-f0-9]+:	66 0f 02 00 +	lar    ax,WORD PTR \[eax\]
+[ 	]*[a-f0-9]+:	0f 03 c0 +	lsl    eax,eax
+[ 	]*[a-f0-9]+:	66 0f 03 c0 +	lsl    ax,ax
+[ 	]*[a-f0-9]+:	0f 03 00 +	lsl    eax,WORD PTR \[eax\]
+[ 	]*[a-f0-9]+:	66 0f 03 00 +	lsl    ax,WORD PTR \[eax\]
 [ 	]*[a-f0-9]+:	8b 04 04 +	mov    eax,DWORD PTR \[esp\+eax\*1\]
 [ 	]*[a-f0-9]+:	8b 04 20 +	mov    eax,DWORD PTR \[eax\+eiz\*1\]
 [ 	]*[a-f0-9]+:	c4 e2 69 92 04 08 +	vgatherdps xmm0,DWORD PTR \[eax\+xmm1\*1\],xmm2
--- a/gas/testsuite/gas/i386/intelbad.l
+++ b/gas/testsuite/gas/i386/intelbad.l
@@ -161,3 +161,11 @@
 .*:181: Error: .*
 .*:183: Error: .*
 .*:184: Error: .*
+.*:186: Error: .*
+.*:187: Error: .*
+.*:188: Error: .*
+.*:189: Error: .*
+.*:191: Error: .*
+.*:192: Error: .*
+.*:193: Error: .*
+.*:194: Error: .*
--- a/gas/testsuite/gas/i386/intelbad.s
+++ b/gas/testsuite/gas/i386/intelbad.s
@@ -182,3 +182,13 @@ start:
 
 	fild	far ptr [ebx]
 	fist	near ptr [ebx]
+
+	lar	eax, ax
+	lar	ax, eax
+	lar	eax, dword ptr [eax]
+	lar	ax, dword ptr [eax]
+
+	lsl	eax, ax
+	lsl	ax, eax
+	lsl	eax, dword ptr [eax]
+	lsl	ax, dword ptr [eax]
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -833,6 +833,8 @@ enum
   MOD_0F01_REG_3,
   MOD_0F01_REG_5,
   MOD_0F01_REG_7,
+  MOD_0F02,
+  MOD_0F03,
   MOD_0F12_PREFIX_0,
   MOD_0F12_PREFIX_2,
   MOD_0F13,
@@ -2115,8 +2117,8 @@ static const struct dis386 dis386_twobyt
   /* 00 */
   { REG_TABLE (REG_0F00 ) },
   { REG_TABLE (REG_0F01 ) },
-  { "larS",		{ Gv, Ew }, 0 },
-  { "lslS",		{ Gv, Ew }, 0 },
+  { MOD_TABLE (MOD_0F02) },
+  { MOD_TABLE (MOD_0F03) },
   { Bad_Opcode },
   { "syscall",		{ XX }, 0 },
   { "clts",		{ XX }, 0 },
@@ -8198,6 +8200,16 @@ static const struct dis386 mod_table[][2
     { RM_TABLE (RM_0F01_REG_7_MOD_3) },
   },
   {
+    /* MOD_0F02 */
+    { "larS",		{ Gv, Mw }, 0 },
+    { "larS",		{ Gv, Ev }, 0 },
+  },
+  {
+    /* MOD_0F03 */
+    { "lslS",		{ Gv, Mw }, 0 },
+    { "lslS",		{ Gv, Ev }, 0 },
+  },
+  {
     /* MOD_0F12_PREFIX_0 */
     { "movlpX",		{ XM, EXq }, 0 },
     { "movhlps",	{ XM, EXq }, 0 },
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -574,14 +574,16 @@ nop, 0x90, None, 0, NoSuf|RepPrefixOk, {
 
 // Protection control.
 arpl, 0x63, None, Cpu286|CpuNo64, Modrm|IgnoreSize|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16, Reg16|Word|Unspecified|BaseIndex }
-lar, 0xf02, None, Cpu286, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+lar, 0xf02, None, Cpu286, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64, Reg16|Reg32|Reg64 }
+lar, 0xf02, None, Cpu286, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Word|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 lgdt, 0xf01, 2, Cpu286|CpuNo64, Modrm|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Fword|Unspecified|BaseIndex }
 lgdt, 0xf01, 2, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64, { Tbyte|Unspecified|BaseIndex }
 lidt, 0xf01, 3, Cpu286|CpuNo64, Modrm|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Fword|Unspecified|BaseIndex }
 lidt, 0xf01, 3, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|NoRex64, { Tbyte|Unspecified|BaseIndex }
 lldt, 0xf00, 2, Cpu286, Modrm|IgnoreSize|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex }
 lmsw, 0xf01, 6, Cpu286, Modrm|IgnoreSize|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex }
-lsl, 0xf03, None, Cpu286, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+lsl, 0xf03, None, Cpu286, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64, Reg16|Reg32|Reg64 }
+lsl, 0xf03, None, Cpu286, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Word|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 ltr, 0xf00, 3, Cpu286, Modrm|IgnoreSize|No_bSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex }
 
 sgdt, 0xf01, 0, Cpu286|CpuNo64, Modrm|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Fword|Unspecified|BaseIndex }


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] x86: add missing CheckRegSize
  2022-11-23 10:32 [PATCH 0/3] x86: correct checking of matching operand sizes Jan Beulich
  2022-11-23 10:33 ` [PATCH 1/3] x86: correct handling of LAR and LSL Jan Beulich
@ 2022-11-23 10:34 ` Jan Beulich
  2022-11-23 10:35 ` [PATCH 3/3] x86: widen applicability and use of CheckRegSize Jan Beulich
  2022-11-23 21:39 ` [PATCH 0/3] x86: correct checking of matching operand sizes H.J. Lu
  3 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2022-11-23 10:34 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

To properly and predictably determine operand size encoding (operand
size or REX.W prefixes), consistent operand sizes need to be specified.
Add CheckRegSize where this was previously missing.

--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -928,11 +928,11 @@ ud2, 0xf0b, None, Cpu186, NoSuf, {}
 // alias for ud2
 ud2a, 0xf0b, None, Cpu186, NoSuf, {}
 // 2nd. official undefined instr.
-ud1, 0xfb9, None, Cpu186, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+ud1, 0xfb9, None, Cpu186, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 // alias for ud1
-ud2b, 0xfb9, None, Cpu186, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+ud2b, 0xfb9, None, Cpu186, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 // 3rd official undefined instr (older CPUs don't take a ModR/M byte)
-ud0, 0xfff, None, Cpu186, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+ud0, 0xfff, None, Cpu186, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 
 cmov<cc>, 0xf4<cc:opc>, None, CpuCMOV, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] x86: widen applicability and use of CheckRegSize
  2022-11-23 10:32 [PATCH 0/3] x86: correct checking of matching operand sizes Jan Beulich
  2022-11-23 10:33 ` [PATCH 1/3] x86: correct handling of LAR and LSL Jan Beulich
  2022-11-23 10:34 ` [PATCH 2/3] x86: add missing CheckRegSize Jan Beulich
@ 2022-11-23 10:35 ` Jan Beulich
  2022-11-29 23:57   ` H.J. Lu
  2022-11-23 21:39 ` [PATCH 0/3] x86: correct checking of matching operand sizes H.J. Lu
  3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2022-11-23 10:35 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

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)?

--- 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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/3] x86: correct checking of matching operand sizes
  2022-11-23 10:32 [PATCH 0/3] x86: correct checking of matching operand sizes Jan Beulich
                   ` (2 preceding siblings ...)
  2022-11-23 10:35 ` [PATCH 3/3] x86: widen applicability and use of CheckRegSize Jan Beulich
@ 2022-11-23 21:39 ` H.J. Lu
  2022-11-24  8:38   ` Jan Beulich
  3 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2022-11-23 21:39 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Wed, Nov 23, 2022 at 2:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> I've spotted a few cases where operand sizes matching wasn't really
> checked, leading to malformed insn/operand combinations to be
> accepted (in the first patch another anomaly is also taken care of).
> This mainly, but not only affects Intel Syntax.
>
> 1: correct handling of LAR and LSL
> 2: add missing CheckRegSize
> 3: widen applicability and use of CheckRegSize
>
> Jan

OK to all.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/3] x86: correct checking of matching operand sizes
  2022-11-23 21:39 ` [PATCH 0/3] x86: correct checking of matching operand sizes H.J. Lu
@ 2022-11-24  8:38   ` Jan Beulich
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2022-11-24  8:38 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 23.11.2022 22:39, H.J. Lu wrote:
> On Wed, Nov 23, 2022 at 2:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> I've spotted a few cases where operand sizes matching wasn't really
>> checked, leading to malformed insn/operand combinations to be
>> accepted (in the first patch another anomaly is also taken care of).
>> This mainly, but not only affects Intel Syntax.
>>
>> 1: correct handling of LAR and LSL
>> 2: add missing CheckRegSize
>> 3: widen applicability and use of CheckRegSize
>>
>> Jan
> 
> OK to all.

Thanks. I've committed these, but I wonder whether you may have overlooked
the remark in the last patch as to the name "CheckRegSize".

Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] x86: widen applicability and use of CheckRegSize
  2022-11-23 10:35 ` [PATCH 3/3] x86: widen applicability and use of CheckRegSize Jan Beulich
@ 2022-11-29 23:57   ` H.J. Lu
  0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2022-11-29 23:57 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

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.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-11-29 23:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 10:32 [PATCH 0/3] x86: correct checking of matching operand sizes Jan Beulich
2022-11-23 10:33 ` [PATCH 1/3] x86: correct handling of LAR and LSL Jan Beulich
2022-11-23 10:34 ` [PATCH 2/3] x86: add missing CheckRegSize Jan Beulich
2022-11-23 10:35 ` [PATCH 3/3] x86: widen applicability and use of CheckRegSize Jan Beulich
2022-11-29 23:57   ` H.J. Lu
2022-11-23 21:39 ` [PATCH 0/3] x86: correct checking of matching operand sizes H.J. Lu
2022-11-24  8:38   ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).