This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ARM gas: handle {...} operands in macros


Frankly, we all know there are many things wrong with how GAS does
parsing.  I think the whitespace scrubber is just nutty.  I think
having it run before macro expansion rather than after is just nutty.
But this is the assembler we have.

This change, and the previous ones I've done, are not wrong.  What's
wrong is the notion that these are "symbol characters".  Quite simply,
tc_symbol_chars is a misnomer, as are symbol_chars and
LEX_IS_SYMBOL_COMPONENT.  Their only uses are in the whitespace
scrubber.  What they really are is characters that can appear in an
operand.

If there's a "real problem" it's that we have the whitespace scrubber
at all, instead of just skipping whitespace everywhere it can appear.
The only potential new bugs that can be introduced are places where
'[', ']', '{', or '}' can appear in operands but the operand parsing
code doesn't skip whitespace around them.  I've gone over the code a
bit more exhaustively and added a few more places where whitespace
skipping might be needed in this version of the patch.


Thanks,
Roland


gas/
2013-06-04  Roland McGrath  <mcgrathr@google.com>

	* config/tc-arm.c (arm_symbol_chars): Include '{' and '}'.
	(arm_reg_parse_multi): Skip whitespace first.
	(parse_reg_list): Likewise.
	(parse_vfp_reg_list): Likewise.

gas/testsuite/
2013-06-04  Roland McGrath  <mcgrathr@google.com>

	* gas/arm/macro-pld.s: Add a 'push {r0}' case.
	* gas/arm/macro-pld.d: Update expected output.
	* gas/arm/macro-vld1.s: New file.
	* gas/arm/macro-vld1.d: New file.

--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -323,8 +323,9 @@ static bfd_boolean unified_syntax = FALSE;

 /* An immediate operand can start with #, and ld*, st*, pld operands
    can contain [ and ].  We need to tell APP not to elide whitespace
-   before a [, which can appear as the first operand for pld.  */
-const char arm_symbol_chars[] = "#[]";
+   before a [, which can appear as the first operand for pld.
+   Likewise, a { can appear as the first operand for push, pop, vld*, etc.  */
+const char arm_symbol_chars[] = "#[]{}";

 enum neon_el_type
 {
@@ -1158,6 +1159,8 @@ arm_reg_parse_multi (char **ccp)
   char *p;
   struct reg_entry *reg;

+  skip_whitespace (start);
+
 #ifdef REGISTER_PREFIX
   if (*start != REGISTER_PREFIX)
     return NULL;
@@ -1583,6 +1586,8 @@ parse_reg_list (char ** strp)
   /* We come back here if we get ranges concatenated by '+' or '|'.  */
   do
     {
+      skip_whitespace (str);
+
       another_range = 0;

       if (*str == '{')
@@ -1734,14 +1739,12 @@ parse_vfp_reg_list (char **ccp, unsigned int
*pbase, enum reg_list_els etype)
   unsigned long mask = 0;
   int i;

-  if (*str != '{')
+  if (skip_past_char (&str, '{') == FAIL)
     {
       inst.error = _("expecting {");
       return FAIL;
     }

-  str++;
-
   switch (etype)
     {
     case REGLIST_VFP_S:
@@ -4030,6 +4033,8 @@ s_arm_unwind_save_mmxwcg (void)
   if (*input_line_pointer == '{')
     input_line_pointer++;

+  skip_whitespace (input_line_pointer);
+
   do
     {
       reg = arm_reg_parse (&input_line_pointer, REG_TYPE_MMXWCG);
--- a/gas/testsuite/gas/arm/macro-pld.d
+++ b/gas/testsuite/gas/arm/macro-pld.d
@@ -6,3 +6,4 @@ Disassembly of section \.text:

 0+ <.*>:
 \s*0:\s+f5d0f000\s+pld\s+\[r0\]
+\s*4:\s+e52d0004\s+push\s+{r0}\s*.*
--- a/gas/testsuite/gas/arm/macro-pld.s
+++ b/gas/testsuite/gas/arm/macro-pld.s
@@ -2,3 +2,4 @@
 	\rest
 .endm
 	foo r0, pld [r0]
+	foo r0, push {r0}
--- /dev/null
+++ b/gas/testsuite/gas/arm/macro-vld1.d
@@ -0,0 +1,8 @@
+#objdump: -dr
+
+.*:     file format .*
+
+Disassembly of section \.text:
+
+0+ <.*>:
+\s*0:\s+f420070f\s+vld1.8\s+{d0},\s*\[r0\]
--- /dev/null
+++ b/gas/testsuite/gas/arm/macro-vld1.s
@@ -0,0 +1,9 @@
+	.fpu neon
+        .macro sfi_breg basereg, insn, operands:vararg
+                .macro _sfi_breg_doit B
+                \insn \operands
+                .endm
+                _sfi_breg_doit \basereg
+                .purgem _sfi_breg_doit
+        .endm
+	sfi_breg r0, vld1.8 {d0}, [\B]


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]