[PATCH v3 0/3] RISC-V: Fix address printer on the disassembler

[Tsukasa OI <research_trasio@irq.a4lg.com>]

Hello,

This patchset contains a fix to PR29342.

Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_fix_addr>

Original Issue as reported by H. Peter Anvin:
<https://sourceware.org/bugzilla/show_bug.cgi?id=29342>



[Changes: v2 -> v3]

-   Remove minor optimization (v2: PATCH 3/4)
    Further optimization (my disassembler optimization batch 2) revealed that
    scanning through the symbol table is necessary.  It means that breaking
    early doesn't help to improve the performance, even if the ELF file has
    the __global_pointer$ symbol.
    Because this "minor optimization" changed the behavior slightly, it's better
    to drop this one.
-   Rename testcase auipc-x0 to dis-addr-1
    I decided to group disassembler-printed address tests.
-   Rebase



[Performance Analysis (Summary): updated per v3]

This patch alone degrades the disassembler performance a bit (usually about 1%).
But I found my next batch of the disassembler optimization patchset somehow
shadows this.  In fact, Binutils with my optimization patchset 1 and Binutils
with my optimization patchset 1 plus this patchset has pretty much
the same performance.



[Changes: v1 -> v2]

-   Also allow `gp'-base symbol (__global_pointer$) to be the highest address
-   Add minor optimization (break early if multiple __global_pointer$ is found)
-   Rebase and minor copyediting



[Cover Letter from v1]

First of all, this bug is caused by address computation of maybe_print_address
on disassembling RV32 programs.

Let me show the pseudocode of `maybe_print_address':

    if (high part of the register [for address sequence] is set)
    {
        pd->print_addr = (high part) + offset;        // (1)
        unset high part (as used);
    }
    else if (base register == `gp' && __global_pointer$ is available)
        pd->print_addr = __global_pointer$ + offset;  // (2)
    else if (base register == `tp' || base register == `zero')
        pd->print_addr = offset;                      // (3)
    // Sign-extend a 32-bit value to 64-bit
    if (instruction is ADDIW or C.ADDIW)
        pd->print_addr = (bfd_vma)(int32_t)pd->print_addr;

In here, it implicitly sign-extends an `int'-typed variable `offset' to generate
an address.  (3) is the direct cause of PR29342 but other locations have
similar issue.

On an example provided by Peter, IOREG_FOO has an address value of 0xffffff00.
However, due to incorrect sign-extension, the disassembler considers that the
`sw' instruction operates on an incorrect address 0xffffffff_ffffff00 EVEN ON
RV32.  This affects symbol lookup.  So, we have to mask (and zero-extend) the
address on RV32 to get correct address 0xffffff00.

Also, the background provided by Peter gives us a context: highest address space
may be important for some systems/programs.  So, not only fixing PR29342, I
decided to make the address -1 (originally reserved as a non-printing address)
printable by separating (a) the address to print and (b) whether the address
should be printed.  This isn't zero-overhead but close to.

This patchset:

1.  fits an address into a 32-bit value on RV32 (resolves PR29342)
2.  makes the highest address printable
    (0xffffffff can be printed as a real symbol)
3.  clarifies the meaning of the `wide' argument
    (`is_addiw' fits the context).

It also has new testcases and a testcase modification (it seems lla32.d is
affected by this patchset but not harmful so that modifying the testcase
lla32.d seemed better).

Thanks,
Tsukasa




Tsukasa OI (3):
  RISC-V: Print highest address on disassembler
  RISC-V: Fix RV32 disassembler address computation
  RISC-V: Add address printer tests on disassembler

 .../gas/riscv/{auipc-x0.d => dis-addr-1.d}    |  0
 .../gas/riscv/{auipc-x0.s => dis-addr-1.s}    |  0
 gas/testsuite/gas/riscv/dis-addr-2-32.d       | 29 ++++++++
 gas/testsuite/gas/riscv/dis-addr-2-64.d       | 33 +++++++++
 gas/testsuite/gas/riscv/dis-addr-2.s          | 70 +++++++++++++++++++
 gas/testsuite/gas/riscv/dis-addr-3-32.d       | 12 ++++
 gas/testsuite/gas/riscv/dis-addr-3-64.d       | 12 ++++
 gas/testsuite/gas/riscv/dis-addr-3.s          | 15 ++++
 gas/testsuite/gas/riscv/lla32.d               |  2 +-
 opcodes/riscv-dis.c                           | 46 +++++++-----
 10 files changed, 202 insertions(+), 17 deletions(-)
 rename gas/testsuite/gas/riscv/{auipc-x0.d => dis-addr-1.d} (100%)
 rename gas/testsuite/gas/riscv/{auipc-x0.s => dis-addr-1.s} (100%)
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-2-32.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-2-64.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-2.s
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-3-32.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-3-64.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-3.s


base-commit: 94e27e8e69a07d69554ee7598419646d3ee99907
-- 
2.34.1