This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] i386: Only check suffix in instruction mnemonic
On Thu, Nov 7, 2019 at 11:41 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 07.11.2019 22:52, H.J. Lu wrote:
> > Another problem is Intel syntax sets instruction suffix from operand size:
>
> An unrelated one, but yes.
>
> > [hjl@gnu-cfl-1 gas]$ cat 2.s
> > .intel_syntax noprefix
> > call WORD PTR [rax]
> > [hjl@gnu-cfl-1 gas]$ as -mintel64 -o 2.o 2.s
> > 2.s: Assembler messages:
> > 2.s:2: Error: invalid instruction suffix for `call'
> > [hjl@gnu-cfl-1 gas]$
> >
> > It is operand size mismatch, not invalid instruction suffix. In case
> > of CMPSD and
> > and MOVSD,the 'l' suffix is invalid, not DWORD on memory operand. But suffix
> > check treats them the same. This makes Intel syntax more complicated. Should
> > Intel syntax disallow suffix in mnemonic? It will make Intel syntax
> > closer to SDM.
>
> There are some suffixes that need honoring. The original authors
> of the code apparently decided that it's better to accept some
> stray/bogus suffixes than to further complicate the code. I guess
> once we have the No_*Suf attributes put straight (as mentioned by
> Michael the other day, matching plans I've been having), then we
> could see about further cleaning up Intel syntax behavior here.
>
I am checking in this.
We should check suffix in instruction mnemonic when matching instruction.
In Intel syntax, normally we check for memory operand size. But the same
mnemonic with 2 different encodings can have the same memory operand
size and i.suffix is set to LONG_DOUBLE_MNEM_SUFFIX from memory operand
size in Intel syntax to distinguish them. When there is no suffix in
mnemonic, we check LONG_DOUBLE_MNEM_SUFFIX in i.suffix for mnemonic
suffix.
gas/
PR gas/25167
* config/tc-i386.c (match_template): Don't check instruction
suffix set from operand.
* testsuite/gas/i386/code16.d: New file.
* testsuite/gas/i386/code16.s: Likewise.
* testsuite/gas/i386/i386.exp: Run code16.
opcodes/
PR gas/25167
* i386-opc.tbl: Remove IgnoreSize from cmpsd and movsd.
* i386-tbl.h: Regenerated.
--
H.J.
From 6689643c27353ddb3f17e9b6692744582be63281 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 8 Nov 2019 07:30:38 -0800
Subject: [PATCH] i386: Only check suffix in instruction mnemonic
We should check suffix in instruction mnemonic when matching instruction.
In Intel syntax, normally we check for memory operand size. But the same
mnemonic with 2 different encodings can have the same memory operand
size and i.suffix is set to LONG_DOUBLE_MNEM_SUFFIX from memory operand
size in Intel syntax to distinguish them. When there is no suffix in
mnemonic, we check LONG_DOUBLE_MNEM_SUFFIX in i.suffix for mnemonic
suffix.
gas/
PR gas/25167
* config/tc-i386.c (match_template): Don't check instruction
suffix set from operand.
* testsuite/gas/i386/code16.d: New file.
* testsuite/gas/i386/code16.s: Likewise.
* testsuite/gas/i386/i386.exp: Run code16.
opcodes/
PR gas/25167
* i386-opc.tbl: Remove IgnoreSize from cmpsd and movsd.
* i386-tbl.h: Regenerated.
---
gas/config/tc-i386.c | 75 +++++++++++-------------
gas/testsuite/gas/i386/code16.d | 15 +++++
gas/testsuite/gas/i386/code16.s | 9 +++
gas/testsuite/gas/i386/i386.exp | 1 +
gas/testsuite/gas/i386/x86-64-branch-4.l | 4 +-
opcodes/i386-opc.tbl | 4 +-
opcodes/i386-tbl.h | 4 +-
7 files changed, 64 insertions(+), 48 deletions(-)
create mode 100644 gas/testsuite/gas/i386/code16.d
create mode 100644 gas/testsuite/gas/i386/code16.s
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index ba6b82af18..abbb93a1c1 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5693,7 +5693,7 @@ match_template (char mnem_suffix)
i386_operand_type overlap0, overlap1, overlap2, overlap3;
i386_operand_type overlap4;
unsigned int found_reverse_match;
- i386_opcode_modifier suffix_check, mnemsuf_check;
+ i386_opcode_modifier suffix_check;
i386_operand_type operand_types [MAX_OPERANDS];
int addr_prefix_disp;
unsigned int j;
@@ -5708,33 +5708,33 @@ match_template (char mnem_suffix)
found_reverse_match = 0;
addr_prefix_disp = -1;
+ /* Prepare for mnemonic suffix check. */
memset (&suffix_check, 0, sizeof (suffix_check));
- if (intel_syntax && i.broadcast)
- /* nothing */;
- else if (i.suffix == BYTE_MNEM_SUFFIX)
- suffix_check.no_bsuf = 1;
- else if (i.suffix == WORD_MNEM_SUFFIX)
- suffix_check.no_wsuf = 1;
- else if (i.suffix == SHORT_MNEM_SUFFIX)
- suffix_check.no_ssuf = 1;
- else if (i.suffix == LONG_MNEM_SUFFIX)
- suffix_check.no_lsuf = 1;
- else if (i.suffix == QWORD_MNEM_SUFFIX)
- suffix_check.no_qsuf = 1;
- 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)
{
- 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;
- }
+ case BYTE_MNEM_SUFFIX:
+ suffix_check.no_bsuf = 1;
+ break;
+ case WORD_MNEM_SUFFIX:
+ suffix_check.no_wsuf = 1;
+ break;
+ case SHORT_MNEM_SUFFIX:
+ suffix_check.no_ssuf = 1;
+ break;
+ case LONG_MNEM_SUFFIX:
+ suffix_check.no_lsuf = 1;
+ break;
+ case QWORD_MNEM_SUFFIX:
+ suffix_check.no_qsuf = 1;
+ break;
+ default:
+ /* NB: In Intel syntax, normally we can check for memory operand
+ size when there is no mnemonic suffix. But jmp and call have
+ 2 different encodings with Dword memory operand size, one with
+ No_ldSuf and the other without. i.suffix is set to
+ LONG_DOUBLE_MNEM_SUFFIX to skip the one with No_ldSuf. */
+ if (i.suffix == LONG_DOUBLE_MNEM_SUFFIX)
+ suffix_check.no_ldsuf = 1;
}
/* Must have right number of operands. */
@@ -5768,23 +5768,14 @@ match_template (char mnem_suffix)
|| (!intel64 && t->opcode_modifier.intel64))
continue;
- /* Check the suffix, except for some instructions in intel mode. */
+ /* Check the suffix. */
i.error = invalid_instruction_suffix;
- if ((!intel_syntax || !t->opcode_modifier.ignoresize)
- && ((t->opcode_modifier.no_bsuf && suffix_check.no_bsuf)
- || (t->opcode_modifier.no_wsuf && suffix_check.no_wsuf)
- || (t->opcode_modifier.no_lsuf && suffix_check.no_lsuf)
- || (t->opcode_modifier.no_ssuf && suffix_check.no_ssuf)
- || (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))
+ if ((t->opcode_modifier.no_bsuf && suffix_check.no_bsuf)
+ || (t->opcode_modifier.no_wsuf && suffix_check.no_wsuf)
+ || (t->opcode_modifier.no_lsuf && suffix_check.no_lsuf)
+ || (t->opcode_modifier.no_ssuf && suffix_check.no_ssuf)
+ || (t->opcode_modifier.no_qsuf && suffix_check.no_qsuf)
+ || (t->opcode_modifier.no_ldsuf && suffix_check.no_ldsuf))
continue;
size_match = operand_size_match (t);
diff --git a/gas/testsuite/gas/i386/code16.d b/gas/testsuite/gas/i386/code16.d
new file mode 100644
index 0000000000..b860448d6c
--- /dev/null
+++ b/gas/testsuite/gas/i386/code16.d
@@ -0,0 +1,15 @@
+#objdump: -drw -mi8086
+#name: i386 with .code16
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <.text>:
+ +[a-f0-9]+: f3 66 a5 rep movsl %ds:\(%si\),%es:\(%di\)
+ +[a-f0-9]+: f3 66 a7 repz cmpsl %es:\(%di\),%ds:\(%si\)
+ +[a-f0-9]+: 66 f3 a5 rep movsl %ds:\(%si\),%es:\(%di\)
+ +[a-f0-9]+: 66 f3 a7 repz cmpsl %es:\(%di\),%ds:\(%si\)
+ +[a-f0-9]+: 66 f3 a5 rep movsl %ds:\(%si\),%es:\(%di\)
+ +[a-f0-9]+: 66 f3 a7 repz cmpsl %es:\(%di\),%ds:\(%si\)
+#pass
diff --git a/gas/testsuite/gas/i386/code16.s b/gas/testsuite/gas/i386/code16.s
new file mode 100644
index 0000000000..d18fa5179a
--- /dev/null
+++ b/gas/testsuite/gas/i386/code16.s
@@ -0,0 +1,9 @@
+ .text
+ .code16
+ rep; movsd
+ rep; cmpsd
+ rep movsd %ds:(%si),%es:(%di)
+ rep cmpsd %es:(%di),%ds:(%si)
+ .intel_syntax noprefix
+ rep movsd dword ptr es:[di], dword ptr ds:[si]
+ rep cmpsd dword ptr ds:[si], dword ptr es:[di]
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index ff2d235713..4e7f8b982d 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -132,6 +132,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_32_check]]
run_dump_test "noreg32"
run_dump_test "addr16"
run_dump_test "addr32"
+ run_dump_test "code16"
run_list_test "oversized16" "-al"
run_dump_test "sse4_1"
run_dump_test "sse4_1-intel"
diff --git a/gas/testsuite/gas/i386/x86-64-branch-4.l b/gas/testsuite/gas/i386/x86-64-branch-4.l
index db55394a56..8f681f86d5 100644
--- a/gas/testsuite/gas/i386/x86-64-branch-4.l
+++ b/gas/testsuite/gas/i386/x86-64-branch-4.l
@@ -7,11 +7,11 @@
.*:9: Error: operand type mismatch for `call'
.*:10: Error: invalid instruction suffix for `call'
.*:11: Error: invalid instruction suffix for `call'
-.*:12: Error: invalid instruction suffix for `call'
+.*:12: Error: operand size mismatch for `call'
.*:13: Error: operand type mismatch for `jmp'
.*:14: Error: invalid instruction suffix for `jmp'
.*:15: Error: invalid instruction suffix for `jmp'
-.*:16: Error: invalid instruction suffix for `jmp'
+.*:16: Error: operand size mismatch for `jmp'
GAS LISTING .*
#...
[ ]*1[ ]+\.text
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 6ed794c08a..1dd2783234 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -1405,7 +1405,7 @@ cmppd, 3, 0x66c2, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexVVVV=1|VexW=1|No_bSu
cmppd, 3, 0x660fc2, None, 2, CpuSSE2, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM }
// Intel mode string compare.
cmpsd, 0, 0xa7, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-cmpsd, 2, 0xa7, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex|EsSeg, Dword|Unspecified|BaseIndex }
+cmpsd, 2, 0xa7, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex|EsSeg, Dword|Unspecified|BaseIndex }
cmpsd, 3, 0xf2c2, None, 1, CpuAVX, Modrm|Vex=3|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Imm8, Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
cmpsd, 3, 0xf20fc2, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8, Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
comisd, 2, 0x662f, None, 1, CpuAVX, Modrm|Vex=3|VexOpcode=0|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
@@ -1441,7 +1441,7 @@ movntpd, 2, 0x662b, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexW=1|No_bSuf|No_wSu
movntpd, 2, 0x660f2b, None, 2, CpuSSE2, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM, Xmmword|Unspecified|BaseIndex }
// Intel mode string move.
movsd, 0, 0xa5, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-movsd, 2, 0xa5, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex|EsSeg }
+movsd, 2, 0xa5, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex|EsSeg }
movsd, 2, 0xf210, None, 1, CpuAVX, D|Modrm|Vex=3|VexOpcode=0|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Qword|Unspecified|BaseIndex, RegXMM }
movsd, 2, 0xf210, None, 1, CpuAVX, D|Modrm|Vex=3|VexOpcode=0|VexVVVV=1|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { RegXMM, RegXMM }
movsd, 2, 0xf20f10, None, 2, CpuSSE2, D|Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
diff --git a/opcodes/i386-tbl.h b/opcodes/i386-tbl.h
index b04a98f25b..a4a2cd7336 100644
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -15607,7 +15607,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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 1, 0, 1, 1, 1, 1, 1,
+ { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 1, 1, 1, 1, 1,
1, 0, 1, 0, 0, 0, 0, 0, 0, 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 },
@@ -16134,7 +16134,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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 1, 0, 1, 1, 1, 1, 1,
+ { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 1, 1, 1, 1, 1,
1, 0, 1, 0, 0, 0, 0, 0, 0, 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 },
--
2.21.0