From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id 793813858C83 for ; Mon, 7 Feb 2022 09:58:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 793813858C83 Message-ID: Date: Mon, 7 Feb 2022 18:58:48 +0900 Mime-Version: 1.0 Subject: Re: [RFC PATCH v2 0/2] gdb, opcodes: Add isa disassembler option to RISC-V Content-Language: en-US To: Nelson Chu Cc: Binutils References: From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.2 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Mon, 07 Feb 2022 09:58:57 -0000 On 2022/02/07 15:47, Nelson Chu wrote: > On Sun, Feb 6, 2022 at 10:11 AM Tsukasa OI via Binutils > wrote: >> >> *** NOTE *** >> PATCH 1 contains non-RISC-V (GDB-related) changes. >> >> >> This patchset adds support for isa=ISA diassembler option to RISC-V. >> >> - objdump: -M isa=rv[32|64]... > > Not sure why we need it, but it would be better to discuss with the > community (gcc and llvm) for something new like this first. You could > try to create new issues on riscv psabi > (https://github.com/riscv-non-isa/riscv-elf-psabi-doc) or riscv asm > manual (https://github.com/riscv-non-isa/riscv-asm-manual). If the > issues should be discussed elsewhere, I believe someone will tell you > there. Nelson, I apologize for proposing something new without proper discussion. This is neither an assembler nor ABI issue (because it's about new diassembler option), so it would be a standalone binutils/GDB issue. So despite that this mailing list would not be the best place, proper place to discuss should be still binutils/GDB-related, I guess. Assuming you read cover letter of my RFC PATCH v1, my intent is: 1. Enable analysis/debugging of RISC-V binary (without ELF header) By default, only I, M, A, F, D, C, Zicsr and Zifencei extensions are correctly disassembled when we pass a binary file to objdump. With ISA option, it would enable disassembling instructions in other extensions (such as Zb* and Q). My original intent was for reverse engineering of binary files but I found this is particularly useful for JTAG-based debugging (where passing an ELF executable to gdb command is not always applicable). Because Zb* extensions are already available in some commercial RISC-V CPUs, the scenario above is not that unrealistic. 2. Consistency with -M priv-spec Existing -M priv-spec option enables to choose appropriate privileged specification version, enabling proper disassembler results. With -M isa, we can use not only arbitrary privileged specification but arbitrary ISA extensions. > >> - gdb: set disassembler-options isa=rv[32|64]... > > You should also send the gdb patch to gdb-patches@sourceware.org. > There will be more gdb experts over there. I get it. After we discuss whether adding ISA option is okay, I could submit the patchset to that mailing list for discussion. Thanks, Tsukasa > >> This patchset is version 2. Version 1 is here: >> >> >> >> My development branch on GitHub: >> >> >> >> [Changes between RFC PATCH v1 and RFC PATCH v2] >> >> The only functional change in the RFC PATCH v2 is that we reset `xlen' >> variable to `0' before parsing ISA string. It has no effect on objdump >> but on GDB, it stops preserving last XLEN value before setting invalid >> ISA string (not starting with "rv32" or "rv64"). >> >> Rest of the changes are editorial. My language got broken while writing >> v1 but I think most of them are fixed in v2. Also, most "-M isa" >> (objdump-only) references are replaced with just "isa" to indicate >> both objdump and GDB options. >> >> Renamed `xlen_set_by_option' to `xlen_by_isa' for clarity (meaning XLEN >> set by "ISA string" option). >> >> >> >> >> Tsukasa OI (2): >> gdb, opcodes: Add non-enum disassembler options >> RISC-V: Add isa disassembler option >> >> gdb/disasm.c | 4 ++++ >> include/dis-asm.h | 3 ++- >> opcodes/arc-dis.c | 2 ++ >> opcodes/mips-dis.c | 2 ++ >> opcodes/riscv-dis.c | 35 ++++++++++++++++++++++++++++------- >> 5 files changed, 38 insertions(+), 8 deletions(-) >> >> >> base-commit: 94e57f287f96af6af97dd086e4a04ddf55b7cf60 >> -- >> 2.32.0 >> >