From 7b94647ad00bb096348ea074fa2f19e1873eb0a7 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 30 Sep 2022 10:13:39 +0200 Subject: [PATCH] x86: improve match_template()'s diagnostics At the example of extractps $0, %xmm0, %xmm0 insertps $0, %xmm0, %eax (both having respectively the same mistake of using the wrong kind of destination register) it is easy to see that current behavior is far from ideal: The former results in "unsupported instruction" for 32-bit code simply because the 2nd template we have is a Cpu64 one. Instead we should aim at emitting the "best" possible error, which will typically be the one where we passed the largest number of checks. Generalize the original "specific_error" approach by making it apply to the entire matching loop, utilizing that line numbers increase as we pass further checks. --- gas/config/tc-i386.c | 81 ++++++++++++++--------- gas/testsuite/gas/i386/inval-tls.l | 4 +- gas/testsuite/gas/i386/noavx512-1.l | 14 ++-- gas/testsuite/gas/i386/noavx512-2.l | 8 +-- gas/testsuite/gas/i386/x86-64-branch-4.l | 10 +-- gas/testsuite/gas/i386/x86-64-branch-5.l | 32 ++++----- gas/testsuite/gas/i386/x86-64-inval-tls.l | 4 +- 7 files changed, 86 insertions(+), 67 deletions(-) diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c index ac60fd5b1e5..3435cb5699d 100644 --- a/gas/config/tc-i386.c +++ b/gas/config/tc-i386.c @@ -2083,12 +2083,7 @@ operand_size_match (const insn_template *t) } if (!t->opcode_modifier.d) - { - mismatch: - if (!match) - i.error = operand_size_mismatch; - return match; - } + return match; /* Check reverse. */ gas_assert ((i.operands >= 2 && i.operands <= 3) @@ -2105,19 +2100,19 @@ operand_size_match (const insn_template *t) if (t->operand_types[j].bitfield.class == Reg && !match_operand_size (t, j, given)) - goto mismatch; + return match; if (t->operand_types[j].bitfield.class == RegSIMD && !match_simd_size (t, j, given)) - goto mismatch; + return match; if (t->operand_types[j].bitfield.instance == Accum && (!match_operand_size (t, j, given) || !match_simd_size (t, j, given))) - goto mismatch; + return match; if ((i.flags[given] & Operand_Mem) && !match_mem_size (t, j, given)) - goto mismatch; + return match; } return match | MATCH_REVERSE; @@ -6386,6 +6381,17 @@ VEX_check_encoding (const insn_template *t) return 0; } +/* Helper function for the progress() macro in match_template(). */ +static INLINE enum i386_error progress (enum i386_error new, + enum i386_error last, + unsigned int line, unsigned int *line_p) +{ + if (line <= *line_p) + return last; + *line_p = line; + return new; +} + static const insn_template * match_template (char mnem_suffix) { @@ -6397,8 +6403,9 @@ match_template (char mnem_suffix) i386_opcode_modifier suffix_check; i386_operand_type operand_types [MAX_OPERANDS]; int addr_prefix_disp; - unsigned int j, size_match, check_register; - enum i386_error specific_error = 0; + unsigned int j, size_match, check_register, errline = __LINE__; + enum i386_error specific_error = number_of_operands_mismatch; +#define progress(err) progress (err, specific_error, __LINE__, &errline) #if MAX_OPERANDS != 5 # error "MAX_OPERANDS must be 5." @@ -6436,36 +6443,33 @@ match_template (char mnem_suffix) suffix_check.no_ldsuf = 1; } - /* Must have right number of operands. */ - i.error = number_of_operands_mismatch; - for (t = current_templates->start; t < current_templates->end; t++) { addr_prefix_disp = -1; found_reverse_match = 0; + /* Must have right number of operands. */ if (i.operands != t->operands) continue; /* Check processor support. */ - i.error = unsupported; + specific_error = progress (unsupported); if (cpu_flags_match (t) != CPU_FLAGS_PERFECT_MATCH) continue; /* Check Pseudo Prefix. */ - i.error = unsupported; if (t->opcode_modifier.pseudovexprefix && !(i.vec_encoding == vex_encoding_vex || i.vec_encoding == vex_encoding_vex3)) continue; /* Check AT&T mnemonic. */ - i.error = unsupported_with_intel_mnemonic; + specific_error = progress (unsupported_with_intel_mnemonic); if (intel_mnemonic && t->opcode_modifier.attmnemonic) continue; /* Check AT&T/Intel syntax. */ - i.error = unsupported_syntax; + specific_error = progress (unsupported_syntax); if ((intel_syntax && t->opcode_modifier.attsyntax) || (!intel_syntax && t->opcode_modifier.intelsyntax)) continue; @@ -6491,7 +6495,7 @@ match_template (char mnem_suffix) } /* Check the suffix. */ - i.error = invalid_instruction_suffix; + specific_error = progress (invalid_instruction_suffix); 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) @@ -6500,6 +6504,7 @@ match_template (char mnem_suffix) || (t->opcode_modifier.no_ldsuf && suffix_check.no_ldsuf)) continue; + specific_error = progress (operand_size_mismatch); size_match = operand_size_match (t); if (!size_match) continue; @@ -6510,11 +6515,9 @@ match_template (char mnem_suffix) as the case of a missing * on the operand is accepted (perhaps with a warning, issued further down). */ + specific_error = progress (operand_type_mismatch); if (i.jumpabsolute && t->opcode_modifier.jump != JUMP_ABSOLUTE) - { - i.error = operand_type_mismatch; - continue; - } + continue; for (j = 0; j < MAX_OPERANDS; j++) operand_types[j] = t->operand_types[j]; @@ -6522,6 +6525,8 @@ match_template (char mnem_suffix) /* In general, don't allow - 64-bit operands outside of 64-bit mode, - 32-bit operands on pre-386. */ + specific_error = progress (mnem_suffix ? invalid_instruction_suffix + : operand_size_mismatch); j = i.imm_operands + (t->operands > i.imm_operands + 1); if (((i.suffix == QWORD_MNEM_SUFFIX && flag_code != CODE_64BIT @@ -6550,7 +6555,7 @@ match_template (char mnem_suffix) { if (VEX_check_encoding (t)) { - specific_error = i.error; + specific_error = progress (i.error); continue; } @@ -6711,6 +6716,8 @@ match_template (char mnem_suffix) i.types[1], operand_types[1]))) { + specific_error = progress (i.error); + /* Check if other direction is valid ... */ if (!t->opcode_modifier.d) continue; @@ -6735,6 +6742,7 @@ match_template (char mnem_suffix) operand_types[0]))) { /* Does not match either direction. */ + specific_error = progress (i.error); continue; } /* found_reverse_match holds which of D or FloatR @@ -6773,7 +6781,10 @@ match_template (char mnem_suffix) operand_types[3], i.types[4], operand_types[4])) - continue; + { + specific_error = progress (i.error); + continue; + } /* Fall through. */ case 4: overlap3 = operand_type_and (i.types[3], operand_types[3]); @@ -6788,7 +6799,10 @@ match_template (char mnem_suffix) operand_types[2], i.types[3], operand_types[3]))) - continue; + { + specific_error = progress (i.error); + continue; + } /* Fall through. */ case 3: overlap2 = operand_type_and (i.types[2], operand_types[2]); @@ -6803,7 +6817,10 @@ match_template (char mnem_suffix) operand_types[1], i.types[2], operand_types[2]))) - continue; + { + specific_error = progress (i.error); + continue; + } break; } } @@ -6814,14 +6831,14 @@ match_template (char mnem_suffix) /* Check if vector operands are valid. */ if (check_VecOperands (t)) { - specific_error = i.error; + specific_error = progress (i.error); continue; } /* Check if VEX/EVEX encoding requirements can be satisfied. */ if (VEX_check_encoding (t)) { - specific_error = i.error; + specific_error = progress (i.error); continue; } @@ -6829,11 +6846,13 @@ match_template (char mnem_suffix) break; } +#undef progress + if (t == current_templates->end) { /* We found no match. */ const char *err_msg; - switch (specific_error ? specific_error : i.error) + switch (specific_error) { default: abort (); diff --git a/gas/testsuite/gas/i386/inval-tls.l b/gas/testsuite/gas/i386/inval-tls.l index dc8a326390c..59e7c30f46f 100644 --- a/gas/testsuite/gas/i386/inval-tls.l +++ b/gas/testsuite/gas/i386/inval-tls.l @@ -1,3 +1,3 @@ .*: Assembler messages: -.*:3: Error: operand size mismatch for `kmovd' -.*:4: Error: operand size mismatch for `kmovd' +.*:3: Error: .* `kmovd' +.*:4: Error: .* `kmovd' diff --git a/gas/testsuite/gas/i386/noavx512-1.l b/gas/testsuite/gas/i386/noavx512-1.l index ece92113a36..15a6fc689ba 100644 --- a/gas/testsuite/gas/i386/noavx512-1.l +++ b/gas/testsuite/gas/i386/noavx512-1.l @@ -1,14 +1,14 @@ .*: Assembler messages: -.*:25: Error: .*unsupported instruction.* +.*:25: Error: .*operand size mismatch.* .*:26: Error: .*unsupported masking.* .*:27: Error: .*unsupported masking.* -.*:47: Error: .*unsupported instruction.* +.*:47: Error: .*operand size mismatch.* .*:48: Error: .*unsupported masking.* .*:49: Error: .*unsupported masking.* .*:50: Error: .*not supported.* .*:51: Error: .*not supported.* .*:52: Error: .*not supported.* -.*:69: Error: .*unsupported instruction.* +.*:69: Error: .*operand size mismatch.* .*:70: Error: .*unsupported masking.* .*:71: Error: .*unsupported masking.* .*:72: Error: .*not supported.* @@ -17,7 +17,7 @@ .*:75: Error: .*not supported.* .*:76: Error: .*not supported.* .*:77: Error: .*not supported.* -.*:91: Error: .*unsupported instruction.* +.*:91: Error: .*operand size mismatch.* .*:92: Error: .*unsupported masking.* .*:93: Error: .*unsupported masking.* .*:94: Error: .*not supported.* @@ -27,7 +27,7 @@ .*:98: Error: .*not supported.* .*:99: Error: .*not supported.* .*:100: Error: .*not supported.* -.*:113: Error: .*unsupported instruction.* +.*:113: Error: .*operand size mismatch.* .*:114: Error: .*unsupported masking.* .*:115: Error: .*unsupported masking.* .*:116: Error: .*not supported.* @@ -40,7 +40,7 @@ .*:126: Error: .*not supported.* .*:127: Error: .*not supported.* .*:128: Error: .*not supported.* -.*:135: Error: .*unsupported instruction.* +.*:135: Error: .*operand size mismatch.* .*:136: Error: .*unsupported masking.* .*:137: Error: .*unsupported masking.* .*:138: Error: .*not supported.* @@ -54,7 +54,7 @@ .*:149: Error: .*not supported.* .*:150: Error: .*not supported.* .*:151: Error: .*not supported.* -.*:157: Error: .*unsupported instruction.* +.*:157: Error: .*operand size mismatch.* .*:158: Error: .*unsupported masking.* .*:159: Error: .*unsupported masking.* .*:160: Error: .*not supported.* diff --git a/gas/testsuite/gas/i386/noavx512-2.l b/gas/testsuite/gas/i386/noavx512-2.l index 7bc4502618d..02c92e0d8db 100644 --- a/gas/testsuite/gas/i386/noavx512-2.l +++ b/gas/testsuite/gas/i386/noavx512-2.l @@ -1,12 +1,12 @@ .*: Assembler messages: -.*:26: Error: .*unsupported instruction.* -.*:27: Error: .*unsupported instruction.* +.*:26: Error: .*unsupported masking.* +.*:27: Error: .*unsupported masking.* .*:29: Error: .*unsupported instruction.* .*:30: Error: .*unsupported instruction.* .*:32: Error: .*unsupported instruction.* .*:33: Error: .*unsupported instruction.* -.*:36: Error: .*unsupported instruction.* -.*:37: Error: .*unsupported instruction.* +.*:36: Error: .*unsupported masking.* +.*:37: Error: .*unsupported masking.* .*:39: Error: .*unsupported instruction.* .*:40: Error: .*unsupported instruction.* .*:43: Error: .*unsupported instruction.* diff --git a/gas/testsuite/gas/i386/x86-64-branch-4.l b/gas/testsuite/gas/i386/x86-64-branch-4.l index 54e32308c2e..a2a78497c00 100644 --- a/gas/testsuite/gas/i386/x86-64-branch-4.l +++ b/gas/testsuite/gas/i386/x86-64-branch-4.l @@ -1,19 +1,19 @@ .*: Assembler messages: .*:2: Error: invalid instruction suffix for `call' .*:3: Error: invalid instruction suffix for `call' -.*:4: Error: operand type mismatch for `jmp' +.*:4: Error: operand (size|type) mismatch for `jmp' .*:5: Error: invalid instruction suffix for `jmp' .*:6: Error: invalid instruction suffix for `jmp' .*:7: Error: invalid instruction suffix for `ret' .*:8: Error: invalid instruction suffix for `ret' -.*:11: Error: operand type mismatch for `call' +.*:11: Error: operand (size|type) mismatch for `call' .*:12: Error: invalid instruction suffix for `call' .*:13: Error: invalid instruction suffix for `call' -.*:14: Error: operand size mismatch for `call' -.*:15: Error: operand type mismatch for `jmp' +.*:14: Error: operand (size|type) mismatch for `call' +.*:15: Error: operand (size|type) mismatch for `jmp' .*:16: Error: invalid instruction suffix for `jmp' .*:17: Error: invalid instruction suffix for `jmp' -.*:18: Error: operand size mismatch for `jmp' +.*:18: Error: operand (size|type) mismatch for `jmp' .*:19: Error: invalid instruction suffix for `ret' .*:20: Error: invalid instruction suffix for `ret' GAS LISTING .* diff --git a/gas/testsuite/gas/i386/x86-64-branch-5.l b/gas/testsuite/gas/i386/x86-64-branch-5.l index 188b6c2184b..61944b2b6f0 100644 --- a/gas/testsuite/gas/i386/x86-64-branch-5.l +++ b/gas/testsuite/gas/i386/x86-64-branch-5.l @@ -1,19 +1,19 @@ .*: Assembler messages: -.*:2: Error: unsupported syntax for `lcall' -.*:3: Error: unsupported syntax for `lfs' -.*:4: Error: unsupported syntax for `lfs' -.*:5: Error: unsupported syntax for `lgs' -.*:6: Error: unsupported syntax for `lgs' -.*:7: Error: unsupported syntax for `ljmp' -.*:8: Error: unsupported syntax for `lss' -.*:9: Error: unsupported syntax for `lss' -.*:12: Error: unsupported syntax for `call' -.*:13: Error: unsupported syntax for `lfs' -.*:14: Error: unsupported syntax for `lfs' -.*:15: Error: unsupported syntax for `lgs' -.*:16: Error: unsupported syntax for `lgs' -.*:17: Error: unsupported syntax for `jmp' -.*:18: Error: unsupported syntax for `lss' -.*:19: Error: unsupported syntax for `lss' +.*:2: Error: invalid instruction suffix for `lcall' +.*:3: Error: operand size mismatch for `lfs' +.*:4: Error: invalid instruction suffix for `lfs' +.*:5: Error: operand size mismatch for `lgs' +.*:6: Error: invalid instruction suffix for `lgs' +.*:7: Error: invalid instruction suffix for `ljmp' +.*:8: Error: operand size mismatch for `lss' +.*:9: Error: invalid instruction suffix for `lss' +.*:12: Error: operand (size|type) mismatch for `call' +.*:13: Error: operand size mismatch for `lfs' +.*:14: Error: operand size mismatch for `lfs' +.*:15: Error: operand size mismatch for `lgs' +.*:16: Error: operand size mismatch for `lgs' +.*:17: Error: operand (size|type) mismatch for `jmp' +.*:18: Error: operand size mismatch for `lss' +.*:19: Error: operand size mismatch for `lss' GAS LISTING .* #pass diff --git a/gas/testsuite/gas/i386/x86-64-inval-tls.l b/gas/testsuite/gas/i386/x86-64-inval-tls.l index 11fa63e459c..4256e6219a4 100644 --- a/gas/testsuite/gas/i386/x86-64-inval-tls.l +++ b/gas/testsuite/gas/i386/x86-64-inval-tls.l @@ -1,3 +1,3 @@ .*: Assembler messages: -.*:3: Error: operand size mismatch for `kmovq' -.*:4: Error: operand size mismatch for `kmovq' +.*:3: Error: .* `kmovq' +.*:4: Error: .* `kmovq' -- 2.30.2