[PATCH] gas: Fix \+ expansion for .irp and .irpc

[Fangrui Song]

On 2024-05-16, Hans-Peter Nilsson wrote:
>> Date: Wed, 15 May 2024 19:42:01 -0700
>> From: Fangrui Song <maskray@google.com>
>
>> On 2024-05-16, Hans-Peter Nilsson wrote:
>> >> From: Fangrui Song <maskray@google.com>
>> >> Date: Wed, 15 May 2024 18:30:26 -0700
>> >
>> >> On Wed, May 15, 2024 at 5:47 PM Hans-Peter Nilsson <hp@axis.com> wrote:
>> >> >
>> >> > > Date: Wed, 15 May 2024 00:04:02 -0700
>> >> > > From: Fangrui Song <maskray@google.com>
>> >> >
>> >> > > From: Fangrui Song <maskray@gcc.gnu.org>
>> >> > >
>> >> > > .irp and .irpc receive a null macro_entry.  \+ causes a crash after the
>> >> > > recent \+ support.  Restore the previous behavior.
>> >> >
>> >> > This patch (your commit b1d28350499d) caused the gas macro
>> >> > test again to fail for cris-* e.g. cris-elf.  Please have a
>> >> > look.  Thanks.
>> >> >
>> >> > brgds, H-P
>> >>
>> >> Sorry for the breakage. Does this work for you?
>> >
>> >No.  This test has passed for cris-elf in the past.
>> >
>> >IMO you should instead move your recent additions to that
>> >test to a *new* test.  Never *add* to a test-case!
>> >
>> >brgds, H-P
>>
>> Splitting the test requires some work...
>>
>> How about this patch?
>
>Hmm... but shouldn't '.print "\+"' *work*?  I mean, it works
>for cris-elf in the macro (with the new semantics), so it
>should work "outside" as well (here in .irp), i.e. emit a
>'+'?  There's no special semantics for '+' for cris-elf;
>it's not a line-separator or comment-character or anything.
>Did I misunderstand something here; isn't this a general bug
>(guessing similar to the one Nick fixed) rather than a
>target quirk?
>
>Iff I *did* misunderstand something and there's no bug: from
>the looks of it, I'd say ok from a maintenance view.

\+ is expanded by .macro directives. .irp .irpc .rept don't expand \=
and gas/read.c:5635 specifies that ONLY_STANDARD_ESCAPES targets will
reject unrecognized escaped characters.

% ~/Dev/binutils-gdb/out/cris/gas/as-new c.s
c.s: Assembler messages:
c.s:1: Error: bad escaped character in string
?
% ~/Dev/binutils-gdb/out/z80/gas/as-new c.s
c.s: Assembler messages:
c.s:1: Error: bad escaped character in string
?

>(No, I did not test it, but I don't actually think you asked
>that, because you know the following.  Still, JFTR: in
>binutils there's generally no target access problem to test
>your own patches.  If you're new to cross-testing (surely
>not?) just add "--target cris-elf", build and make check as
>compared to your native build.)
>
>brgds, H-P

I have experience testing targets popular on desktop/server but I have
never learned the full list of targets...

I guess there is probably no way other than enumerating every target and
doing a build...  I have done the following for cris-elf and z80-elf.

   (mkdir out/cris && cd out/cris
   ../../configure --target=cris-elf && make -j 50 all-gas && make -j 50 check-gas RUNTESTFLAGS='macros.exp')

   # some non-macro tests failed (e.g. FAIL: elf section2 list), but they are unrelated to this patch.

   (mkdir out/z80 && cd out/z80
   ../../configure --target=z80-elf && make -j 50 all-gas && make -j 50 check-gas RUNTESTFLAGS='macros.exp')