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

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

[Tsukasa OI]

*** NOTE ***
PATCH 1 contains non-RISC-V (GDB-related) changes.


This patchset adds support for -M isa=ISA diassembler option to RISC-V.

The background is simple.  This patchset enables disassembling non-GC
instructions on binary files (without .riscv.attributes section).

However, this isn't that simple as it requires non-arch changes.



[PATCH 1: gdb, opcodes: Add non-enum disassembler options]

There is a portable mechanism for disassembler options and used on some
architectures:

-   ARC
-   Arm
-   MIPS
-   PowerPC
-   RISC-V
-   S/390

However, it only supports following forms:

-   [NAME]
-   [NAME]=[ENUM_VALUE]

Valid values for [ENUM_VALUE] must be predefined in
`disasm_option_arg_t.values'.  For instance, for -M cpu=[CPU] in ARC
architecture, opcodes/arc-dis.c builds valid CPU model list from
include/elf/arc-cpu.def.

In PATCH 1 of this patchset, it adds following format:

-   [NAME]=[ARBITRARY_VALUE] (however cannot contain ",")

This is identified by `NULL' value of `disasm_option_arg_t.values'
(normally, this is a non-NULL pointer to a NULL-terminated list).

Note that this patch modifies following architectures (that use
similar code to print disassembler help message) for consistency:

-   ARC
-   MIPS
-   RISC-V

In the future, adding "verify" function to disasm_option_arg_t (or some)
might be an option as it may provide flexible argument validation.



[PATCH 2: RISC-V: Add -M isa disassembler option]

-M isa=ISA is very simple.  But we have multiple ways to set proper
XLEN for given situation.

In this patch, I propose following precedence rules (objdump/gdb):

1.  XLEN-specified arch (-m riscv:rv[32|64] / set arch riscv:rv[32|64])
2.  ISA option (-M isa=rv[32|64]... / set disassembler-options isa=...)
3.  ELF class in the "ELF header" (note that this must be the last
    because dummy "ELF header" exists even in binary files)

This enables XLEN switching by ISA option on architecture riscv but not
on riscv:rv32 or riscv:rv64 (architecture with fixed XLEN is preferred).

I preferred not to warn if XLEN in above three conflict because of
"dummy ELF header" described above and possible dynamic capability when
used together with GDB.  Still, adding it might be an option.




Tsukasa OI (2):
  gdb, opcodes: Add non-enum disassembler options
  RISC-V: Add -M 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 | 34 +++++++++++++++++++++++++++-------
 5 files changed, 37 insertions(+), 8 deletions(-)


base-commit: eb06e60a982e3903161252edf8fb8ae0c018c467
-- 
2.32.0

[RFC PATCH 1/2] gdb, opcodes: Add non-enum disassembler options

[Tsukasa OI]

This commit makes `NULL' value of disasm_option_arg_t.values which
allows arbitrary (non-enum) disassembler options.

gdb/ChangeLog:

	* gdb/disasm.c (set_disassembler_options): Add arbitrary non-
	enum disassembler options.
	(show_disassembler_options_sfunc): Add support for arbitrary
	non-enum disassembler options.

include/ChangeLog:

	* dis-asm.h (disasm_option_arg_t): Update comment of `values'
	to allow arbitrary non-enum disassembler options.

opcodes/ChangeLog:

	* riscv-dis.c (print_riscv_disassembler_options): Support
	arbitrary non-enum disassembler options on printing help string.
	* arc-dis.c (print_arc_disassembler_options): Likewise.
	* mips-dis.c (print_mips_disassembler_options): Likewise.
---
 gdb/disasm.c        | 4 ++++
 include/dis-asm.h   | 3 ++-
 opcodes/arc-dis.c   | 2 ++
 opcodes/mips-dis.c  | 2 ++
 opcodes/riscv-dis.c | 2 ++
 5 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index 5cd1f5adbd2..eb7e2fec9fe 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -1009,6 +1009,8 @@ set_disassembler_options (const char *prospective_options)
 	    if (memcmp (opt, valid_options->name[i], len) != 0)
 	      continue;
 	    arg = opt + len;
+	    if (valid_options->arg[i]->values == NULL)
+	      break;
 	    for (j = 0; valid_options->arg[i]->values[j] != NULL; j++)
 	      if (disassembler_options_cmp
 		    (arg, valid_options->arg[i]->values[j]) == 0)
@@ -1130,6 +1132,8 @@ The following disassembler options are supported for use with the\n\
 
       for (i = 0; valid_args[i].name != NULL; i++)
 	{
+	  if (valid_args[i].values == NULL)
+	    continue;
 	  fprintf_filtered (file, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 			    valid_args[i].name);
diff --git a/include/dis-asm.h b/include/dis-asm.h
index 317592448a2..1826fe97ba7 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -239,7 +239,8 @@ typedef struct
   /* Option argument name to use in descriptions.  */
   const char *name;
 
-  /* Vector of acceptable option argument values, NULL-terminated.  */
+  /* Vector of acceptable option argument values, NULL-terminated.
+     NULL if any values are accepted.  */
   const char **values;
 } disasm_option_arg_t;
 
diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index 771f786d18e..41371967c3b 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -1555,6 +1555,8 @@ print_arc_disassembler_options (FILE *stream)
   for (i = 0; args[i].name != NULL; ++i)
     {
       size_t len = 3;
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
index 9db604ffb39..faeebccfc3b 100644
--- a/opcodes/mips-dis.c
+++ b/opcodes/mips-dis.c
@@ -2809,6 +2809,8 @@ with the -M switch (multiple options should be separated by commas):\n\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 34724d4aec5..13ddff01c52 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1132,6 +1132,8 @@ with the -M switch (multiple options should be separated by commas):\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
-- 
2.32.0

[RFC PATCH 2/2] RISC-V: Add -M isa disassembler option

[Tsukasa OI]

This commit adds -M isa=ISA disassembler option to objdump and gdb.
ISA string given has a higher precedence than ELF header but lower than
BFD architecture with specific XLEN (riscv:rv32 and riscv:rv64).

opcodes/ChangeLog:

	* riscv-dis.c (default_isa): Rename from local `default_arch'.
	(xlen_set_by_option) New.
	(set_default_riscv_dis_options): Initialize default ISA and
	RISC-V subset.
	(parse_riscv_dis_option): Parse -M isa=ISA option.
	(riscv_disassemble_insn): Update xlen setting rules.
	(riscv_get_disassembler): Stop setting default ISA here.
	Instead, pass default ISA for later initialization.
	(riscv_option_arg_t): Add arguments for -M isa=ISA option.
	(riscv_options): Add -M isa=ISA option.
	(disassembler_options_riscv): Add handling for -M isa=ISA.
---
 opcodes/riscv-dis.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 13ddff01c52..b67597542b5 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -34,8 +34,10 @@
 
 static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
 static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
+static const char *default_isa = "rv64gc";
 
 unsigned xlen = 0;
+static unsigned xlen_set_by_option = 0;
 
 static riscv_subset_list_t riscv_subsets;
 static riscv_parse_subset_t riscv_rps_dis =
@@ -71,6 +73,9 @@ set_default_riscv_dis_options (void)
   riscv_gpr_names = riscv_gpr_names_abi;
   riscv_fpr_names = riscv_fpr_names_abi;
   no_aliases = 0;
+  riscv_release_subset_list (&riscv_subsets);
+  riscv_parse_subset (&riscv_rps_dis, default_isa);
+  xlen_set_by_option = 0;
 }
 
 static bool
@@ -134,6 +139,12 @@ parse_riscv_dis_option (const char *option)
 				 option, value, name);
 	}
     }
+  else if (strcmp (option, "isa") == 0)
+    {
+      riscv_release_subset_list (&riscv_subsets);
+      riscv_parse_subset (&riscv_rps_dis, value);
+      xlen_set_by_option = xlen;
+    }
   else
     {
       /* xgettext:c-format */
@@ -592,11 +603,16 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
   op = riscv_hash[OP_HASH_IDX (word)];
   if (op != NULL)
     {
-      /* If XLEN is not known, get its value from the ELF class.  */
+      /* Set XLEN with following precedence rules:
+	 1. -m riscv:rv[32|64] option (gdb: set arch riscv:rv[32|64])
+	 2. -M isa option (gdb: set disassembler-options isa=rv[32|64])
+	 3. ELF class in the ELF header.  */
       if (info->mach == bfd_mach_riscv64)
 	xlen = 64;
       else if (info->mach == bfd_mach_riscv32)
 	xlen = 32;
+      else if (xlen_set_by_option)
+        xlen = xlen_set_by_option;
       else if (info->section != NULL)
 	{
 	  Elf_Internal_Ehdr *ehdr = elf_elfheader (info->section->owner);
@@ -947,8 +963,6 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
 disassembler_ftype
 riscv_get_disassembler (bfd *abfd)
 {
-  const char *default_arch = "rv64gc";
-
   if (abfd)
     {
       const struct elf_backend_data *ebd = get_elf_backend_data (abfd);
@@ -965,13 +979,10 @@ riscv_get_disassembler (bfd *abfd)
 						      attr[Tag_b].i,
 						      attr[Tag_c].i,
 						      &default_priv_spec);
-	      default_arch = attr[Tag_RISCV_arch].s;
+	      default_isa = attr[Tag_RISCV_arch].s;
 	    }
 	}
     }
-
-  riscv_release_subset_list (&riscv_subsets);
-  riscv_parse_subset (&riscv_rps_dis, default_arch);
   return print_insn_riscv;
 }
 
@@ -1000,6 +1011,7 @@ riscv_symbol_is_valid (asymbol * sym,
 typedef enum
 {
   RISCV_OPTION_ARG_NONE = -1,
+  RISCV_OPTION_ARG_ISA,
   RISCV_OPTION_ARG_PRIV_SPEC,
 
   RISCV_OPTION_ARG_COUNT
@@ -1020,6 +1032,9 @@ static struct
   { "no-aliases",
     N_("Disassemble only into canonical instructions."),
     RISCV_OPTION_ARG_NONE },
+  { "isa=",
+    N_("Disassemble using chosen ISA and extensions."),
+    RISCV_OPTION_ARG_ISA },
   { "priv-spec=",
     N_("Print the CSR according to the chosen privilege spec."),
     RISCV_OPTION_ARG_PRIV_SPEC }
@@ -1044,6 +1059,9 @@ disassembler_options_riscv (void)
 
       args = XNEWVEC (disasm_option_arg_t, num_args + 1);
 
+      args[RISCV_OPTION_ARG_ISA].name = "ISA";
+      args[RISCV_OPTION_ARG_ISA].values = NULL;
+
       args[RISCV_OPTION_ARG_PRIV_SPEC].name = "SPEC";
       priv_spec_count = PRIV_SPEC_CLASS_DRAFT - PRIV_SPEC_CLASS_NONE - 1;
       args[RISCV_OPTION_ARG_PRIV_SPEC].values
-- 
2.32.0

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

[Tsukasa OI]

*** 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]...
-   gdb: set disassembler-options isa=rv[32|64]...

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

[RFC PATCH v2 1/2] gdb, opcodes: Add non-enum disassembler options

[Tsukasa OI]

This commit adds support for non-enum disassembler options that allow
arbitrary argument (not including ",").  This is identified by `NULL'
value of disasm_option_arg_t.values.

gdb/ChangeLog:

	* gdb/disasm.c (set_disassembler_options): Add support for
	non-enum disassembler options.
	(show_disassembler_options_sfunc): Likewise.

include/ChangeLog:

	* dis-asm.h (disasm_option_arg_t): Update comment of `values'
	to allow non-enum disassembler options.

opcodes/ChangeLog:

	* riscv-dis.c (print_riscv_disassembler_options): Support
	non-enum disassembler options on printing disassembler help.
	* arc-dis.c (print_arc_disassembler_options): Likewise.
	* mips-dis.c (print_mips_disassembler_options): Likewise.
---
 gdb/disasm.c        | 4 ++++
 include/dis-asm.h   | 3 ++-
 opcodes/arc-dis.c   | 2 ++
 opcodes/mips-dis.c  | 2 ++
 opcodes/riscv-dis.c | 2 ++
 5 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index 5cd1f5adbd2..eb7e2fec9fe 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -1009,6 +1009,8 @@ set_disassembler_options (const char *prospective_options)
 	    if (memcmp (opt, valid_options->name[i], len) != 0)
 	      continue;
 	    arg = opt + len;
+	    if (valid_options->arg[i]->values == NULL)
+	      break;
 	    for (j = 0; valid_options->arg[i]->values[j] != NULL; j++)
 	      if (disassembler_options_cmp
 		    (arg, valid_options->arg[i]->values[j]) == 0)
@@ -1130,6 +1132,8 @@ The following disassembler options are supported for use with the\n\
 
       for (i = 0; valid_args[i].name != NULL; i++)
 	{
+	  if (valid_args[i].values == NULL)
+	    continue;
 	  fprintf_filtered (file, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 			    valid_args[i].name);
diff --git a/include/dis-asm.h b/include/dis-asm.h
index 317592448a2..1826fe97ba7 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -239,7 +239,8 @@ typedef struct
   /* Option argument name to use in descriptions.  */
   const char *name;
 
-  /* Vector of acceptable option argument values, NULL-terminated.  */
+  /* Vector of acceptable option argument values, NULL-terminated.
+     NULL if any values are accepted.  */
   const char **values;
 } disasm_option_arg_t;
 
diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index 771f786d18e..41371967c3b 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -1555,6 +1555,8 @@ print_arc_disassembler_options (FILE *stream)
   for (i = 0; args[i].name != NULL; ++i)
     {
       size_t len = 3;
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
index 9db604ffb39..faeebccfc3b 100644
--- a/opcodes/mips-dis.c
+++ b/opcodes/mips-dis.c
@@ -2809,6 +2809,8 @@ with the -M switch (multiple options should be separated by commas):\n\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 34724d4aec5..13ddff01c52 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1132,6 +1132,8 @@ with the -M switch (multiple options should be separated by commas):\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
-- 
2.32.0

[RFC PATCH v2 2/2] RISC-V: Add isa disassembler option

[Tsukasa OI]

This commit adds isa=ISA disassembler option to objdump and gdb.
ISA string given has a higher precedence than ELF header but lower than
BFD architecture with specific XLEN (riscv:rv32 and riscv:rv64).

opcodes/ChangeLog:

	* riscv-dis.c (default_isa): Rename from local `default_arch'.
	(xlen_by_isa) New.  (set_default_riscv_dis_options): Initialize
	default ISA and RISC-V subset.
	(parse_riscv_dis_option): Parse isa=ISA option with resetting
	previous XLEN.
	(riscv_disassemble_insn): Update XLEN precedence rules.
	(riscv_get_disassembler): Stop setting default ISA here.
	Instead, pass default ISA for later initialization.
	(riscv_option_arg_t): Add arguments for isa=ISA option.
	(riscv_options): Add isa=ISA option.
	(disassembler_options_riscv): Add handling for isa=ISA.
---
 opcodes/riscv-dis.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 13ddff01c52..ae4c5893063 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -34,8 +34,10 @@
 
 static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
 static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
+static const char *default_isa = "rv64gc";
 
 unsigned xlen = 0;
+static unsigned xlen_by_isa = 0;
 
 static riscv_subset_list_t riscv_subsets;
 static riscv_parse_subset_t riscv_rps_dis =
@@ -71,6 +73,9 @@ set_default_riscv_dis_options (void)
   riscv_gpr_names = riscv_gpr_names_abi;
   riscv_fpr_names = riscv_fpr_names_abi;
   no_aliases = 0;
+  riscv_release_subset_list (&riscv_subsets);
+  riscv_parse_subset (&riscv_rps_dis, default_isa);
+  xlen_by_isa = 0;
 }
 
 static bool
@@ -134,6 +139,13 @@ parse_riscv_dis_option (const char *option)
 				 option, value, name);
 	}
     }
+  else if (strcmp (option, "isa") == 0)
+    {
+      xlen = 0;
+      riscv_release_subset_list (&riscv_subsets);
+      riscv_parse_subset (&riscv_rps_dis, value);
+      xlen_by_isa = xlen;
+    }
   else
     {
       /* xgettext:c-format */
@@ -592,11 +604,16 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
   op = riscv_hash[OP_HASH_IDX (word)];
   if (op != NULL)
     {
-      /* If XLEN is not known, get its value from the ELF class.  */
+      /* Set XLEN with following precedence rules:
+	 1. -m riscv:rv[32|64] option (gdb: set arch riscv:rv[32|64])
+	 2. -M isa=rv[32|64]... option (gdb: set disassembler-options isa=...)
+	 3. ELF class in the ELF header.  */
       if (info->mach == bfd_mach_riscv64)
 	xlen = 64;
       else if (info->mach == bfd_mach_riscv32)
 	xlen = 32;
+      else if (xlen_by_isa)
+        xlen = xlen_by_isa;
       else if (info->section != NULL)
 	{
 	  Elf_Internal_Ehdr *ehdr = elf_elfheader (info->section->owner);
@@ -947,8 +964,6 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
 disassembler_ftype
 riscv_get_disassembler (bfd *abfd)
 {
-  const char *default_arch = "rv64gc";
-
   if (abfd)
     {
       const struct elf_backend_data *ebd = get_elf_backend_data (abfd);
@@ -965,13 +980,10 @@ riscv_get_disassembler (bfd *abfd)
 						      attr[Tag_b].i,
 						      attr[Tag_c].i,
 						      &default_priv_spec);
-	      default_arch = attr[Tag_RISCV_arch].s;
+	      default_isa = attr[Tag_RISCV_arch].s;
 	    }
 	}
     }
-
-  riscv_release_subset_list (&riscv_subsets);
-  riscv_parse_subset (&riscv_rps_dis, default_arch);
   return print_insn_riscv;
 }
 
@@ -1000,6 +1012,7 @@ riscv_symbol_is_valid (asymbol * sym,
 typedef enum
 {
   RISCV_OPTION_ARG_NONE = -1,
+  RISCV_OPTION_ARG_ISA,
   RISCV_OPTION_ARG_PRIV_SPEC,
 
   RISCV_OPTION_ARG_COUNT
@@ -1020,6 +1033,9 @@ static struct
   { "no-aliases",
     N_("Disassemble only into canonical instructions."),
     RISCV_OPTION_ARG_NONE },
+  { "isa=",
+    N_("Disassemble using chosen ISA and extensions."),
+    RISCV_OPTION_ARG_ISA },
   { "priv-spec=",
     N_("Print the CSR according to the chosen privilege spec."),
     RISCV_OPTION_ARG_PRIV_SPEC }
@@ -1044,6 +1060,9 @@ disassembler_options_riscv (void)
 
       args = XNEWVEC (disasm_option_arg_t, num_args + 1);
 
+      args[RISCV_OPTION_ARG_ISA].name = "ISA";
+      args[RISCV_OPTION_ARG_ISA].values = NULL;
+
       args[RISCV_OPTION_ARG_PRIV_SPEC].name = "SPEC";
       priv_spec_count = PRIV_SPEC_CLASS_DRAFT - PRIV_SPEC_CLASS_NONE - 1;
       args[RISCV_OPTION_ARG_PRIV_SPEC].values
-- 
2.32.0

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

[Nelson Chu]

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.

> -   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.

> 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
>

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

[Tsukasa OI]

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
>>
> 

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

[Kito Cheng]

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
> >>
> >

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

[Tsukasa OI]

Hi Kito,

That's the one I needed! Thanks for the pointer.

Tsukasa

On 2022/02/07 19:06, Kito Cheng wrote:
> 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
>>>>
>>>
> 

Re: [RFC PATCH 2/2] RISC-V: Add -M isa disassembler option

[Palmer Dabbelt]

On Sat, 05 Feb 2022 02:24:11 PST (-0800), binutils@sourceware.org wrote:
> This commit adds -M isa=ISA disassembler option to objdump and gdb.
> ISA string given has a higher precedence than ELF header but lower than
> BFD architecture with specific XLEN (riscv:rv32 and riscv:rv64).

IMO this should be called arch, not isa.  We use arch everywhere else, 
and while I know it's kind of odd (IIRC earlier version of the toolchain 
even called it "isa") it's better to be consistent.

> opcodes/ChangeLog:
>
> 	* riscv-dis.c (default_isa): Rename from local `default_arch'.
> 	(xlen_set_by_option) New.
> 	(set_default_riscv_dis_options): Initialize default ISA and
> 	RISC-V subset.
> 	(parse_riscv_dis_option): Parse -M isa=ISA option.
> 	(riscv_disassemble_insn): Update xlen setting rules.
> 	(riscv_get_disassembler): Stop setting default ISA here.
> 	Instead, pass default ISA for later initialization.
> 	(riscv_option_arg_t): Add arguments for -M isa=ISA option.
> 	(riscv_options): Add -M isa=ISA option.
> 	(disassembler_options_riscv): Add handling for -M isa=ISA.
> ---
>  opcodes/riscv-dis.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 13ddff01c52..b67597542b5 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -34,8 +34,10 @@
>
>  static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
>  static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
> +static const char *default_isa = "rv64gc";

That should probably be set by the target we're built for, but it looks 
like that's how this was before so it doesn't need to be fixed now.

>  unsigned xlen = 0;
> +static unsigned xlen_set_by_option = 0;
>
>  static riscv_subset_list_t riscv_subsets;
>  static riscv_parse_subset_t riscv_rps_dis =
> @@ -71,6 +73,9 @@ set_default_riscv_dis_options (void)
>    riscv_gpr_names = riscv_gpr_names_abi;
>    riscv_fpr_names = riscv_fpr_names_abi;
>    no_aliases = 0;
> +  riscv_release_subset_list (&riscv_subsets);
> +  riscv_parse_subset (&riscv_rps_dis, default_isa);
> +  xlen_set_by_option = 0;
>  }
>
>  static bool
> @@ -134,6 +139,12 @@ parse_riscv_dis_option (const char *option)
>  				 option, value, name);
>  	}
>      }
> +  else if (strcmp (option, "isa") == 0)
> +    {
> +      riscv_release_subset_list (&riscv_subsets);
> +      riscv_parse_subset (&riscv_rps_dis, value);
> +      xlen_set_by_option = xlen;
> +    }
>    else
>      {
>        /* xgettext:c-format */
> @@ -592,11 +603,16 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>    op = riscv_hash[OP_HASH_IDX (word)];
>    if (op != NULL)
>      {
> -      /* If XLEN is not known, get its value from the ELF class.  */
> +      /* Set XLEN with following precedence rules:
> +	 1. -m riscv:rv[32|64] option (gdb: set arch riscv:rv[32|64])
> +	 2. -M isa option (gdb: set disassembler-options isa=rv[32|64])
> +	 3. ELF class in the ELF header.  */
>        if (info->mach == bfd_mach_riscv64)
>  	xlen = 64;
>        else if (info->mach == bfd_mach_riscv32)
>  	xlen = 32;
> +      else if (xlen_set_by_option)
> +        xlen = xlen_set_by_option;
>        else if (info->section != NULL)
>  	{
>  	  Elf_Internal_Ehdr *ehdr = elf_elfheader (info->section->owner);
> @@ -947,8 +963,6 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>  disassembler_ftype
>  riscv_get_disassembler (bfd *abfd)
>  {
> -  const char *default_arch = "rv64gc";
> -
>    if (abfd)
>      {
>        const struct elf_backend_data *ebd = get_elf_backend_data (abfd);
> @@ -965,13 +979,10 @@ riscv_get_disassembler (bfd *abfd)
>  						      attr[Tag_b].i,
>  						      attr[Tag_c].i,
>  						      &default_priv_spec);
> -	      default_arch = attr[Tag_RISCV_arch].s;
> +	      default_isa = attr[Tag_RISCV_arch].s;
>  	    }
>  	}
>      }
> -
> -  riscv_release_subset_list (&riscv_subsets);
> -  riscv_parse_subset (&riscv_rps_dis, default_arch);
>    return print_insn_riscv;
>  }
>
> @@ -1000,6 +1011,7 @@ riscv_symbol_is_valid (asymbol * sym,
>  typedef enum
>  {
>    RISCV_OPTION_ARG_NONE = -1,
> +  RISCV_OPTION_ARG_ISA,
>    RISCV_OPTION_ARG_PRIV_SPEC,
>
>    RISCV_OPTION_ARG_COUNT
> @@ -1020,6 +1032,9 @@ static struct
>    { "no-aliases",
>      N_("Disassemble only into canonical instructions."),
>      RISCV_OPTION_ARG_NONE },
> +  { "isa=",
> +    N_("Disassemble using chosen ISA and extensions."),
> +    RISCV_OPTION_ARG_ISA },
>    { "priv-spec=",
>      N_("Print the CSR according to the chosen privilege spec."),
>      RISCV_OPTION_ARG_PRIV_SPEC }
> @@ -1044,6 +1059,9 @@ disassembler_options_riscv (void)
>
>        args = XNEWVEC (disasm_option_arg_t, num_args + 1);
>
> +      args[RISCV_OPTION_ARG_ISA].name = "ISA";
> +      args[RISCV_OPTION_ARG_ISA].values = NULL;
> +
>        args[RISCV_OPTION_ARG_PRIV_SPEC].name = "SPEC";
>        priv_spec_count = PRIV_SPEC_CLASS_DRAFT - PRIV_SPEC_CLASS_NONE - 1;
>        args[RISCV_OPTION_ARG_PRIV_SPEC].values

Otherwise this looks good.  Thanks!