x86: don't ignore mandatory pseudo prefixes

Message ID a958e696-0339-521a-e64f-e350521f8af0@suse.com
State New
Headers show
Series
  • x86: don't ignore mandatory pseudo prefixes
Related show

Commit Message

Jan Beulich June 8, 2020, 6:47 a.m.
{vex}, {vex3}, and {evex} are mandatory prefixes, and hence should not
be randomly ignored. Fix this for insns without operands as well as for
insns referencing the high 16 [XYZ]MM registers. To achieve the former,
re-purpose VEX_check_operands(), renaming it to VEX_check_encoding() and
moving its only operand check to check_VecOperands().

This involves fixing a testcase relying on {vex2} to get ignored.

gas/
2020-06-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (vex_encoding_error): New enumerator.
	(VEX_check_operands): Rename to VEX_check_encoding. Check
	for vex_encoding_error. Move Imm4 handling ...
	(check_VecOperands): ... here.
	(match_template): Call VEX_check_encoding when there are no
	operands. Split construct calling check_VecOperands and
	VEX_check_encoding (when there are operands).
	(check_register): Don't blindly set vex_encoding_evex.
	* testsuite/gas/i386/pseudos-bad.s,
	testsuite/gas/i386/pseudos-bad.l: New.
	* testsuite/gas/i386/i386.exp: Run new test.
	* testsuite/gas/i386/xmmhi64.s: Drop {vex2}.

Comments

Alan Modra via Binutils June 8, 2020, 11:42 a.m. | #1
On Sun, Jun 7, 2020 at 11:47 PM Jan Beulich <jbeulich@suse.com> wrote:
>

> {vex}, {vex3}, and {evex} are mandatory prefixes, and hence should not

> be randomly ignored. Fix this for insns without operands as well as for

> insns referencing the high 16 [XYZ]MM registers. To achieve the former,

> re-purpose VEX_check_operands(), renaming it to VEX_check_encoding() and

> moving its only operand check to check_VecOperands().

>

> This involves fixing a testcase relying on {vex2} to get ignored.

>

> gas/

> 2020-06-XX  Jan Beulich  <jbeulich@suse.com>

>

>         * config/tc-i386.c (vex_encoding_error): New enumerator.

>         (VEX_check_operands): Rename to VEX_check_encoding. Check

>         for vex_encoding_error. Move Imm4 handling ...

>         (check_VecOperands): ... here.

>         (match_template): Call VEX_check_encoding when there are no

>         operands. Split construct calling check_VecOperands and

>         VEX_check_encoding (when there are operands).

>         (check_register): Don't blindly set vex_encoding_evex.

>         * testsuite/gas/i386/pseudos-bad.s,

>         testsuite/gas/i386/pseudos-bad.l: New.

>         * testsuite/gas/i386/i386.exp: Run new test.

>         * testsuite/gas/i386/xmmhi64.s: Drop {vex2}.

>


OK.

Thanks.

-- 
H.J.

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -424,7 +424,8 @@  struct _i386_insn
 	vex_encoding_default = 0,
 	vex_encoding_vex,
 	vex_encoding_vex3,
-	vex_encoding_evex
+	vex_encoding_evex,
+	vex_encoding_error
       } vec_encoding;
 
     /* REP prefix.  */
@@ -5999,6 +6000,20 @@  check_VecOperands (const insn_template *
 	}
     }
 
+  /* Check the special Imm4 cases; must be the first operand.  */
+  if (t->cpu_flags.bitfield.cpuxop && t->operands == 5)
+    {
+      if (i.op[0].imms->X_op != O_constant
+	  || !fits_in_imm4 (i.op[0].imms->X_add_number))
+	{
+	  i.error = bad_imm4;
+	  return 1;
+	}
+
+      /* Turn off Imm<N> so that update_imm won't complain.  */
+      operand_type_set (&i.types[0], 0);
+    }
+
   /* Check vector Disp8 operand.  */
   if (t->opcode_modifier.disp8memshift
       && i.disp_encoding != disp_encoding_32bit)
@@ -6068,12 +6083,17 @@  check_VecOperands (const insn_template *
   return 0;
 }
 
-/* Check if operands are valid for the instruction.  Update VEX
-   operand types.  */
+/* Check if encoding requirements are met by the instruction.  */
 
 static int
-VEX_check_operands (const insn_template *t)
+VEX_check_encoding (const insn_template *t)
 {
+  if (i.vec_encoding == vex_encoding_error)
+    {
+      i.error = unsupported;
+      return 1;
+    }
+
   if (i.vec_encoding == vex_encoding_evex)
     {
       /* This instruction must be encoded with EVEX prefix.  */
@@ -6096,20 +6116,6 @@  VEX_check_operands (const insn_template
       return 0;
     }
 
-  /* Check the special Imm4 cases; must be the first operand.  */
-  if (t->cpu_flags.bitfield.cpuxop && t->operands == 5)
-    {
-      if (i.op[0].imms->X_op != O_constant
-	  || !fits_in_imm4 (i.op[0].imms->X_add_number))
-	{
-	  i.error = bad_imm4;
-	  return 1;
-	}
-
-      /* Turn off Imm<N> so that update_imm won't complain.  */
-      operand_type_set (&i.types[0], 0);
-    }
-
   return 0;
 }
 
@@ -6265,8 +6271,16 @@  match_template (char mnem_suffix)
 
       /* Do not verify operands when there are none.  */
       if (!t->operands)
-	/* We've found a match; break out of loop.  */
-	break;
+	{
+	  if (VEX_check_encoding (t))
+	    {
+	      specific_error = i.error;
+	      continue;
+	    }
+
+	  /* We've found a match; break out of loop.  */
+	  break;
+	}
 
       if (!t->opcode_modifier.jump
 	  || t->opcode_modifier.jump == JUMP_ABSOLUTE)
@@ -6509,8 +6523,15 @@  match_template (char mnem_suffix)
 	     slip through to break.  */
 	}
 
-      /* Check if vector and VEX operands are valid.  */
-      if (check_VecOperands (t) || VEX_check_operands (t))
+      /* Check if vector operands are valid.  */
+      if (check_VecOperands (t))
+	{
+	  specific_error = i.error;
+	  continue;
+	}
+
+      /* Check if VEX/EVEX encoding requirements can be satisfied.  */
+      if (VEX_check_encoding (t))
 	{
 	  specific_error = i.error;
 	  continue;
@@ -12394,7 +12415,10 @@  static bfd_boolean check_register (const
 	  || flag_code != CODE_64BIT)
 	return FALSE;
 
-      i.vec_encoding = vex_encoding_evex;
+      if (i.vec_encoding == vex_encoding_default)
+	i.vec_encoding = vex_encoding_evex;
+      else if (i.vec_encoding != vex_encoding_evex)
+	i.vec_encoding = vex_encoding_error;
     }
 
   if (((r->reg_flags & (RegRex64 | RegRex)) || r->reg_type.bitfield.qword)
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -492,6 +492,7 @@  if [expr ([istarget "i*86-*-*"] ||  [ist
     run_list_test "cet-ibt-inval"
     run_list_test "cet-shstk-inval"
     run_dump_test "pseudos"
+    run_list_test "pseudos-bad"
     run_dump_test "notrack"
     run_dump_test "notrack-intel"
     run_list_test "notrackbad" "-al"
@@ -1074,6 +1075,7 @@  if [expr ([istarget "i*86-*-*"] || [ista
     run_list_test "x86-64-cet-ibt-inval"
     run_list_test "x86-64-cet-shstk-inval"
     run_dump_test "x86-64-pseudos"
+    run_list_test "x86-64-pseudos-bad"
     run_dump_test "x86-64-notrack"
     run_dump_test "x86-64-notrack-intel"
     run_list_test "x86-64-notrackbad" "-al"
--- /dev/null
+++ b/gas/testsuite/gas/i386/pseudos-bad.l
@@ -0,0 +1,9 @@ 
+.*: Assembler messages:
+.*:3: Error: .*`nop'.*
+.*:4: Error: .*`nop'.*
+.*:6: Error: .*`nop'.*
+.*:7: Error: .*`nop'.*
+.*:9: Error: .*`nop'.*
+.*:10: Error: .*`nop'.*
+.*:12: Error: .*`vzeroall'.*
+.*:13: Error: .*`vmovmskps'.*
--- /dev/null
+++ b/gas/testsuite/gas/i386/pseudos-bad.s
@@ -0,0 +1,13 @@ 
+	.text
+pseudos:
+	{vex} nop
+	{vex} nop %eax
+
+	{vex3} nop
+	{vex3} nop %eax
+
+	{evex} nop
+	{evex} nop %eax
+
+	{evex} vzeroall
+	{evex} vmovmskps %xmm0, %eax
--- a/gas/testsuite/gas/i386/xmmhi64.s
+++ b/gas/testsuite/gas/i386/xmmhi64.s
@@ -2,6 +2,6 @@ 
 	.intel_syntax noprefix
 	.code64
 xmm:
-	{vex2} vaddps	xmm0, xmm1, xmm16
-	{vex2} vaddps	ymm0, ymm1, ymm16
-	{vex2} vaddps	zmm0, zmm1, zmm16
+	vaddps	xmm0, xmm1, xmm16
+	vaddps	ymm0, ymm1, ymm16
+	vaddps	zmm0, zmm1, zmm16