public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler
@ 2022-08-27  0:10 Tsukasa OI
  2022-08-27  0:10 ` [PATCH v8 1/7] RISC-V: Add address printer tests with ADDIW Tsukasa OI
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Tsukasa OI @ 2022-08-27  0:10 UTC (permalink / raw)
  To: Tsukasa OI, H . Peter Anvin, Palmer Dabbelt, Andrew Waterman,
	Jim Wilson, Nelson Chu
  Cc: binutils

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.



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


^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-09-02  6:18 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-27  0:10 [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler Tsukasa OI
2022-08-27  0:10 ` [PATCH v8 1/7] RISC-V: Add address printer tests with ADDIW Tsukasa OI
2022-08-27  0:10 ` [PATCH v8 2/7] RISC-V: Fix JALR target address computation Tsukasa OI
2022-08-27  0:11 ` [PATCH v8 3/7] RISC-V: Fix RV32 disassembler " Tsukasa OI
2022-08-27  0:11 ` [PATCH v8 4/7] RISC-V: Print highest address on the disassembler Tsukasa OI
2022-08-27  0:11 ` [PATCH v8 5/7] RISC-V: Print top GP-relative addresses " Tsukasa OI
2022-08-27  0:11 ` [PATCH v8 6/7] RISC-V: Clarify that `wide' is only used for ADDIW Tsukasa OI
2022-08-27  0:11 ` [PATCH v8 7/7] RISC-V: Make `is_addiw' parameter bool Tsukasa OI
2022-08-27  0:22 ` [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 1/7] RISC-V: Add address printer tests with ADDIW Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 2/7] RISC-V: Fix JALR target address computation Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 3/7] RISC-V: Fix RV32 disassembler " Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 4/7] RISC-V: Print highest address on the disassembler Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 5/7] RISC-V: Print top GP-relative addresses " Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 6/7] RISC-V: Clarify that `wide' is only used for ADDIW Tsukasa OI
2022-08-27  0:22   ` [PATCH v8 7/7] RISC-V: Make `is_addiw' parameter bool Tsukasa OI
2022-08-27  0:28   ` [PATCH v8 0/7] RISC-V: Fix address printer on the disassembler Tsukasa OI
2022-09-02  6:18 ` Nelson Chu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).