Re: [RFC PATCH v2 0/2] gdb, opcodes: Add isa disassembler option to RISC-V

[Tsukasa OI <research_trasio@irq.a4lg.com>]

On 2022/02/07 15:47, Nelson Chu wrote:
> On Sun, Feb 6, 2022 at 10:11 AM Tsukasa OI via Binutils
> <binutils@sourceware.org> 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,
<https://sourceware.org/pipermail/binutils/2022-February/119639.html>

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:
>> <https://sourceware.org/pipermail/binutils/2022-February/119639.html>
>> <https://github.com/a4lg/binutils-gdb/tree/a4lg/snapshots/2022-02-05/riscv-dis-isa>
>>
>> My development branch on GitHub:
>> <https://github.com/a4lg/binutils-gdb/tree/riscv-dis-isa>
>>
>>
>> [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
>>
>