[PATCH v8 0/7] RISC-V: Fix address printer on the disassembler

Tsukasa OI research_trasio@irq.a4lg.com
Sat Aug 27 00:22:52 GMT 2022


Hello,

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>

Previous Information on v1...v7:
<https://sourceware.org/pipermail/binutils/2022-August/122552.html>
See this e-mail for background.


[Changes: v8 -> v9]

Sorry! I forgot to remove redundant ChangeLog lines.



Based on Nelson's feedback, I functionally splitted the patchset to four
mostly independent parts.  Tests are self-contained to each part (unlike
dis-addr-2.s in PATCH v7) so that we can review each part independently.

(1) [PATCH 1-1] Additional tests for existing part: sequence with ADDIW
                (because we will test other cases in detail on other parts)
(2) [PATCH 2-3] Fix to PR29342 and a new JALR issue
                (both discovered by H. Peter Anvin)
(3) [PATCH 4-5] Make the disassembler able to print the highest address
                and GP-relative addresses even if the GP is the top addr
(4) [PATCH 6-7] Code Tidying: Rename and retype `wide' parameter of
                `maybe_print_address' for further clarification of the code


On 2022/08/24 20:22, Nelson Chu wrote:
> I think there are three dis-assembler issues here,
> 
> 1. PR29342
> 2. The target address of jump
> 3. Should we show the target address when it is -1

cf. <https://sourceware.org/pipermail/binutils/2022-August/122573.html>

Based on Nelson's classification, part (2) corresponds [1.] and [2.] and
(3) corresponds [3.].  About issues [1.] and [2.], PATCH 2 fixes [2.] and
PATCH 3 fixes [1.] but tests for [2.] is in the PATCH 3 (not PATCH 2).

This is because PATCH 3 tests address printing in various ways including
JALR instructions.  So splitting them to PATCH 2 seemed too redundant.


And, I couldn't come up with a good explanation to `binutils/NEWS'
describing the changes in the part (3).  Could someone help me?  It enables
printing following addresses on disassembling:

-   The highest address (-1)
-   GP-relative addresses if GP has the highest address (-1)



[Extraction Guide]

If we extract each part ((1) through (4)) using git rebase, we may
encounter some conflicts and test failures.  Even so, to extract (3) and
(4), I recommend to apply at least (2) through (4) first and then extract
using git rebase.

(1)
    This is completely independent from the rest.
(2)
    This is independent from (1).
(3)
    This is independent from (1).
    This is MOSTLY independent from (2) but extracting causes one conflict
    and two test errors (that depend on the bugfix on (2)):
        # Conflict: keep those lines
          else
            return;  /* Don't print the address.  */
          pd->to_print_addr = true;
        # Test Failures: two changes are required to pass check-gas
            # gas/testsuite/gas/riscv/dis-addr-topaddr-32.d
                old: <addr_top>
                new: <addr_top\+0x0>
            # gas/testsuite/gas/riscv/dis-addr-topaddr-gp-32.d
                old: <addr_rel_gp_pos>
                new: <__global_pointer\$\+0x5>
(4)
    This is independent from (1).
    This is independent from (3).
    This is MOSTLY independent from (2) but extracting causes one conflict.
        # Conflict: change like this and keep (to NOT keep the bugfix in (2))
        old: maybe_print_address (pd, rs1, 0, 0);
        new: maybe_print_address (pd, rs1, 0, false);




Tsukasa OI (7):
  RISC-V: Add address printer tests with ADDIW
  RISC-V: Fix JALR target address computation
  RISC-V: Fix RV32 disassembler address computation
  RISC-V: Print highest address on the disassembler
  RISC-V: Print top GP-relative addresses on the disassembler
  RISC-V: Clarify that `wide' is only used for ADDIW
  RISC-V: Make `is_addiw' parameter bool

 gas/testsuite/gas/riscv/dis-addr-addiw-a.d    | 18 +++++
 gas/testsuite/gas/riscv/dis-addr-addiw-b.d    | 18 +++++
 gas/testsuite/gas/riscv/dis-addr-addiw.s      | 28 ++++++++
 .../gas/riscv/dis-addr-overflow-32.d          | 30 ++++++++
 .../gas/riscv/dis-addr-overflow-64.d          | 34 +++++++++
 gas/testsuite/gas/riscv/dis-addr-overflow.s   | 70 +++++++++++++++++++
 gas/testsuite/gas/riscv/dis-addr-topaddr-32.d | 11 +++
 gas/testsuite/gas/riscv/dis-addr-topaddr-64.d | 11 +++
 .../gas/riscv/dis-addr-topaddr-gp-32.d        | 12 ++++
 .../gas/riscv/dis-addr-topaddr-gp-64.d        | 12 ++++
 gas/testsuite/gas/riscv/dis-addr-topaddr-gp.s | 15 ++++
 gas/testsuite/gas/riscv/dis-addr-topaddr.s    | 10 +++
 gas/testsuite/gas/riscv/lla32.d               |  2 +-
 opcodes/riscv-dis.c                           | 46 +++++++-----
 14 files changed, 300 insertions(+), 17 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-addiw-a.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-addiw-b.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-addiw.s
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-overflow-32.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-overflow-64.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-overflow.s
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-32.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-64.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-gp-32.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-gp-64.d
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr-gp.s
 create mode 100644 gas/testsuite/gas/riscv/dis-addr-topaddr.s


base-commit: 46e59b72f21029f2a863e3828cec76a03283b49d
-- 
2.34.1



More information about the Binutils mailing list