From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id 48D2A3858293 for ; Tue, 9 Aug 2022 03:44:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 48D2A3858293 Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 8A16F300089; Tue, 9 Aug 2022 03:44:28 +0000 (UTC) From: Tsukasa OI To: Tsukasa OI , "H . Peter Anvin" , Nelson Chu , Kito Cheng , Palmer Dabbelt Cc: binutils@sourceware.org Subject: [PATCH v4 0/4] RISC-V: Fix address printer on the disassembler Date: Tue, 9 Aug 2022 12:44:23 +0900 Message-Id: In-Reply-To: References: Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Aug 2022 03:44:32 -0000 Hello, This patchset contains a fix to PR29342. Tracker on GitHub: Original Issue as reported by H. Peter Anvin: [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). Thanks, Tsukasa Tsukasa OI (4): 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 .../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/lla32.d | 2 +- opcodes/riscv-dis.c | 46 ++++++++---- 10 files changed, 210 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: 65c9841b6fee984714509acef6e52366363072b6 -- 2.34.1