From 3ac21baa8498d3aa9951f79e2c3336d532eeff7b Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 16 Jul 2018 08:19:21 +0200 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/ChangeLog | 16 +++++ gas/config/tc-i386.c | 115 +++++++++++++++++---------------- gas/testsuite/gas/i386/inval.l | 73 ++++++++++++--------- gas/testsuite/gas/i386/inval.s | 5 ++ 4 files changed, 123 insertions(+), 86 deletions(-) diff --git a/gas/ChangeLog b/gas/ChangeLog index 770c44c8d2..7e5f27d671 100644 --- a/gas/ChangeLog +++ b/gas/ChangeLog @@ -1,3 +1,19 @@ +2018-07-16 Jan Beulich + + * 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. + 2018-07-13 Nick Clifton * testsuite/gas/elf/missing-build-notes.s: New test. Checks that diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c index baecdecd9a..fd2e81740d 100644 --- a/gas/config/tc-i386.c +++ b/gas/config/tc-i386.c @@ -1901,70 +1901,74 @@ operand_type_check (i386_operand_type t, enum operand_type c) 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 1 if there is no size conflict on any operands for - instruction template T. */ +/* 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). */ -static INLINE int +#define MATCH_STRAIGHT 1 +#define MATCH_REVERSE 2 + +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 *t) 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]); diff --git a/gas/testsuite/gas/i386/inval.l b/gas/testsuite/gas/i386/inval.l index 53eb7e7353..d5d8500bbe 100644 --- 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 diff --git a/gas/testsuite/gas/i386/inval.s b/gas/testsuite/gas/i386/inval.s index 35d99cd65b..dbf8b965a1 100644 --- 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]