[PATCH v7 0/5] RISC-V: Fix address printer on the disassembler

Tsukasa OI research_trasio@irq.a4lg.com
Wed Aug 24 01:26:51 GMT 2022


This patchset contains a fix to PR29342.
Not only this patchset fixes a Bugzilla issue, it's also a requirement of
my future core disassembler changes.  So, it would be nice that this fix
is merged soon.

Tracker on GitHub:

Original Issue as reported by H. Peter Anvin:

[Changes: v6 -> v7]

-   Add tests involving ADDIW/C.ADDIW instructions
    I found that some MIPS tests used `--adjust-vma' and that can be used
    to adjust text segment to pretty much any location.  Using this, it
    enabled to test ADDIW/C.ADDIW corner cases.
    Note that regular section (with non-zero base address) and non-linked
    section (with zero base address) with --adjust-vma is indistinguishable
    from the view of libopcodes.
-   Rebase (every time)

[Changes: v5 -> v6]

-   Fix wrong file name in the ChangeLog
    Sorry for making small mistakes repeatedly...
-   Rebase (again)

[Changes: v4 -> v5]

-   Fix incorrect attribution in the commit message.

[Changes: v3 -> v4]

-   Added a bug fix discovered by Palmer Dabbelt.
    The original bug was in 2018.
-   Added testcases to make sure that the bug is fixed.
-   Rebase (again)

[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).


Tsukasa OI (5):
  RISC-V: Print highest address on disassembler
  RISC-V: Fix RV32 disassembler address computation
  RISC-V: Fix JALR target address computation
  RISC-V: Add address printer tests on disassembler
  RISC-V: Add address printer tests with ADDIW

 .../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       | 31 ++++++++
 gas/testsuite/gas/riscv/dis-addr-2-64.d       | 35 +++++++++
 gas/testsuite/gas/riscv/dis-addr-2.s          | 74 +++++++++++++++++++
 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/dis-addr-4.s          | 28 +++++++
 gas/testsuite/gas/riscv/dis-addr-4a.d         | 18 +++++
 gas/testsuite/gas/riscv/dis-addr-4b.d         | 18 +++++
 gas/testsuite/gas/riscv/lla32.d               |  2 +-
 opcodes/riscv-dis.c                           | 46 ++++++++----
 13 files changed, 274 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
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-4.s
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-4a.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-4b.d

base-commit: 044193ebf6a2197a36c75cbc1ec7e77440a37b26

More information about the Binutils mailing list