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: fix operand checking for MOVSD


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

commit 8325cc6398187c12e0fe04a68a21e4eb5f44fa20
Author: Jan Beulich <jbeulich@novell.com>
Date:   Fri Jul 1 08:56:13 2016 +0200

    x86/Intel: fix operand checking for MOVSD
    
    The dual purpose mnemonic (string move vs scalar double move) breaks
    the assumption that the isstring flag would be set on both the first
    and last entry in the current set of templates, which results in bogus
    or missing diagnostics for the string move variant of the mnemonic.
    Short of mostly rewriting i386_index_check() and its interaction with
    the rest of the code, simply shrink the template set to just string
    instructions when encountering the second memory operand, and run
    i386_index_check() a second time for the first memory operand after
    that reduction.

Diff:
---
 gas/ChangeLog                         | 15 ++++++++++
 gas/config/tc-i386-intel.c            |  4 +++
 gas/config/tc-i386.c                  | 54 ++++++++++++++++++++++++++++++++++-
 gas/testsuite/gas/i386/i386.exp       |  4 ++-
 gas/testsuite/gas/i386/intel-movs.s   | 18 ++++++++++++
 gas/testsuite/gas/i386/intel-movs32.d | 19 ++++++++++++
 gas/testsuite/gas/i386/intel-movs64.d | 22 ++++++++++++++
 opcodes/ChangeLog                     |  5 ++++
 opcodes/i386-opc.tbl                  |  4 +--
 opcodes/i386-tbl.h                    |  4 +--
 10 files changed, 143 insertions(+), 6 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 1e04d5a..a654185 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,18 @@
+2016-07-01  Jan Beulich  <jbeulich@suse.com>
+
+	* config/tc-i386.c (struct _i386_insn): New field memop1_string.
+	(md_assemble): Free first memory operand string.
+	(i386_index_check): Use repprefixok to distingush xlat from
+	other (real) string ops.
+	(maybe_adjust_templates): New.
+	(i386_att_operand). Call it. Store first memory operand string.
+	* config/tc-i386-intel.c (i386_intel_operand): Likewise.
+	* testsuite/gas/i386/intel-movs.s: New.
+	* testsuite/gas/i386/intel-movs32.d: New.
+	* testsuite/gas/i386/intel-movs64.d: New.
+	* testsuite/gas/i386/i386.exp: Run new tests. Invoke as for
+	64-bits tests with "--defsym x86_64=1 --strip-local-absolute".
+
 2016-06-30  Maciej W. Rozycki  <macro@imgtec.com>
 
 	* config/tc-mips.c (get_append_method): Fix a comment typo.
diff --git a/gas/config/tc-i386-intel.c b/gas/config/tc-i386-intel.c
index 418b930..1db778e 100644
--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -821,6 +821,8 @@ i386_intel_operand (char *operand_string, int got_a_float)
 	   || intel_state.is_mem)
     {
       /* Memory operand.  */
+      if (i.mem_operands == 1 && !maybe_adjust_templates ())
+	return 0;
       if ((int) i.mem_operands
 	  >= 2 - !current_templates->start->opcode_modifier.isstring)
 	{
@@ -983,6 +985,8 @@ i386_intel_operand (char *operand_string, int got_a_float)
 	return 0;
 
       i.types[this_operand].bitfield.mem = 1;
+      if (i.mem_operands == 0)
+	i.memop1_string = xstrdup (operand_string);
       ++i.mem_operands;
     }
   else
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 457f557..d85dcd9 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -326,6 +326,9 @@ struct _i386_insn
        explicit segment overrides are given.  */
     const seg_entry *seg[2];
 
+    /* Copied first memory operand string, for re-checking.  */
+    char *memop1_string;
+
     /* PREFIX holds all the given prefix opcodes (usually null).
        PREFIXES is the number of prefix opcodes.  */
     unsigned int prefixes;
@@ -3555,6 +3558,8 @@ md_assemble (char *line)
 
   line = parse_operands (line, mnemonic);
   this_operand = -1;
+  xfree (i.memop1_string);
+  i.memop1_string = NULL;
   if (line == NULL)
     return;
 
@@ -8510,7 +8515,7 @@ i386_index_check (const char *operand_string)
 
       kind = "string address";
 
-      if (current_templates->start->opcode_modifier.w)
+      if (current_templates->start->opcode_modifier.repprefixok)
 	{
 	  i386_operand_type type = current_templates->end[-1].operand_types[0];
 
@@ -8661,6 +8666,49 @@ RC_SAE_immediate (const char *imm_start)
   return 1;
 }
 
+/* Only string instructions can have a second memory operand, so
+   reduce current_templates to just those if it contains any.  */
+static int
+maybe_adjust_templates (void)
+{
+  const insn_template *t;
+
+  gas_assert (i.mem_operands == 1);
+
+  for (t = current_templates->start; t < current_templates->end; ++t)
+    if (t->opcode_modifier.isstring)
+      break;
+
+  if (t < current_templates->end)
+    {
+      static templates aux_templates;
+      bfd_boolean recheck;
+
+      aux_templates.start = t;
+      for (; t < current_templates->end; ++t)
+	if (!t->opcode_modifier.isstring)
+	  break;
+      aux_templates.end = t;
+
+      /* Determine whether to re-check the first memory operand.  */
+      recheck = (aux_templates.start != current_templates->start
+		 || t != current_templates->end);
+
+      current_templates = &aux_templates;
+
+      if (recheck)
+	{
+	  i.mem_operands = 0;
+	  if (i.memop1_string != NULL
+	      && i386_index_check (i.memop1_string) == 0)
+	    return 0;
+	  i.mem_operands = 1;
+	}
+    }
+
+  return 1;
+}
+
 /* Parse OPERAND_STRING into the i386_insn structure I.  Returns zero
    on error.  */
 
@@ -8800,6 +8848,8 @@ i386_att_operand (char *operand_string)
       char *vop_start;
 
     do_memory_reference:
+      if (i.mem_operands == 1 && !maybe_adjust_templates ())
+	return 0;
       if ((i.mem_operands == 1
 	   && !current_templates->start->opcode_modifier.isstring)
 	  || i.mem_operands == 2)
@@ -8977,6 +9027,8 @@ i386_att_operand (char *operand_string)
       if (i386_index_check (operand_string) == 0)
 	return 0;
       i.types[this_operand].bitfield.mem = 1;
+      if (i.mem_operands == 0)
+	i.memop1_string = xstrdup (operand_string);
       i.mem_operands++;
     }
   else
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index c63e81b..393910c 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -401,6 +401,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
 	run_dump_test "att-regs"
 	run_dump_test "intel-got32"
 	run_dump_test "intel-regs"
+	run_dump_test "intel-movs32"
 	run_list_test "inval-equ-1" "-al"
 	run_list_test "inval-equ-2" "-al"
 	run_dump_test "ifunc"
@@ -482,7 +483,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
 
     global ASFLAGS
     set old_ASFLAGS "$ASFLAGS"
-    set ASFLAGS "$ASFLAGS --64"
+    set ASFLAGS "$ASFLAGS --64 --defsym x86_64=1 --strip-local-absolute"
 
     run_dump_test "x86_64"
     run_dump_test "x86_64-intel"
@@ -564,6 +565,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
     run_dump_test "x86-64-sib-intel"
     run_dump_test "x86-64-disp"
     run_dump_test "x86-64-disp-intel"
+    run_dump_test "intel-movs64"
     run_dump_test "x86-64-disp32"
     run_dump_test "rexw"
     run_list_test "x86-64-specific-reg"
diff --git a/gas/testsuite/gas/i386/intel-movs.s b/gas/testsuite/gas/i386/intel-movs.s
new file mode 100644
index 0000000..2ea1c96
--- /dev/null
+++ b/gas/testsuite/gas/i386/intel-movs.s
@@ -0,0 +1,18 @@
+	.text
+	.intel_syntax noprefix
+
+movs:
+	movsb
+	movsb	es:[edi], [esi]
+	movsb	es:[edi], fs:[esi]
+	movsw
+	movsw	es:[edi], [esi]
+	movsw	es:[edi], fs:[esi]
+	movsd
+	movsd	es:[edi], [esi]
+	movsd	es:[edi], fs:[esi]
+.ifdef x86_64
+	movsq
+	movsq	es:[rdi], [rsi]
+	movsq	es:[rdi], fs:[rsi]
+.endif
diff --git a/gas/testsuite/gas/i386/intel-movs32.d b/gas/testsuite/gas/i386/intel-movs32.d
new file mode 100644
index 0000000..43d5033
--- /dev/null
+++ b/gas/testsuite/gas/i386/intel-movs32.d
@@ -0,0 +1,19 @@
+#objdump: -dMintel
+#source: intel-movs.s
+#name: x86 Intel movs (32-bit object)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <movs>:
+[ 	]*[a-f0-9]+:	a4 *	movs(b *| +BYTE PTR es:\[edi\],(BYTE PTR )?(ds:)?\[esi\])
+[ 	]*[a-f0-9]+:	a4 *	movs(b *| +BYTE PTR es:\[edi\],(BYTE PTR )?(ds:)?\[esi\])
+[ 	]*[a-f0-9]+:	64 a4 *	movs +BYTE PTR es:\[edi\],(BYTE PTR )?fs:\[esi\]
+[ 	]*[a-f0-9]+:	66 a5 *	movs(w *| +WORD PTR es:\[edi\],(WORD PTR )?(ds:)?\[esi\])
+[ 	]*[a-f0-9]+:	66 a5 *	movs(w *| +WORD PTR es:\[edi\],(WORD PTR )?(ds:)?\[esi\])
+[ 	]*[a-f0-9]+:	64 66 a5 *	movs +WORD PTR es:\[edi\],(WORD PTR )?fs:\[esi\]
+[ 	]*[a-f0-9]+:	a5 *	movs(d *| +DWORD PTR es:\[edi\],(DWORD PTR )?(ds:)?\[esi\])
+[ 	]*[a-f0-9]+:	a5 *	movs(d *| +DWORD PTR es:\[edi\],(DWORD PTR )?(ds:)?\[esi\])
+[ 	]*[a-f0-9]+:	64 a5 *	movs +DWORD PTR es:\[edi\],(DWORD PTR )?fs:?\[esi\]
+#pass
diff --git a/gas/testsuite/gas/i386/intel-movs64.d b/gas/testsuite/gas/i386/intel-movs64.d
new file mode 100644
index 0000000..b61be72
--- /dev/null
+++ b/gas/testsuite/gas/i386/intel-movs64.d
@@ -0,0 +1,22 @@
+#objdump: -dMintel
+#source: intel-movs.s
+#name: x86 Intel movs (64-bit object)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <movs>:
+[ 	]*[a-f0-9]+:	a4 *	movs(b *| +BYTE PTR es:\[rdi\],(BYTE PTR )?(ds:)?\[rsi\])
+[ 	]*[a-f0-9]+:	67 a4 *	movs +BYTE PTR es:\[edi\],(BYTE PTR )?(ds:)?\[esi\]
+[ 	]*[a-f0-9]+:	64 67 a4 *	movs +BYTE PTR es:\[edi\],(BYTE PTR )?fs:\[esi\]
+[ 	]*[a-f0-9]+:	66 a5 *	movs(w *| +WORD PTR es:\[rdi\],(WORD PTR )?(ds:)?\[rsi\])
+[ 	]*[a-f0-9]+:	67 66 a5 *	movs +WORD PTR es:\[edi\],(WORD PTR )?(ds:)?\[esi\]
+[ 	]*[a-f0-9]+:	64 67 66 a5 *	movs +WORD PTR es:\[edi\],(WORD PTR )?fs:\[esi\]
+[ 	]*[a-f0-9]+:	a5 *	movs(d *| +DWORD PTR es:\[rdi\],(DWORD PTR )?(ds:)?\[rsi\])
+[ 	]*[a-f0-9]+:	67 a5 *	movs +DWORD PTR es:\[edi\],(DWORD PTR )?(ds:)?\[esi\]
+[ 	]*[a-f0-9]+:	64 67 a5 *	movs +DWORD PTR es:\[edi\],(DWORD PTR )?fs:\[esi\]
+[ 	]*[a-f0-9]+:	48 a5 *	movs(q *| +QWORD PTR es:\[rdi\],(QWORD PTR )?(ds:)?\[rsi\])
+[ 	]*[a-f0-9]+:	48 a5 *	movs(q *| +QWORD PTR es:\[rdi\],(QWORD PTR )?(ds:)?\[rsi\])
+[ 	]*[a-f0-9]+:	64 48 a5 *	movs +QWORD PTR es:\[rdi\],(QWORD PTR )?fs:?\[rsi\]
+#pass
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index c2cdb49..f171baa 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,8 @@
+2016-07-01  Jan Beulich  <jbeulich@suse.com>
+
+	* i386-opc.tbl (xlat): Remove RepPrefixOk.
+	* i386-tbl.h: Re-generate.
+
 2016-06-30  Yao Qi  <yao.qi@linaro.org>
 
 	* arm-dis.c (print_insn): Fix typo in comment.
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 91f5b4a..2d1dc33 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -474,8 +474,8 @@ stos, 2, 0xaa, None, 1, 0, W|CheckRegSize|No_sSuf|No_ldSuf|IsString|RepPrefixOk,
 ssto, 0, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
 ssto, 1, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|EsSeg }
 ssto, 2, 0xaa, None, 1, 0, W|CheckRegSize|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Acc|Byte|Word|Dword|Qword, Byte|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|EsSeg }
-xlat, 0, 0xd7, None, 1, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-xlat, 1, 0xd7, None, 1, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
+xlat, 0, 0xd7, None, 1, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString, { 0 }
+xlat, 1, 0xd7, None, 1, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString, { Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
 
 // Bit manipulation.
 bsf, 2, 0xfbc, None, 2, Cpu386, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf|RepPrefixOk, { Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S, Reg16|Reg32|Reg64 }
diff --git a/opcodes/i386-tbl.h b/opcodes/i386-tbl.h
index 66f0074..142e447 100644
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -4985,7 +4985,7 @@ const insn_template i386_optab[] =
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
     { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
-      1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
+      1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0 },
     { { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
@@ -4998,7 +4998,7 @@ const insn_template i386_optab[] =
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } },
     { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
-      1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
+      1, 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0 },
     { { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,


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