[PATCH 2/6] x86: improve matching diagnostics

Jan Beulich jbeulich@suse.com
Wed Apr 30 13:30:05 GMT 2025


Many times in the past I was puzzled by seeing "operand size mismatch"
when really "operand type mismatch" would be far more appropriate. As it
turns out, there were at least two flaws: In the single operand case we
didn't propagate i.error to match_template()'s local specific_error when
noticing a type mismatch. And then operand_size_match() was too eager in
invoking match_mem_size(): Especially the Unspecified attribute can get
in the way there when the expected operand isn't a memory one (and hence
Unspecified would not be set in the operand template, whereas it's
uniformly set for memory operands in AT&T syntax).

(In the x86-64-lkgs-inval testcase the particular error for the two
bogus Intel syntax forms doesn't really matter; all we ought to care
about there isthat there is _some_ error.)
---
There are more issues in this area. For example,

	vcvtpd2dq (%rax), %xmm0

is neither a type nor a size mismatch.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2630,7 +2630,9 @@ operand_size_match (const insn_template
 	  break;
 	}
 
-      if ((i.flags[j] & Operand_Mem) && !match_mem_size (t, j, j))
+      if ((i.flags[j] & Operand_Mem)
+	  && operand_type_check (t->operand_types[j], anymem)
+	  && !match_mem_size (t, j, j))
 	{
 	  match = 0;
 	  break;
@@ -2667,7 +2669,9 @@ operand_size_match (const insn_template
 	      || !match_simd_size (t, j, given)))
 	return match;
 
-      if ((i.flags[given] & Operand_Mem) && !match_mem_size (t, j, given))
+      if ((i.flags[given] & Operand_Mem)
+	  && operand_type_check (t->operand_types[j], anymem)
+	  && !match_mem_size (t, j, given))
 	return match;
     }
 
@@ -9363,7 +9367,10 @@ match_template (char mnem_suffix)
 	{
 	case 1:
 	  if (!operand_type_match (overlap0, i.types[0]))
-	    continue;
+	    {
+	      specific_error = progress (i.error);
+	      continue;
+	    }
 
 	  /* Allow the ModR/M encoding to be requested by using the {load} or
 	     {store} pseudo prefix on an applicable insn.  */
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -833,6 +833,7 @@ if {![istarget "i*86-*-elfiamcu"] && [ga
 }
 run_list_test "string-bad" ""
 run_list_test "reg-bad" ""
+run_list_test "inval-type"
 run_list_test "space1" "-al"
 if {![istarget "i*86-*-elfiamcu"]} then {
     run_list_test "xmmword"
--- /dev/null
+++ b/gas/testsuite/gas/i386/inval-type.l
@@ -0,0 +1,6 @@
+.*: Assembler messages:
+.*: Error: operand type mismatch for .bsf.
+.*: Error: operand type mismatch for .bswap.
+.*: Error: operand type mismatch for .bswap.
+.*: Error: operand type mismatch for .movntdqa.
+.*: Error: operand type mismatch for .movntdq.
--- /dev/null
+++ b/gas/testsuite/gas/i386/inval-type.s
@@ -0,0 +1,10 @@
+	.text
+
+# All the following should yield "operand type mismatch" (or something yet more
+# specific), but _not_ "operand size mismatch".
+
+	bsf	%eax, (%eax)
+	bswap	%mm0
+	bswapl	%xmm0
+	movntdqa %xmm0, (%eax)
+	movntdq	(%eax), %xmm0
--- a/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l
+++ b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l
@@ -1,5 +1,5 @@
 .* Assembler messages:
 .*:4: Error: operand size mismatch for `pushp'
 .*:5: Error: operand size mismatch for `popp'
-.*:6: Error: operand size mismatch for `pushp'
-.*:7: Error: operand size mismatch for `popp'
+.*:6: Error: operand type mismatch for `pushp'
+.*:7: Error: operand type mismatch for `popp'
--- a/gas/testsuite/gas/i386/x86-64-lkgs-inval.l
+++ b/gas/testsuite/gas/i386/x86-64-lkgs-inval.l
@@ -5,5 +5,5 @@
 .*:8: Error: invalid instruction suffix for `lkgs'
 .*:11: Error: invalid instruction suffix for `lkgs'
 .*:12: Error: invalid instruction suffix for `lkgs'
-.*:13: Error: invalid instruction suffix for `lkgs'
-.*:14: Error: invalid instruction suffix for `lkgs'
+.*:13: Error: .* for `lkgs'
+.*:14: Error: .* for `lkgs'



More information about the Binutils mailing list