This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] x86: fix operand size checking
- From: "Jan Beulich" <JBeulich at suse dot com>
- To: <binutils at sourceware dot org>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>
- Date: Fri, 13 Jul 2018 08:19:14 -0600
- Subject: [PATCH] x86: fix operand size checking
Currently mov to/from control, debug, and test register insns accept any
size GPR operand (general pattern: templates with D set and both
operands being registers in distinct register files). This is due to
improper checking of the reverse case, including not informing the
caller whether a straight and/or reverse match was successful.
The helper functions need to be told two indexes: One to index the given
operand types array, and the other to index the template one. The caller
must attempt a further straight match only if the function reported a
straight match (and respectively for reverse matches).
gas/
2018-07-13 Jan Beulich <jbeulich@suse.com>
* config/tc-i386.c (match_reg_size): Split second parameter
into two.
(match_simd_size): Likewise.
(match_mem_size): Likewise.
(MATCH_STRAIGHT, MATCH_REVERSE): Define.
(operand_size_match): Change return type. New local variable
"match". Always check for reverse match when opcode_modifier.d
is set.
(match_template) New local variable "size_match". Skip further
matching if operand_size_match() did not report a respective
match.
* testsuite/gas/i386/inval.s: Add control register reads/writes.
* testsuite/gas/i386/inval.l: Adjust expectations.
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -1901,70 +1901,74 @@ operand_type_check (i386_operand_type t,
operand J for instruction template T. */
static INLINE int
-match_reg_size (const insn_template *t, unsigned int j)
+match_reg_size (const insn_template *t, unsigned int wanted, unsigned int given)
{
- return !((i.types[j].bitfield.byte
- && !t->operand_types[j].bitfield.byte)
- || (i.types[j].bitfield.word
- && !t->operand_types[j].bitfield.word)
- || (i.types[j].bitfield.dword
- && !t->operand_types[j].bitfield.dword)
- || (i.types[j].bitfield.qword
- && !t->operand_types[j].bitfield.qword)
- || (i.types[j].bitfield.tbyte
- && !t->operand_types[j].bitfield.tbyte));
+ return !((i.types[given].bitfield.byte
+ && !t->operand_types[wanted].bitfield.byte)
+ || (i.types[given].bitfield.word
+ && !t->operand_types[wanted].bitfield.word)
+ || (i.types[given].bitfield.dword
+ && !t->operand_types[wanted].bitfield.dword)
+ || (i.types[given].bitfield.qword
+ && !t->operand_types[wanted].bitfield.qword)
+ || (i.types[given].bitfield.tbyte
+ && !t->operand_types[wanted].bitfield.tbyte));
}
/* Return 1 if there is no conflict in SIMD register on
operand J for instruction template T. */
static INLINE int
-match_simd_size (const insn_template *t, unsigned int j)
+match_simd_size (const insn_template *t, unsigned int wanted, unsigned int given)
{
- return !((i.types[j].bitfield.xmmword
- && !t->operand_types[j].bitfield.xmmword)
- || (i.types[j].bitfield.ymmword
- && !t->operand_types[j].bitfield.ymmword)
- || (i.types[j].bitfield.zmmword
- && !t->operand_types[j].bitfield.zmmword));
+ return !((i.types[given].bitfield.xmmword
+ && !t->operand_types[wanted].bitfield.xmmword)
+ || (i.types[given].bitfield.ymmword
+ && !t->operand_types[wanted].bitfield.ymmword)
+ || (i.types[given].bitfield.zmmword
+ && !t->operand_types[wanted].bitfield.zmmword));
}
/* Return 1 if there is no conflict in any size on operand J for
instruction template T. */
static INLINE int
-match_mem_size (const insn_template *t, unsigned int j)
+match_mem_size (const insn_template *t, unsigned int wanted, unsigned int given)
{
- return (match_reg_size (t, j)
- && !((i.types[j].bitfield.unspecified
+ return (match_reg_size (t, wanted, given)
+ && !((i.types[given].bitfield.unspecified
&& !i.broadcast
- && !t->operand_types[j].bitfield.unspecified)
- || (i.types[j].bitfield.fword
- && !t->operand_types[j].bitfield.fword)
+ && !t->operand_types[wanted].bitfield.unspecified)
+ || (i.types[given].bitfield.fword
+ && !t->operand_types[wanted].bitfield.fword)
/* For scalar opcode templates to allow register and memory
operands at the same time, some special casing is needed
here. Also for v{,p}broadcast*, {,v}pmov{s,z}*, and
down-conversion vpmov*. */
- || ((t->operand_types[j].bitfield.regsimd
+ || ((t->operand_types[wanted].bitfield.regsimd
&& !t->opcode_modifier.broadcast
- && (t->operand_types[j].bitfield.byte
- || t->operand_types[j].bitfield.word
- || t->operand_types[j].bitfield.dword
- || t->operand_types[j].bitfield.qword))
- ? (i.types[j].bitfield.xmmword
- || i.types[j].bitfield.ymmword
- || i.types[j].bitfield.zmmword)
- : !match_simd_size(t, j))));
-}
+ && (t->operand_types[wanted].bitfield.byte
+ || t->operand_types[wanted].bitfield.word
+ || t->operand_types[wanted].bitfield.dword
+ || t->operand_types[wanted].bitfield.qword))
+ ? (i.types[given].bitfield.xmmword
+ || i.types[given].bitfield.ymmword
+ || i.types[given].bitfield.zmmword)
+ : !match_simd_size(t, wanted, given))));
+}
+
+/* Return value has MATCH_STRAIGHT set if there is no size conflict on any
+ operands for instruction template T, and it has MATCH_REVERSE set if there
+ is no size conflict on any operands for the template with operands reversed
+ (and the template allows for reversing in the first place). */
-/* Return 1 if there is no size conflict on any operands for
- instruction template T. */
+#define MATCH_STRAIGHT 1
+#define MATCH_REVERSE 2
-static INLINE int
+static INLINE unsigned int
operand_size_match (const insn_template *t)
{
- unsigned int j;
- int match = 1;
+ unsigned int j, match = MATCH_STRAIGHT;
/* Don't check jump instructions. */
if (t->opcode_modifier.jump
@@ -1981,59 +1985,57 @@ operand_size_match (const insn_template
continue;
if (t->operand_types[j].bitfield.reg
- && !match_reg_size (t, j))
+ && !match_reg_size (t, j, j))
{
match = 0;
break;
}
if (t->operand_types[j].bitfield.regsimd
- && !match_simd_size (t, j))
+ && !match_simd_size (t, j, j))
{
match = 0;
break;
}
if (t->operand_types[j].bitfield.acc
- && (!match_reg_size (t, j) || !match_simd_size (t, j)))
+ && (!match_reg_size (t, j, j) || !match_simd_size (t, j, j)))
{
match = 0;
break;
}
- if (i.types[j].bitfield.mem && !match_mem_size (t, j))
+ if (i.types[j].bitfield.mem && !match_mem_size (t, j, j))
{
match = 0;
break;
}
}
- if (match)
- return match;
- else if (!t->opcode_modifier.d)
+ if (!t->opcode_modifier.d)
{
mismatch:
- i.error = operand_size_mismatch;
- return 0;
+ if (!match)
+ i.error = operand_size_mismatch;
+ return match;
}
/* Check reverse. */
gas_assert (i.operands == 2);
- match = 1;
for (j = 0; j < 2; j++)
{
if ((t->operand_types[j].bitfield.reg
|| t->operand_types[j].bitfield.acc)
- && !match_reg_size (t, j ? 0 : 1))
+ && !match_reg_size (t, j, !j))
goto mismatch;
- if (i.types[j].bitfield.mem
- && !match_mem_size (t, j ? 0 : 1))
+ if (i.types[!j].bitfield.mem
+ && !match_mem_size (t, j, !j))
goto mismatch;
}
- return match;
+ return match | MATCH_REVERSE;
}
static INLINE int
@@ -5286,7 +5288,7 @@ match_template (char mnem_suffix)
i386_operand_type operand_types [MAX_OPERANDS];
int addr_prefix_disp;
unsigned int j;
- unsigned int found_cpu_match;
+ unsigned int found_cpu_match, size_match;
unsigned int check_register;
enum i386_error specific_error = 0;
@@ -5375,7 +5377,8 @@ match_template (char mnem_suffix)
|| (t->opcode_modifier.no_ldsuf && mnemsuf_check.no_ldsuf))
continue;
- if (!operand_size_match (t))
+ size_match = operand_size_match (t);
+ if (!size_match)
continue;
for (j = 0; j < MAX_OPERANDS; j++)
@@ -5502,6 +5505,8 @@ match_template (char mnem_suffix)
&& i.types[0].bitfield.acc
&& operand_type_check (i.types[1], anymem))
continue;
+ if (!(size_match & MATCH_STRAIGHT))
+ goto check_reverse;
/* If we want store form, we reverse direction of operands. */
if (i.dir_encoding == dir_encoding_store
&& t->opcode_modifier.d)
@@ -5531,6 +5536,8 @@ match_template (char mnem_suffix)
continue;
check_reverse:
+ if (!(size_match & MATCH_REVERSE))
+ continue;
/* Try reversing direction of operands. */
overlap0 = operand_type_and (i.types[0], operand_types[1]);
overlap1 = operand_type_and (i.types[1], operand_types[0]);
--- a/gas/testsuite/gas/i386/inval.l
+++ b/gas/testsuite/gas/i386/inval.l
@@ -59,11 +59,10 @@
.*:63: Error: .*
.*:64: Error: .*
.*:65: Error: .*
-.*:68: Error: .*
-.*:69: Error: .*
-.*:70: Error: .*
-.*:71: Error: .*
-.*:72: Error: .*
+.*:67: Error: .*mov.*
+.*:68: Error: .*mov.*
+.*:69: Error: .*mov.*
+.*:70: Error: .*mov.*
.*:73: Error: .*
.*:74: Error: .*
.*:75: Error: .*
@@ -75,14 +74,19 @@
.*:81: Error: .*
.*:82: Error: .*
.*:83: Error: .*
+.*:84: Error: .*
.*:85: Error: .*
.*:86: Error: .*
.*:87: Error: .*
.*:88: Error: .*
.*:90: Error: .*
-.*:92: Error: .*shl.*
-.*:93: Error: .*rol.*
-.*:94: Error: .*rcl.*
+.*:91: Error: .*
+.*:92: Error: .*
+.*:93: Error: .*
+.*:95: Error: .*
+.*:97: Error: .*shl.*
+.*:98: Error: .*rol.*
+.*:99: Error: .*rcl.*
GAS LISTING .*
@@ -155,30 +159,35 @@ GAS LISTING .*
[ ]*64[ ]+add \(%eiz\), %eax
[ ]*65[ ]+add \(%eax\), %eiz
[ ]*66[ ]+
-[ ]*67[ ]+\.intel_syntax noprefix
-[ ]*68[ ]+cvtsi2ss xmm1,QWORD PTR \[eax\]
-[ ]*69[ ]+cvtsi2sd xmm1,QWORD PTR \[eax\]
-[ ]*70[ ]+cvtsi2ssq xmm1,QWORD PTR \[eax\]
-[ ]*71[ ]+cvtsi2sdq xmm1,QWORD PTR \[eax\]
-[ ]*72[ ]+movq xmm1, XMMWORD PTR \[esp\]
-[ ]*73[ ]+movq xmm1, DWORD PTR \[esp\]
-[ ]*74[ ]+movq xmm1, WORD PTR \[esp\]
-[ ]*75[ ]+movq xmm1, BYTE PTR \[esp\]
-[ ]*76[ ]+movq XMMWORD PTR \[esp\],xmm1
-[ ]*77[ ]+movq DWORD PTR \[esp\],xmm1
-[ ]*78[ ]+movq WORD PTR \[esp\],xmm1
-[ ]*79[ ]+movq BYTE PTR \[esp\],xmm1
-[ ]*80[ ]+fnstsw eax
-[ ]*81[ ]+fnstsw al
-[ ]*82[ ]+fstsw eax
-[ ]*83[ ]+fstsw al
-[ ]*84[ ]+
-[ ]*85[ ]+movsx ax, \[eax\]
-[ ]*86[ ]+movsx eax, \[eax\]
-[ ]*87[ ]+movzx ax, \[eax\]
-[ ]*88[ ]+movzx eax, \[eax\]
-[ ]*89[ ]+
-[ ]*90[ ]+movnti word ptr \[eax\], ax
+[ ]*[1-9][0-9]*[ ]+mov %cr0, %di
+[ ]*[1-9][0-9]*[ ]+mov %ax, %cr7
+[ ]*[1-9][0-9]*[ ]+mov %cr0, %bh
+[ ]*[1-9][0-9]*[ ]+mov %al, %cr7
+[ ]*[1-9][0-9]*[ ]+
+[ ]*[1-9][0-9]*[ ]+\.intel_syntax noprefix
+[ ]*[1-9][0-9]*[ ]+cvtsi2ss xmm1,QWORD PTR \[eax\]
+[ ]*[1-9][0-9]*[ ]+cvtsi2sd xmm1,QWORD PTR \[eax\]
+[ ]*[1-9][0-9]*[ ]+cvtsi2ssq xmm1,QWORD PTR \[eax\]
+[ ]*[1-9][0-9]*[ ]+cvtsi2sdq xmm1,QWORD PTR \[eax\]
+[ ]*[1-9][0-9]*[ ]+movq xmm1, XMMWORD PTR \[esp\]
+[ ]*[1-9][0-9]*[ ]+movq xmm1, DWORD PTR \[esp\]
+[ ]*[1-9][0-9]*[ ]+movq xmm1, WORD PTR \[esp\]
+[ ]*[1-9][0-9]*[ ]+movq xmm1, BYTE PTR \[esp\]
+[ ]*[1-9][0-9]*[ ]+movq XMMWORD PTR \[esp\],xmm1
+[ ]*[1-9][0-9]*[ ]+movq DWORD PTR \[esp\],xmm1
+[ ]*[1-9][0-9]*[ ]+movq WORD PTR \[esp\],xmm1
+[ ]*[1-9][0-9]*[ ]+movq BYTE PTR \[esp\],xmm1
+[ ]*[1-9][0-9]*[ ]+fnstsw eax
+[ ]*[1-9][0-9]*[ ]+fnstsw al
+[ ]*[1-9][0-9]*[ ]+fstsw eax
+[ ]*[1-9][0-9]*[ ]+fstsw al
+[ ]*[1-9][0-9]*[ ]+
+[ ]*[1-9][0-9]*[ ]+movsx ax, \[eax\]
+[ ]*[1-9][0-9]*[ ]+movsx eax, \[eax\]
+[ ]*[1-9][0-9]*[ ]+movzx ax, \[eax\]
+[ ]*[1-9][0-9]*[ ]+movzx eax, \[eax\]
+[ ]*[1-9][0-9]*[ ]+
+[ ]*[1-9][0-9]*[ ]+movnti word ptr \[eax\], ax
[ ]*[1-9][0-9]*[ ]+
[ ]*[1-9][0-9]*[ ]+shl \[eax\], 1
[ ]*[1-9][0-9]*[ ]+rol \[ecx\], 2
--- a/gas/testsuite/gas/i386/inval.s
+++ b/gas/testsuite/gas/i386/inval.s
@@ -64,6 +64,11 @@ movntiw %ax, (%eax)
add (%eiz), %eax
add (%eax), %eiz
+ mov %cr0, %di
+ mov %ax, %cr7
+ mov %cr0, %bh
+ mov %al, %cr7
+
.intel_syntax noprefix
cvtsi2ss xmm1,QWORD PTR [eax]
cvtsi2sd xmm1,QWORD PTR [eax]