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

[Kito Cheng <kito.cheng@gmail.com>]

Hi Tsukasa, Nelson:

I guess riscv-toolchain-convention[1] is the right place to document
and discuss that,
-M no-aliases and -M numeric are also implemented on llvm-objdump[2],
so I think that would be great to add document for new -M option.

[1] https://github.com/riscv-non-isa/riscv-toolchain-conventions#disassembler-objdump-behaviour
[2] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp#L48

On Mon, Feb 7, 2022 at 5:59 PM Tsukasa OI via Binutils
<binutils@sourceware.org> wrote:
>
> 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
> >>
> >