[PATCH v3 00/13] RISC-V: assorted fixes and (hopefully) improvements
Jiawei
jiawei@iscas.ac.cn
Mon Jun 22 13:35:51 GMT 2026
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20260622/d6499c77/attachment-0001.htm>
More information about the Binutils
mailing list