This is the mail archive of the binutils-cvs@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] x86/Intel: don't accept bogus instructions


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=83b16ac69492ab493bfc87f147bf84c167bc6f30

commit 83b16ac69492ab493bfc87f147bf84c167bc6f30
Author: Jan Beulich <jbeulich@novell.com>
Date:   Fri Jul 1 09:03:02 2016 +0200

    x86/Intel: don't accept bogus instructions
    
    ... due to their last byte looking like a suffix, when after its
    stripping a matching instruction can be found. Since memory operand
    size specifiers in Intel mode get converted into suffix representation
    internally, we need to keep track of the actual mnemonic suffix which
    may have got trimmed off, and check its validity while looking for a
    matching template. I tripper over this quite some time again after
    support for AMD's SSE5 instructions got removed, as at that point some
    of the SSE5 mnemonics, other than expected, didn't fail to assemble.
    But the problem affects many more instructions, namely (almost) all
    MMX, SSE, and AVX ones as it looks. I don't think it makes sense to
    add a testcase covering all of them, nor do I think it makes sense to
    pick out some random examples for a new test case.

Diff:
---
 gas/ChangeLog                       | 10 ++++++++++
 gas/config/tc-i386.c                | 32 +++++++++++++++++++++++++++-----
 gas/testsuite/gas/i386/i386.exp     |  2 ++
 gas/testsuite/gas/i386/suffix-bad.l | 15 +++++++++++++++
 gas/testsuite/gas/i386/suffix-bad.s | 18 ++++++++++++++++++
 5 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index db25ef2..2e14703 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,5 +1,15 @@
 2016-07-01  Jan Beulich  <jbeulich@suse.com>
 
+	PR gas/20318
+	* config/tc-i386.c (match_template): Add char parameter,
+	consumed in Intel mode for an extra suffix check.
+	(md_assemble): New local variable mnem_suffix.
+	* testsuite/gas/i386/suffix-bad.s: New.
+	* testsuite/gas/i386/suffix-bad.l: New.
+	* testsuite/gas/i386/i386.exp: Run new test (twice).
+
+2016-07-01  Jan Beulich  <jbeulich@suse.com>
+
 	* testsuite/gas/i386/movz.s: New.
 	* testsuite/gas/i386/movz32.d: New.
 	* testsuite/gas/i386/movz64.d: New.
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index d85dcd9..9f1b7f0 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -173,7 +173,7 @@ static void swap_operands (void);
 static void swap_2_operands (int, int);
 static void optimize_imm (void);
 static void optimize_disp (void);
-static const insn_template *match_template (void);
+static const insn_template *match_template (char);
 static int check_string (void);
 static int process_suffix (void);
 static int check_byte_reg (void);
@@ -3537,7 +3537,7 @@ void
 md_assemble (char *line)
 {
   unsigned int j;
-  char mnemonic[MAX_MNEM_SIZE];
+  char mnemonic[MAX_MNEM_SIZE], mnem_suffix;
   const insn_template *t;
 
   /* Initialize globals.  */
@@ -3555,6 +3555,7 @@ md_assemble (char *line)
   line = parse_insn (line, mnemonic);
   if (line == NULL)
     return;
+  mnem_suffix = i.suffix;
 
   line = parse_operands (line, mnemonic);
   this_operand = -1;
@@ -3600,7 +3601,7 @@ md_assemble (char *line)
      making sure the overlap of the given operands types is consistent
      with the template operand types.  */
 
-  if (!(t = match_template ()))
+  if (!(t = match_template (mnem_suffix)))
     return;
 
   if (sse_check != check_none
@@ -4724,14 +4725,14 @@ VEX_check_operands (const insn_template *t)
 }
 
 static const insn_template *
-match_template (void)
+match_template (char mnem_suffix)
 {
   /* Points to template once we've found it.  */
   const insn_template *t;
   i386_operand_type overlap0, overlap1, overlap2, overlap3;
   i386_operand_type overlap4;
   unsigned int found_reverse_match;
-  i386_opcode_modifier suffix_check;
+  i386_opcode_modifier suffix_check, mnemsuf_check;
   i386_operand_type operand_types [MAX_OPERANDS];
   int addr_prefix_disp;
   unsigned int j;
@@ -4760,6 +4761,19 @@ match_template (void)
   else if (i.suffix == LONG_DOUBLE_MNEM_SUFFIX)
     suffix_check.no_ldsuf = 1;
 
+  memset (&mnemsuf_check, 0, sizeof (mnemsuf_check));
+  if (intel_syntax)
+    {
+      switch (mnem_suffix)
+	{
+	case BYTE_MNEM_SUFFIX:  mnemsuf_check.no_bsuf = 1; break;
+	case WORD_MNEM_SUFFIX:  mnemsuf_check.no_wsuf = 1; break;
+	case SHORT_MNEM_SUFFIX: mnemsuf_check.no_ssuf = 1; break;
+	case LONG_MNEM_SUFFIX:  mnemsuf_check.no_lsuf = 1; break;
+	case QWORD_MNEM_SUFFIX: mnemsuf_check.no_qsuf = 1; break;
+	}
+    }
+
   /* Must have right number of operands.  */
   i.error = number_of_operands_mismatch;
 
@@ -4805,6 +4819,14 @@ match_template (void)
 	      || (t->opcode_modifier.no_qsuf && suffix_check.no_qsuf)
 	      || (t->opcode_modifier.no_ldsuf && suffix_check.no_ldsuf)))
 	continue;
+      /* In Intel mode all mnemonic suffixes must be explicitly allowed.  */
+      if ((t->opcode_modifier.no_bsuf && mnemsuf_check.no_bsuf)
+	  || (t->opcode_modifier.no_wsuf && mnemsuf_check.no_wsuf)
+	  || (t->opcode_modifier.no_lsuf && mnemsuf_check.no_lsuf)
+	  || (t->opcode_modifier.no_ssuf && mnemsuf_check.no_ssuf)
+	  || (t->opcode_modifier.no_qsuf && mnemsuf_check.no_qsuf)
+	  || (t->opcode_modifier.no_ldsuf && mnemsuf_check.no_ldsuf))
+	continue;
 
       if (!operand_size_match (t))
 	continue;
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index 871004b..6e00262 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -76,6 +76,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
     run_dump_test "smx"
     run_dump_test "suffix"
     run_dump_test "suffix-intel"
+    run_list_test "suffix-bad"
     run_dump_test "immed32"
     run_dump_test "equ"
     run_dump_test "divide"
@@ -571,6 +572,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
     run_dump_test "x86-64-disp32"
     run_dump_test "rexw"
     run_list_test "x86-64-specific-reg"
+    run_list_test "suffix-bad"
     run_list_test "x86-64-suffix-bad"
     run_dump_test "x86-64-fxsave"
     run_dump_test "x86-64-fxsave-intel"
diff --git a/gas/testsuite/gas/i386/suffix-bad.l b/gas/testsuite/gas/i386/suffix-bad.l
new file mode 100644
index 0000000..10868c3
--- /dev/null
+++ b/gas/testsuite/gas/i386/suffix-bad.l
@@ -0,0 +1,15 @@
+.*: Assembler messages:
+.*:3: Error: .*
+.*:4: Error: .*
+.*:5: Error: .*
+.*:6: Error: .*
+.*:7: Error: .*
+.*:8: Error: .*
+.*:9: Error: .*
+.*:12: Error: .*
+.*:13: Error: .*
+.*:14: Error: .*
+.*:15: Error: .*
+.*:16: Error: .*
+.*:17: Error: .*
+.*:18: Error: .*
diff --git a/gas/testsuite/gas/i386/suffix-bad.s b/gas/testsuite/gas/i386/suffix-bad.s
new file mode 100644
index 0000000..fceffe9
--- /dev/null
+++ b/gas/testsuite/gas/i386/suffix-bad.s
@@ -0,0 +1,18 @@
+	.text
+suffix:
+	phadddb %xmm0, %xmm1
+	phadddd %xmm0, %xmm1
+	phadddl %xmm0, %xmm1
+	phadddld %xmm0, %xmm1
+	phadddq %xmm0, %xmm1
+	phaddds %xmm0, %xmm1
+	phadddw %xmm0, %xmm1
+
+	.intel_syntax noprefix
+	phadddb xmm0, xmm1
+	phadddd xmm0, xmm1
+	phadddl xmm0, xmm1
+	phadddld xmm0, xmm1
+	phadddq xmm0, xmm1
+	phaddds xmm0, xmm1
+	phadddw xmm0, xmm1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]