[PATCH v2.5 01.5/17] x86: accept whitespace around prefix separator

Jan Beulich jbeulich@suse.com
Fri Jul 19 13:22:57 GMT 2024


... and prediction suffix comma. Other than documented /**/ comments
currently aren't really converted to a single space, at least not for
x86 in its most common configurations. That'll be fixed subsequently, at
which point blanks may appear where so far none were expected.
Furthermore not permitting blanks around these separators wasn't quite
logical anyway - such constructs are composite ones, and hence
components ought to have been permitted to be separated by whitespace
from the very beginning. Furthermore note how, due to the scrubber being
overly aggressive in removing whitespace, some similar construct with a
prefix were already accepted.

Note how certain other checks in parse_insn() can be simplified as a
result.

While there for the prediction suffix also make checks case-insensitive
and check for a proper trailing separator.
---
v2.5: New.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -7958,6 +7958,8 @@ parse_insn (const char *line, char *mnem
 
   while (1)
     {
+      const char *split;
+
       mnem_p = mnemonic;
       /* Pseudo-prefixes start with an opening figure brace.  */
       if ((*mnem_p = *l) == '{')
@@ -7982,9 +7984,10 @@ parse_insn (const char *line, char *mnem
 	    }
 	  l++;
 	}
-      /* Pseudo-prefixes end with a closing figure brace.  */
-      if (*mnemonic == '{' && is_space_char (*l))
+      split = l;
+      if (is_space_char (*l))
 	++l;
+      /* Pseudo-prefixes end with a closing figure brace.  */
       if (*mnemonic == '{' && *l == '}')
 	{
 	  *mnem_p++ = *l++;
@@ -7992,12 +7995,10 @@ parse_insn (const char *line, char *mnem
 	    goto too_long;
 	  *mnem_p = '\0';
 
-	  /* Point l at the closing brace if there's no other separator.  */
-	  if (*l != END_OF_INSN && !is_space_char (*l)
-	      && *l != PREFIX_SEPARATOR)
-	    --l;
+	  if (is_space_char (*l))
+	    ++l;
 	}
-      else if (!is_space_char (*l)
+      else if (l == split
 	       && *l != END_OF_INSN
 	       && (intel_syntax
 		   || (*l != PREFIX_SEPARATOR && *l != ',')))
@@ -8005,7 +8006,7 @@ parse_insn (const char *line, char *mnem
 	  if (mode != parse_all)
 	    break;
 	  as_bad (_("invalid character %s in mnemonic"),
-		  output_invalid (*l));
+		  output_invalid (*split));
 	  return NULL;
 	}
       if (token_start == l)
@@ -8021,7 +8022,6 @@ parse_insn (const char *line, char *mnem
       op_lookup (mnemonic);
 
       if (*l != END_OF_INSN
-	  && (!is_space_char (*l) || l[1] != END_OF_INSN)
 	  && current_templates.start
 	  && current_templates.start->opcode_modifier.isprefix)
 	{
@@ -8143,7 +8143,10 @@ parse_insn (const char *line, char *mnem
 		}
 	    }
 	  /* Skip past PREFIX_SEPARATOR and reset token_start.  */
-	  token_start = ++l;
+	  l += (!intel_syntax && *l == PREFIX_SEPARATOR);
+	  if (is_space_char (*l))
+	    ++l;
+	  token_start = l;
 	}
       else
 	break;
@@ -8235,8 +8238,7 @@ parse_insn (const char *line, char *mnem
 		}
 	      /* For compatibility reasons accept MOVSD and CMPSD without
 	         operands even in AT&T mode.  */
-	      else if (*l == END_OF_INSN
-		       || (is_space_char (*l) && l[1] == END_OF_INSN))
+	      else if (*l == END_OF_INSN)
 		{
 		  mnem_p[-1] = '\0';
 		  op_lookup (mnemonic);
@@ -8278,8 +8280,9 @@ parse_insn (const char *line, char *mnem
       l += length;
     }
 
-  if (current_templates.start->opcode_modifier.jump == JUMP
-      || current_templates.start->opcode_modifier.jump == JUMP_BYTE)
+  if ((current_templates.start->opcode_modifier.jump == JUMP
+       || current_templates.start->opcode_modifier.jump == JUMP_BYTE)
+      && *l == ',')
     {
       /* Check for a branch hint.  We allow ",pt" and ",pn" for
 	 predict taken and predict not taken respectively.
@@ -8287,21 +8290,29 @@ parse_insn (const char *line, char *mnem
 	 and jcxz insns (JumpByte) for current Pentium4 chips.  They
 	 may work in the future and it doesn't hurt to accept them
 	 now.  */
-      if (l[0] == ',' && l[1] == 'p')
+      token_start = l++;
+      if (is_space_char (*l))
+	++l;
+      if (TOLOWER (*l) == 'p' && ISALPHA (l[1])
+	  && (l[2] == END_OF_INSN || is_space_char (l[2])))
 	{
-	  if (l[2] == 't')
+	  if (TOLOWER (l[1]) == 't')
 	    {
 	      if (!add_prefix (DS_PREFIX_OPCODE))
 		return NULL;
-	      l += 3;
+	      l += 2;
 	    }
-	  else if (l[2] == 'n')
+	  else if (TOLOWER (l[1]) == 'n')
 	    {
 	      if (!add_prefix (CS_PREFIX_OPCODE))
 		return NULL;
-	      l += 3;
+	      l += 2;
 	    }
+	  else
+	    l = token_start;
 	}
+      else
+	l = token_start;
     }
   /* Any other comma loses.  */
   if (*l == ',')
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -106,6 +106,7 @@ if [gas_32_check] then {
     run_list_test "equ-2" "-al"
     run_list_test "equ-bad"
     run_dump_test "curly"
+    run_dump_test "separator"
     run_dump_test "divide"
     run_dump_test "quoted"
     run_dump_test "quoted2"
--- /dev/null
+++ b/gas/testsuite/gas/i386/separator.d
@@ -0,0 +1,27 @@
+#objdump: -dw
+#name: whitespace around special separators
+
+.*: +file format .*
+
+Disassembly of section \.text:
+
+0+ <separators>:
+[ 	]*[a-f0-9]+:	3e 72 fd +	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 72 fd +	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 72 fd +	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 72 fd +	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 72 fd +	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 0f 82 f9 ff ff ff 	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 0f 82 f9 ff ff ff 	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 0f 82 f9 ff ff ff 	j[cb],pt .*
+[ 	]*[a-f0-9]+:	3e 0f 82 f9 ff ff ff 	j[cb],pt .*
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+[ 	]*[a-f0-9]+:	65 f7 d8 +	gs neg %eax
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/separator.s
@@ -0,0 +1,41 @@
+	.text
+separators:
+	jc,pt		.
+	jc ,pt		.
+	jc, pt		.
+	jc , pt		.
+	jc/**/,/**/pt	.
+
+	{disp32} jc,pt	.
+	{disp32} jc ,pt .
+	{disp32} jc, pt .
+	{disp32} jc , pt .
+
+	# Which block to use depends on whether / starts a comment.
+	.ifeq 1/2
+
+	gs/neg	%eax
+	gs /neg	%eax
+	gs/ neg	%eax
+	gs / neg %eax
+	gs/**///**/neg %eax
+
+	{disp32} gs/neg %eax
+	{disp32} gs /neg %eax
+	{disp32} gs/ neg %eax
+	{disp32} gs / neg %eax
+
+	.else
+
+	gs\neg	%eax
+	gs \neg	%eax
+	gs\ neg	%eax
+	gs \ neg %eax
+	gs/**/\/**/neg %eax
+
+	{disp32} gs\neg %eax
+	{disp32} gs \neg %eax
+	{disp32} gs\ neg %eax
+	{disp32} gs \ neg %eax
+
+	.endif



More information about the Binutils mailing list