This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] ARM gas: handle {...} operands in macros
- From: Roland McGrath <mcgrathr at google dot com>
- To: Richard Earnshaw <rearnsha at arm dot com>
- Cc: Jan Beulich <JBeulich at suse dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 4 Jun 2013 10:49:48 -0700
- Subject: Re: [PATCH] ARM gas: handle {...} operands in macros
- References: <CAB=4xhrny3RcfC=LewUK30mp_9JsRVK_5cn1mgAu0Tz1tJYtHw at mail dot gmail dot com> <51ACBB25 dot 4050509 at arm dot com> <51ACD93302000078000DAA8B at nat28 dot tlf dot novell dot com> <51ACBDD5 dot 1080502 at arm dot com> <51ACDB8C02000078000DAAB0 at nat28 dot tlf dot novell dot com> <51ACC02D dot 4070007 at arm dot com>
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]