Home Original page

[PATCH] ARM: fix macro expansion with a pld insn as a macro argument

Jan Beulich JBeulich@suse.com
Tue Nov 13 07:51:00 GMT 2012 
>>> On 12.11.12 at 19:34, Roland McGrath <mcgrathr@google.com> wrote:
> On Sun, Nov 11, 2012 at 11:45 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> Including the possibility of symbol names now suddenly being
>> allowed to include those characters?
> 
> The change has no such effect.  tc_symbol_chars only affects the APP pass
> (whitespace canonicalization).  It does not change the actual parsing of
> operands at all.
> 
>>> +/* 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[] = "#[]";
>>
>> For your purposes, wouldn't it suffice to just have "[" here? Or
>> otherwise, the comment appears inconsistent with the actual set
>> of characters.
> 
> tc_symbol_chars is described as "characters which may appear in an operand".
> For my test case, just "[" would be sufficient.  But as #[] are characters
> which may appear in an operand, it seemed the appropriate setting.  Since ]
> never appears somewhere where whitespace before it is relevant, it makes no
> difference to include it and it could be omitted if one wanted to explain
> the divergence from the simple documentation of what tc_symbol_chars is for.

Oh, right, it's been too long since I last looked at that. Sorry for
the confusion.

> There is an example where this is relevant for #, but its presence in
> line_comment_chars trumps tc_symbol_chars and so that is still broken:
> 
> 	.macro foo arg, rest:vararg
> 		\rest
> 	.endm
> 		foo r0, svc #123
> 
> But I'm not immediately concerned with this case, so I'm only sending the
> fix for the bug that I cited originally.

That's pretty unfortunate. I'd really like to see problems like this
to be addressed in their entirety, rather than just piecewise -
that's, in my opinion, part of the reason why there are so many
half baked things in binutils, which the unsuspecting way too
easily stumbles across.

>> Along with the first comment above, this seems the wrong
>> approach to me in any case. Instead, I would expect these
>> characters to be treated as operator ones, which ought to have
>> the effect of the elided white space to be benign. Or is there a
>> strict need to have white space between opcode and first
>> operand, even if the boundary is recognizable without?
> 
> This reaction seems to be based on some generally sensible theory of
> parsing, which has nothing to do with how GAS actaully behaves.  The APP
> pass canonicalizes whitespace in a string that still has not been parsed or
> tokenized in any general way.  The string then gets to the actual parsing
> of lines via md_assemble, and that relies on having a space between the
> mnemonic an the first operand.

Fact is that this pre-parsing has been causing problems every
once in a while, but I don't see anyone stepping up and fixing
this altogether (not the least because of how fragile this is, and
hence how high the potential of regressions would be).

Jan
More information about the Binutils mailing list