This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] opcodes/riscv: Hide '.L0 ' fake symbols
- From: Jim Wilson <jimw at sifive dot com>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>
- Cc: Binutils <binutils at sourceware dot org>, Palmer Dabbelt <palmer at sifive dot com>
- Date: Mon, 3 Dec 2018 12:24:03 -0800
- Subject: Re: [PATCH] opcodes/riscv: Hide '.L0 ' fake symbols
- References: <20181203152300.22152-1-andrew.burgess@embecosm.com>
On Mon, Dec 3, 2018 at 7:23 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> The RISC-V assembler generates fake labels with the name '.L0 ' as
> part of the debug information (see
> gas/config/tc-riscv.h:FAKE_LABEL_NAME).
They are also used for relaxable relocations, such as a medany auipc
instruction.
> include/ChangeLog:
> * dis-asm.h (riscv_symbol_is_valid): Declare.
>
> opcodes/ChangeLog:
> * disassembler.c (disassemble_init_for_target): Add RISC-V
> initialisation.
> * riscv-dis.c (riscv_symbol_is_valid): New function.
This looks mostly OK to me, with the fix for the problem that Andreas
pointed out except that ...
> +/* Prevent use of the fake labels that are generated as part of the DWARF
> + debug in the assembler. The fake labels are edefined in
> + gas/config/tc-riscv.h as FAKE_LABEL_NAME. */
> ...
> + return (strcmp (name, ".L0 ") != 0);
You have a comment here pointing at GAS, but there is no comment in
GAS pointing here. The value for FAKE_LABEL_NAME has already changed
at least once in gas, so we should have a better connection here to
make sure that a future gas change won't accidentally break this
disassembler support. Putting a comment in gas pointing here would be
OK. Another possible solution is to move the definition of
FAKE_LABEL_NAME into include/opcodes/riscv.h, and then you can use it
directly here. gas/config/tc-riscv.h already includes this header
file so I think this should work OK.
You also have a typo in the comment, edefined -> defined.
Jim