Re: [PATCH v3 00/13] RISC-V: assorted fixes and (hopefully) improvements

On 2026/6/19 19:45, Jan Beulich wrote:
> [1]https://sourceware.org/pipermail/binutils/2023-March/126601.html

Hi Jan,

I did a more detailed local investigation of this old RFC.

I tested this on current trunk and also on a checkout close to the original
RFC date, around 2023-03-10. The exact numeric altmacro testcase from the
old RFC, using

m1 %v2-v1 17
m2b 81 243

already emits the expected bytes

a2 11

both on current trunk and on the 2023-03-10 checkout I tested. So that
testcase by itself does not demonstrate the issue for me.

However, the underlying problem still seems to exist. A symbolic variant:

.equ s1, 81
.equ s2, 243
m1 %s2-s1 17

fails without the patch with:

% operator needs absolute expression

GDB shows temp_ilp() parsing the string

s2-s1 17

and get_symbol_name() later seeing

s1 17

while input_from_string is true. Since RISC-V currently uses space as
FAKE_LABEL_CHAR, the generic expression parser can treat that space as part
of the symbol-name scan in this mode. That looks like the concrete root
cause.

Rebasing the RFC idea to use ".L0?" / '?' fixes this symbolic testcase. I
also kept the gas/app.c lex[] change from the RFC, so that 
FAKE_LABEL_CHAR is
accepted by the scrubber as part of generated/internal symbol names when
needed. In my tests this did not make ordinary unquoted '?' symbols 
accepted:
"user?symbol:" was still rejected, while quoted user symbols such as
"user?symbol" remained visible. So '?' looks like a reasonable replacement:
it is not a whitespace separator like the current space character, and I did
not see it introduce broad user-symbol parsing or hiding regressions in the
cases I tested.

I also think the gas/write.h comment update is still useful. The old RISC-V
choice of a space character shows that FAKE_LABEL_CHAR should not merely be
distinct from normal symbol characters; it also should not be a separator or
an operator-start character.

I rebuilt all-gas/all-binutils and ran:

make check-gas RUNTEST=... RUNTESTFLAGS="gas/all/gas.exp=altmacro"
# of expected passes            112
# of expected failures          8
# of unsupported tests          2

make check-gas RUNTEST=... RUNTESTFLAGS="riscv.exp"
# of expected passes            350

For the RISC-V tests I had to update the expected fake label spelling in
la-variants.d from ".L0 " to".L0?".

I would not revive the old patch exactly as-is, though. New objdump should
also hide old-style ".L0 " fake labels for old gas / new objdump
compatibility; otherwise objdump -dr can show labels such as:

0000000000000004 <.L0 >:

The reverse direction, new gas with old objdump, still exposes ".L0?", but I
do not think that can be fixed from the new sources.

One more thing I noticed is that even with patched gas and patched objdump,
objdump -dr can still print fake labels in relocation annotations, for 
example:

R_RISCV_PCREL_LO12_I .L0?

This path seems to bypass riscv_symbol_is_valid(). I think this should be
discussed as a separate objdump issue rather than hidden inside the
FAKE_LABEL_CHAR change, especially since these fake labels also help 
show the
PCREL HI/LO pairing.

Regarding the other points you raised, I agree that 
make_internal_label() using
the same FAKE_LABEL_NAME for all instances is not ideal. It makes relocation
output harder to associate with the specific generated label. But I think
renaming those internal labels to follow the fb_label_name() / 
dollar_label_name()
style is a separate cleanup from changing the fake-label character.

For LOCAL_LABEL_CHAR / DOLLAR_LABEL_CHAR handling in read_symbol_name() /
get_symbol_name(), I do not have a concrete failing case yet, so I would 
prefer
not to mix that into this change.

I also tested quoted symbols containing '?'. Quoted user symbols such as
"user?symbol" remained visible, and unquoted "user?symbol:" was still
rejected. Symbols like ".Luser?" are still subject to the existing generic
.L local-symbol filtering, but I did not see a new broad false-hiding or
symbol-lexing regression caused by using '?' as FAKE_LABEL_CHAR.

So my recommendation is:

  *

    use '?' rather than space for the RISC-V fake label character;

  *

    keep the gas/app.c lex[] handling from the RFC;

  *

    keep the gas/write.h constraint clarification;

  *

    add a stronger symbolic altmacro testcase, such as "%s2-s1 17";

  *

    add compatibility in the RISC-V objdump symbol-valid hook so new objdump
    hides both ".L0?" and old ".L0 ";

  *

    leave relocation annotation cleanup, make_internal_label() naming, and
    broader LOCAL_LABEL_CHAR / DOLLAR_LABEL_CHAR handling as separate
    follow-ups.

Please let me know if I misunderstood any part of the original issue or the
intended fake-label handling.

Best regards,
Jiawei