public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] RISC-V: Add overridable "priv-spec" and "arch" disassembler options
@ 2022-11-20  2:23 Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 1/3] RISC-V: Make "priv-spec" overridable Tsukasa OI
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Tsukasa OI @ 2022-11-20  2:23 UTC (permalink / raw)
  To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils, gdb-patches

*** IMPORTANT NOTES ***
-   PATCH 3/3 contains GDB changes.

This patchset:

-   Adds support for "arch=ARCH" diassembler option to RISC-V and
-   Makes existing "priv-spec=SPEC" disassembler option
    overridable on ELF files.
-   Adds GDB testcases

Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_arch_priv_spec>
Branch on GitHub:
<https://github.com/a4lg/binutils-gdb/tree/riscv-dis-arch-priv-spec>

(proposed "isa" option) PATCH v1:
<https://sourceware.org/pipermail/binutils/2022-February/119639.html>
(proposed "isa" option) PATCH v2:
<https://sourceware.org/pipermail/binutils/2022-February/119644.html>

The idea is reviewed by Palmer Dabbelt and issues pointed out were fixed
in this version:
<https://sourceware.org/pipermail/binutils/2022-February/119862.html>

Also, a prerequisite to this patchset is merged:
<https://sourceware.org/pipermail/binutils/2022-September/122719.html>
<https://sourceware.org/pipermail/gdb-patches/2022-September/191677.html>



** this patchset does not apply to master directly. **

This patchset requires following patchset to be applied first:
<https://sourceware.org/pipermail/binutils/2022-November/124378.html>
but not my optimization patchsets.

If my optimization patchsets:
<https://sourceware.org/pipermail/binutils/2022-November/124378.html>
<https://sourceware.org/pipermail/binutils/2022-November/124523.html>
are merged first, slight modification to this patchset
(to Binutils part) is required.  This is not affected on the GDB side.

That "slightly modified" branch (optimization first) is managed on GitHub:
<https://github.com/a4lg/binutils-gdb/tree/riscv-dis-arch-priv-spec.after-opt>





Let me recap the background of this patchset.

[Background]

Normally, libopcodes (used by objdump and GDB) on RISC-V sets:

1. Default ISA and extensions from either:
    -   ELF mapping symbols (with ISA string)
    -   ELF .riscv.attributes section
    -   "rv32gc"/"rv64gc" depending on ELF architecture or configuration
2. Default privileged specification version from:
    -   ELF .riscv.attributes section
    -   Latest privileged specification
        (if not ELF attributes are available)

But there are some cases that this automatic detection is not good.
This patchset is going to be particularly useful on:

-   gdb: Baremetal debugging on GDB (e.g. via OpenOCD)
-   objdump: Reverse engineering binary files (without ELF headers)

In both cases, self-describing architectural information is missing (unlike
regular ELF files we generate).  On such cases, that is very helpful to have
an option to specify ISA and specifications without help of ELF attributes.

Another situation where we need this option is that a program is compiled
for general/old specification but actually uses new/specialized ones (as
shown in an example below).

This patchset enables setting various ISA extensions / privileged
specifications for debugging / reverse engineering software
on various RISC-V processors:

-   objdump:
    -   -M arch=rv[32|64]...
    -   -M priv-spec=SPEC
-   GDB
    -   set disassembler-options arch=rv[32|64]...
    -   set disassembler-options priv-spec=SPEC
    Note that all disassembler options must be specified as
    a comma-separated list (specifying set disassembler-options twice makes
    only the last option applied).



[Example: OpenSBI]

This is not hypothetical and even has a problem on ELF files.
OpenSBI is the prime example.

OpenSBI can be compiled with the current RISC-V GNU Toolchain (with
privileged specification version 1.11) but uses hypervisor instructions and
privileged specification 1.12 CSRs.  It also has ELF arch attribute
"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0" (RV64GC with ISA version 2.2) if we
compile for RV64 and there's no H-extension reference here.

An Excerpt from the Source Code (OpenSBI 1.1, lib/sbi/sbi_hfence.S):

        .align 3
        .global __sbi_hfence_gvma_vmid_gpa
    __sbi_hfence_gvma_vmid_gpa:
        /*
        * rs1 = a0 (GPA >> 2)
        * rs2 = a1 (VMID)
        * HFENCE.GVMA a0, a1
        * 0110001 01011 01010 000 00000 1110011
        */
        .word 0x62b50073
        ret

Before:

-   Command:
    riscv64-unknown-elf-objdump -d fw_jump.elf
-   Target:
    fw_jump.elf from OpenSBI 1.1 (RV64)
-   Compiler/Toolchain:
    RISC-V GNU Toolchain 2022.06.10

    00000000800097c8 <__sbi_hfence_gvma_vmid_gpa>:
        800097c8:  62b50073                .4byte  0x62b50073
        800097cc:  8082                    ret
        800097ce:  0001                    nop

After:

-   Command:
    riscv64-unknown-elf-objdump -M arch=rv64gch -d fw_jump.elf

    00000000800097c8 <__sbi_hfence_gvma_vmid_gpa>:
        800097c8:  62b50073                hfence.gvma     a0,a1
        800097cc:  8082                    ret
        800097ce:  0001                    nop

Note:

Even after this patchset, only 1 of 8 "hfence.*" instructions in OpenSBI
is correctly disassembled on objdump but the reason is separate: OpenSBI
uses ".word", not ".insn" (the disassembler handles some instructions
emitted with ".word" as data).  In this case, "objdump -D" can
be a workaround.



[PATCH 2/3: RISC-V: Add -M arch disassembler option]

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

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

1.  BFD architecture set by either (in the following order):
    a.  Machine architecture
        (-m riscv:rv[32|64] / set arch riscv:rv[32|64])
    b.  ELF class in the ELF header
        (only when disassembling RISC-V ELF files)
    Note:
    This is effective only if XLEN-specific RISC-V BFD architecture
    is set.  For instance, if XLEN-neutral machine is specified by
    "-m riscv", BFD architecture is ignored on XLEN selection.
2.  ISA string set by either (in the following order):
    a.  ISA string option
        (-M arch=rv[32|64].. / set disassembler-options arch=..)
    b.  ELF mapping symbols with ISA string
        (only when disassembling RISC-V ELF files)
    c.  ELF attributes
        (only when disassembling RISC-V ELF files)
3.  ELF class in the "ELF header"

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 generate a warning if XLEN if they conflict
because of "ELF header" (may be a dummy while processing a binary file
or not useful if the input is a non-RISC-V ELF file) and possible
flexibility when used together with GDB.

Still, adding it might be an option.

Cross-toolchain XLEN precedence rules with some prerequisites will
look like this (of course, rules above conforms to it):

0.  When disassembling, the disassembler MAY infer the target machine
    (with or without specific XLEN) from the input or other conditions
    automatically unless the target machine is explicitly specified.
0.  If the ISA string is not specified, the disassembler SHALL use either
    the architecture from mapping symbols with ISA string (if exists),
    architecture tag from ELF attributes or the default ISA and extensions
    matching the target machine (as possible).
    This default ISA is normally either "rv32gc" or "rv64gc" but the
    disassembler MAY have its own defaults.

1.  If the target machine is not set or XLEN-neutral (can be both
    RV32 or RV64, "-m riscv" on objdump for example), XLEN portion of
    the ISA string takes precedence.
2.  If the target machine is XLEN-specific (either RV32 or RV64),
    target XLEN takes precedence.  If the ISA string is specified but
    differs in XLEN, the disassembler MAY:
    -   raise an error or,
    -   ignore XLEN part of the ISA string and try to conform given ISA
        (except XLEN) as possible.



[Changes between RFC PATCH v2 and PATCH v3]

-   Rebased
-   Renamed option name from "isa" to "arch" based on precious feedback
    from Palmer Dabbelt.
-   XLEN precedence rules are updated per actual behavior (auto-setting
    BFD architecture by given RISC-V ELF files were not considered in
    RFC PATCH v1 and v2).
-   Added some tests to check architecture override.
-   Added support for mapping symbols with ISA string.



[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 (3):
  RISC-V: Make "priv-spec" overridable
  RISC-V: Add "arch" disassembler option
  gdb/testsuite: RISC-V disassembler option tests

 gas/testsuite/gas/riscv/dis-arch-override-1.d |  13 ++
 gas/testsuite/gas/riscv/dis-arch-override-2.d |  13 ++
 gas/testsuite/gas/riscv/dis-arch-override-3.d |  13 ++
 gas/testsuite/gas/riscv/dis-arch-override.s   |  45 ++++++
 .../gas/riscv/dis-priv-spec-override-1.d      |  10 ++
 .../gas/riscv/dis-priv-spec-override-2.d      |  10 ++
 .../gas/riscv/dis-priv-spec-override.s        |   2 +
 .../gdb.arch/riscv-disassembler-options.exp   | 129 ++++++++++++++++++
 .../gdb.arch/riscv-disassembler-options.s     |  29 ++++
 opcodes/riscv-dis.c                           | 100 ++++++++++----
 10 files changed, 337 insertions(+), 27 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override-1.d
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override-2.d
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override-3.d
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override.s
 create mode 100644 gas/testsuite/gas/riscv/dis-priv-spec-override-1.d
 create mode 100644 gas/testsuite/gas/riscv/dis-priv-spec-override-2.d
 create mode 100644 gas/testsuite/gas/riscv/dis-priv-spec-override.s
 create mode 100644 gdb/testsuite/gdb.arch/riscv-disassembler-options.exp
 create mode 100644 gdb/testsuite/gdb.arch/riscv-disassembler-options.s


base-commit: f3fcf98b44621fb8768cf11121d3fd77089bca5b
-- 
2.38.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/3] RISC-V: Make "priv-spec" overridable
  2022-11-20  2:23 [PATCH v3 0/3] RISC-V: Add overridable "priv-spec" and "arch" disassembler options Tsukasa OI
@ 2022-11-20  2:23 ` Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 2/3] RISC-V: Add "arch" disassembler option Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 3/3] gdb/testsuite: RISC-V disassembler option tests Tsukasa OI
  2 siblings, 0 replies; 4+ messages in thread
From: Tsukasa OI @ 2022-11-20  2:23 UTC (permalink / raw)
  To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils, gdb-patches

From: Tsukasa OI <research_trasio@irq.a4lg.com>

This commit makes existing disassembler option "priv-spec" overridable on
ELF files with attributes.

Existing implementation is helpful on debugging binary files but on ELF
files, the value specified by "priv-spec" option must match the attributes.

However, there's a case where privileged specification ELF attributes and
actual CSRs as used in the program differs.  For instance, OpenSBI is the
prime example.

This commit enables objdump and GDB to ignore ELF attributes but instead
use custom specification (e.g. even if OpenSBI is compiled with priv-spec
1.11 toolchain, we can correctly disassemble with priv-spec=1.12 option).

gas/ChangeLog:

	* testsuite/gas/riscv/dis-priv-spec-override.s: New privileged
	specification override tests.
	* testsuite/gas/riscv/dis-priv-spec-override-1.d: Likewise.
	* testsuite/gas/riscv/dis-priv-spec-override-2.d: Likewise.

opcodes/ChangeLog:

	* riscv-dis.c (default_priv_spec) Set to initial default version.
	(priv_spec, is_custom_priv_spec) New.
	(is_init_csr): Define as a file-scope variable instead a local
	variable of print_insn_args.
	(init_riscv_dis_state_for_arch_and_options): Keep track of
	"priv-spec" changes.
	(set_default_riscv_dis_options): Initialize "priv-spec".
	(parse_riscv_dis_option): Make "priv-spec" overridable.
	(print_insn_args): Use file-scope is_init_csr variable.
	(riscv_get_disassembler): Move fallback of default_priv_spec
	from print_insn_args.  Initialize "priv-spec".
---
 .../gas/riscv/dis-priv-spec-override-1.d      | 10 +++
 .../gas/riscv/dis-priv-spec-override-2.d      | 10 +++
 .../gas/riscv/dis-priv-spec-override.s        |  2 +
 opcodes/riscv-dis.c                           | 70 ++++++++++++-------
 4 files changed, 68 insertions(+), 24 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/dis-priv-spec-override-1.d
 create mode 100644 gas/testsuite/gas/riscv/dis-priv-spec-override-2.d
 create mode 100644 gas/testsuite/gas/riscv/dis-priv-spec-override.s

diff --git a/gas/testsuite/gas/riscv/dis-priv-spec-override-1.d b/gas/testsuite/gas/riscv/dis-priv-spec-override-1.d
new file mode 100644
index 000000000000..2cceebbf35a4
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-priv-spec-override-1.d
@@ -0,0 +1,10 @@
+#as: -march=rv64i_zicsr -mpriv-spec=1.11
+#source: dis-priv-spec-override.s
+#objdump: -d -M priv-spec=1.12
+
+.*:[ 	]+file format .*
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ 	]+[0-9a-f]+:[ 	]+3d002573[ 	]+csrr[ 	]+a0,pmpaddr32
diff --git a/gas/testsuite/gas/riscv/dis-priv-spec-override-2.d b/gas/testsuite/gas/riscv/dis-priv-spec-override-2.d
new file mode 100644
index 000000000000..200955e0ffe5
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-priv-spec-override-2.d
@@ -0,0 +1,10 @@
+#as: -march=rv64i_zicsr -mpriv-spec=1.12
+#source: dis-priv-spec-override.s
+#objdump: -d -M priv-spec=1.11
+
+.*:[ 	]+file format .*
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ 	]+[0-9a-f]+:[ 	]+3d002573[ 	]+csrr[ 	]+a0,0x3d0
diff --git a/gas/testsuite/gas/riscv/dis-priv-spec-override.s b/gas/testsuite/gas/riscv/dis-priv-spec-override.s
new file mode 100644
index 000000000000..c3d1726e0b5a
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-priv-spec-override.s
@@ -0,0 +1,2 @@
+target:
+	csrr	a0, 0x3d0
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 328a34501549..e7dded63c402 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -49,7 +49,16 @@ static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
 
 /* Default privileged specification
    (as specified by the ELF attributes or the `priv-spec' option).  */
-static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
+static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_DRAFT - 1;
+
+/* Current privileged specification version.  */
+static enum riscv_spec_class priv_spec = PRIV_SPEC_CLASS_DRAFT - 1;
+
+/* If set, a custom privileged specification is specified.  */
+static bool is_custom_priv_spec = false;
+
+/* Reset when reinitializing the CSR hash table is required.  */
+static bool is_init_csr = false;
 
 /* RISC-V disassembler architecture context type.  */
 typedef struct
@@ -205,6 +214,18 @@ init_riscv_dis_state_for_arch (void)
 static void
 init_riscv_dis_state_for_arch_and_options (void)
 {
+  static bool init = false;
+  static enum riscv_spec_class prev_priv_spec;
+  /* Set current privileged specification.  */
+  if (!is_custom_priv_spec)
+    priv_spec = default_priv_spec;
+  /* First call to this function.  */
+  if (!init)
+    {
+      init = true;
+      /* Save initial options.  */
+      prev_priv_spec = priv_spec;
+    }
   /* If the architecture string is changed, update XLEN.  */
   if (is_arch_changed)
     update_riscv_dis_xlen (NULL);
@@ -215,7 +236,12 @@ init_riscv_dis_state_for_arch_and_options (void)
       = !riscv_subset_supports (&riscv_rps_dis, "zfinx")
 	    ? (is_numeric ? riscv_fpr_names_numeric : riscv_fpr_names_abi)
 	    : riscv_gpr_names;
+  /* Reset CSR hash table if either the architecture or the privileged
+     specification version is changed.  */
+  if (prev_priv_spec != priv_spec)
+    is_init_csr = false;
   /* Save previous options and mark them "unchanged".  */
+  prev_priv_spec = priv_spec;
   is_arch_changed = false;
 }
 
@@ -267,6 +293,7 @@ set_default_riscv_dis_options (void)
 {
   no_aliases = false;
   is_numeric = false;
+  is_custom_priv_spec = false;
 }
 
 /* Parse RISC-V disassembler option (without arguments).  */
@@ -314,21 +341,18 @@ parse_riscv_dis_option (const char *option)
   value = equal + 1;
   if (strcmp (option, "priv-spec") == 0)
     {
-      enum riscv_spec_class priv_spec = PRIV_SPEC_CLASS_NONE;
-      const char *name = NULL;
-
-      RISCV_GET_PRIV_SPEC_CLASS (value, priv_spec);
-      if (priv_spec == PRIV_SPEC_CLASS_NONE)
-	opcodes_error_handler (_("unknown privileged spec set by %s=%s"),
-			       option, value);
-      else if (default_priv_spec == PRIV_SPEC_CLASS_NONE)
-	default_priv_spec = priv_spec;
-      else if (default_priv_spec != priv_spec)
+      enum riscv_spec_class priv_spec_new = PRIV_SPEC_CLASS_NONE;
+      RISCV_GET_PRIV_SPEC_CLASS (value, priv_spec_new);
+      if (priv_spec_new == PRIV_SPEC_CLASS_NONE)
 	{
-	  RISCV_GET_PRIV_SPEC_NAME (name, default_priv_spec);
-	  opcodes_error_handler (_("mis-matched privilege spec set by %s=%s, "
-				   "the elf privilege attribute is %s"),
-				 option, value, name);
+	  opcodes_error_handler (_ ("unknown privileged spec set by %s=%s."
+				    "not overriding"),
+				 option, value);
+	}
+      else
+	{
+	  is_custom_priv_spec = true;
+	  priv_spec = priv_spec_new;
 	}
     }
   else
@@ -721,31 +745,26 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
 	case 'E':
 	  {
 	    static const char *riscv_csr_hash[4096]; /* Total 2^12 CSRs.  */
-	    static bool init_csr = false;
 	    unsigned int csr = EXTRACT_OPERAND (CSR, l);
 
-	    if (!init_csr)
+	    if (!is_init_csr)
 	      {
 		unsigned int i;
 		for (i = 0; i < 4096; i++)
 		  riscv_csr_hash[i] = NULL;
 
-		/* Set to the newest privileged version.  */
-		if (default_priv_spec == PRIV_SPEC_CLASS_NONE)
-		  default_priv_spec = PRIV_SPEC_CLASS_DRAFT - 1;
-
 #define DECLARE_CSR(name, num, class, define_version, abort_version)	\
 		if (riscv_csr_hash[num] == NULL 			\
 		    && ((define_version == PRIV_SPEC_CLASS_NONE 	\
 			 && abort_version == PRIV_SPEC_CLASS_NONE)	\
-			|| (default_priv_spec >= define_version 	\
-			    && default_priv_spec < abort_version)))	\
+			|| (priv_spec >= define_version 	\
+			    && priv_spec < abort_version)))	\
 		  riscv_csr_hash[num] = #name;
 #define DECLARE_CSR_ALIAS(name, num, class, define_version, abort_version) \
 		DECLARE_CSR (name, num, class, define_version, abort_version)
 #include "opcode/riscv-opc.h"
 #undef DECLARE_CSR
-		init_csr = true;
+		is_init_csr = true;
 	      }
 
 	    if (riscv_csr_hash[csr] != NULL)
@@ -1283,6 +1302,9 @@ riscv_get_disassembler (bfd *abfd)
 	     in the attributes, use the default value.  */
 	  if (!default_arch)
 	    default_arch = initial_default_arch;
+	  /* By default, set to the newest privileged version.  */
+	  if (default_priv_spec == PRIV_SPEC_CLASS_NONE)
+	    default_priv_spec = PRIV_SPEC_CLASS_DRAFT - 1;
 	}
     }
 
-- 
2.38.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 2/3] RISC-V: Add "arch" disassembler option
  2022-11-20  2:23 [PATCH v3 0/3] RISC-V: Add overridable "priv-spec" and "arch" disassembler options Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 1/3] RISC-V: Make "priv-spec" overridable Tsukasa OI
@ 2022-11-20  2:23 ` Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 3/3] gdb/testsuite: RISC-V disassembler option tests Tsukasa OI
  2 siblings, 0 replies; 4+ messages in thread
From: Tsukasa OI @ 2022-11-20  2:23 UTC (permalink / raw)
  To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils, gdb-patches

From: Tsukasa OI <research_trasio@irq.a4lg.com>

This commit adds new disassembler option "arch" to override architecture
string.  Existing implementations is helpful on regular ELF files but
for instance, when debugging a binary with OpenOCD + GDB, there's a case
where such information is not available.

Also, there's a case where changing the architecture to disassemble is
helpful, even on ELF files.  For instance, OpenSBI is the prime example.

This commit enables objdump and GDB to use custom architecture string
instead of ELF attributes and mapping symbols (e.g. even if OpenSBI is
compiled with -march=rv64gc, we can enable H extension when disassembling
OpenSBI by specifying an architecture string: rv64gch).

It can be also helpful to test overwrapping encodings (e.g. standard hints).

gas/ChangeLog:

	* testsuite/gas/riscv/dis-arch-override.s: New arch override tests.
	* testsuite/gas/riscv/dis-arch-override-1.d: Likewise.
	* testsuite/gas/riscv/dis-arch-override-2.d: Likewise.
	* testsuite/gas/riscv/dis-arch-override-3.d: Likewise.

opcodes/ChangeLog:

	* riscv-dis.c (is_custom_arch) New.
	(update_riscv_dis_xlen): Update XLEN precedence rules.
	(set_default_riscv_dis_options): Initialize "arch".
	(parse_riscv_dis_option): Add the "arch" option.
	(riscv_get_map_state): Ignore ISA string from mapping symbols if
	custom "arch" is specified.
	(riscv_get_disassembler): Update the architecture depending on the
	custom "arch" option.
	(riscv_option_arg_t) Add RISCV_OPTION_ARG_ARCH.
	(riscv_options): Add the "arch" option.
	(disassembler_options_riscv): Add the "arch" option.
---
 gas/testsuite/gas/riscv/dis-arch-override-1.d | 13 ++++++
 gas/testsuite/gas/riscv/dis-arch-override-2.d | 13 ++++++
 gas/testsuite/gas/riscv/dis-arch-override-3.d | 13 ++++++
 gas/testsuite/gas/riscv/dis-arch-override.s   | 45 +++++++++++++++++++
 opcodes/riscv-dis.c                           | 30 +++++++++++--
 5 files changed, 111 insertions(+), 3 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override-1.d
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override-2.d
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override-3.d
 create mode 100644 gas/testsuite/gas/riscv/dis-arch-override.s

diff --git a/gas/testsuite/gas/riscv/dis-arch-override-1.d b/gas/testsuite/gas/riscv/dis-arch-override-1.d
new file mode 100644
index 000000000000..1df81236e2aa
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-arch-override-1.d
@@ -0,0 +1,13 @@
+#as: -march=rv64imfd_zbb
+#source: dis-arch-override.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ 	]+[0-9a-f]+:[ 	]+00c5f053[ 	]+fadd\.s[ 	]+ft0,fa1,fa2
+[ 	]+[0-9a-f]+:[ 	]+02c5f053[ 	]+fadd\.d[ 	]+ft0,fa1,fa2
+[ 	]+[0-9a-f]+:[ 	]+30102573[ 	]+csrr[ 	]+a0,misa
+[ 	]+[0-9a-f]+:[ 	]+0805c53b[ 	]+zext\.h[ 	]+a0,a1
diff --git a/gas/testsuite/gas/riscv/dis-arch-override-2.d b/gas/testsuite/gas/riscv/dis-arch-override-2.d
new file mode 100644
index 000000000000..a556f293fb52
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-arch-override-2.d
@@ -0,0 +1,13 @@
+#as: -march=rv64imfd_zbb
+#source: dis-arch-override.s
+#objdump: -d -M arch=rv32im_zfinx_zbkb
+
+.*:[ 	]+file format .*
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ 	]+[0-9a-f]+:[ 	]+00c5f053[ 	]+fadd\.s[ 	]+zero,a1,a2
+[ 	]+[0-9a-f]+:[ 	]+02c5f053[ 	]+\.4byte[ 	]+0x2c5f053
+[ 	]+[0-9a-f]+:[ 	]+30102573[ 	]+csrr[ 	]+a0,misa
+[ 	]+[0-9a-f]+:[ 	]+0805c53b[ 	]+packw[ 	]+a0,a1,zero
diff --git a/gas/testsuite/gas/riscv/dis-arch-override-3.d b/gas/testsuite/gas/riscv/dis-arch-override-3.d
new file mode 100644
index 000000000000..924da72b2797
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-arch-override-3.d
@@ -0,0 +1,13 @@
+#as: -march=rv64imfd_zbb
+#source: dis-arch-override.s
+#objdump: -d -m riscv -M arch=rv32im_zfinx_zbkb,numeric
+
+.*:[ 	]+file format .*
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ 	]+[0-9a-f]+:[ 	]+00c5f053[ 	]+fadd\.s[ 	]+x0,x11,x12
+[ 	]+[0-9a-f]+:[ 	]+02c5f053[ 	]+\.4byte[ 	]+0x2c5f053
+[ 	]+[0-9a-f]+:[ 	]+30102573[ 	]+csrr[ 	]+x10,misa
+[ 	]+[0-9a-f]+:[ 	]+0805c53b[ 	]+\.4byte[ 	]+0x805c53b
diff --git a/gas/testsuite/gas/riscv/dis-arch-override.s b/gas/testsuite/gas/riscv/dis-arch-override.s
new file mode 100644
index 000000000000..0c2fec44ed3a
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-arch-override.s
@@ -0,0 +1,45 @@
+# Assembler configuration:
+#    -march=rv64imfd_zbb
+# Disassembler configurations:
+#    Test 1: -d
+#    Test 2: -d -M arch=rv32im_zfinx_zbkb
+#    Test 3: -d -m riscv -M arch=rv32im_zfinx_zbkb,numeric
+
+target:
+	# Assembler               : fadd.s (F)
+	# Disassembler (test 2/3) : fadd.s (Zfinx)
+	# Test that all three operands point to GPRs.
+	fadd.s	ft0, fa1, fa2
+
+	# Assembler               : fadd.d (D)
+	# Disassembler (test 2/3) : (invalid)
+	# On disassembler option on test 2, Zdinx is not present.  So,
+	# it should be disassembled as an invalid instruction.
+	fadd.d	ft0, fa1, fa2
+
+	# Assembler               : csrr (Zicsr)
+	# Disassembler (test 2/3) : csrr (Zicsr)
+	# When assembling, Zicsr is implied by F.  When disassembling,
+	# Zicsr is implied by Zfinx.  On both cases, csrr should be
+	# disassembled as csrr.
+	csrr	a0, misa
+
+	# Assembler               : zext.h (Zbb)
+	# Disassembler (test 2)   : packw  (Zbkb)
+	# Disassembler (test 3)   : (invalid)
+	# Since zext.h specialized instruction does not exist in Zbkb
+	# and we disassemble the output with Zbkb, this instruction
+	# should be disassembled as a packw instruction (on RV64).
+	#
+	# We specify arch=rv32im_zfinx_zbkb on disassembling on test
+	# 2 and 3.  But, XLEN part of the ISA string is effective
+	# only if XLEN-neutral machine is specified by `-m riscv' option
+	# (because we are disassembling 64-bit RISC-V ELF file, BFD
+	# architecture is set to `riscv:rv64' unless `-m' option
+	# is specified).
+	#
+	# As a result, test 3 (with `-m riscv' option) disassembles with
+	# RV32 but test 2 (without it) does with RV64.
+	# It changes the result of disassembling since packw instruction
+	# is invalid on RV32.
+	zext.h	a0, a1
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index e7dded63c402..03272d4f64ce 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -35,6 +35,9 @@
 /* Default architecture string (if not available).  */
 static const char *const initial_default_arch = "rv64gc";
 
+/* If set, a custom architecture string is specified.  */
+static bool is_custom_arch = false;
+
 /* Current XLEN for the disassembler.  */
 static unsigned xlen = 0;
 
@@ -183,8 +186,10 @@ update_riscv_dis_xlen (struct disassemble_info *info)
        This is only effective if XLEN-specific BFD machine architecture is
        chosen.  If XLEN-neutral (like riscv), BFD machine architecture is
        ignored on XLEN selection.
-     2. Non-default RISC-V architecture string set by either an ELF
-	attribute or a mapping symbol with ISA string.
+     2. Non-default RISC-V architecture string set by either:
+       a. -M arch=... option (GDB: set disassembler-options arch=...),
+       b. A mapping symbol with ISA string or
+       c. An ELF attribute
      3. ELF class in dummy ELF header.  */
   if (xlen_by_mach != 0)
     xlen = xlen_by_mach;
@@ -294,6 +299,7 @@ set_default_riscv_dis_options (void)
   no_aliases = false;
   is_numeric = false;
   is_custom_priv_spec = false;
+  is_custom_arch = false;
 }
 
 /* Parse RISC-V disassembler option (without arguments).  */
@@ -355,6 +361,11 @@ parse_riscv_dis_option (const char *option)
 	  priv_spec = priv_spec_new;
 	}
     }
+  else if (strcmp (option, "arch") == 0)
+    {
+      is_custom_arch = true;
+      update_riscv_dis_arch (&dis_arch_context_override, value);
+    }
   else
     {
       /* xgettext:c-format */
@@ -1024,6 +1035,9 @@ riscv_get_map_state (int n,
   *state = newstate;
   if (newstate == MAP_INSN && update)
   {
+    /* Skip if a custom architecture is specified.  */
+    if (is_custom_arch)
+      return true;
     if (arch)
       {
 	/* Override the architecture.  */
@@ -1308,7 +1322,10 @@ riscv_get_disassembler (bfd *abfd)
 	}
     }
 
-  update_riscv_dis_arch (&dis_arch_context_default, default_arch);
+  if (is_custom_arch)
+    set_riscv_dis_arch_context (&dis_arch_context_default, default_arch);
+  else
+    update_riscv_dis_arch (&dis_arch_context_default, default_arch);
   return print_insn_riscv;
 }
 
@@ -1359,6 +1376,7 @@ riscv_symbol_is_valid (asymbol * sym,
 typedef enum
 {
   RISCV_OPTION_ARG_NONE = -1,
+  RISCV_OPTION_ARG_ARCH,
   RISCV_OPTION_ARG_PRIV_SPEC,
 
   RISCV_OPTION_ARG_COUNT
@@ -1376,6 +1394,9 @@ static struct
   { "numeric",
     N_("Print numeric register names, rather than ABI names."),
     RISCV_OPTION_ARG_NONE },
+  { "arch=",
+    N_("Disassemble using specified ISA and extensions."),
+    RISCV_OPTION_ARG_ARCH },
   { "no-aliases",
     N_("Disassemble only into canonical instructions."),
     RISCV_OPTION_ARG_NONE },
@@ -1403,6 +1424,9 @@ disassembler_options_riscv (void)
 
       args = XNEWVEC (disasm_option_arg_t, num_args + 1);
 
+      args[RISCV_OPTION_ARG_ARCH].name = "ARCH";
+      args[RISCV_OPTION_ARG_ARCH].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.38.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 3/3] gdb/testsuite: RISC-V disassembler option tests
  2022-11-20  2:23 [PATCH v3 0/3] RISC-V: Add overridable "priv-spec" and "arch" disassembler options Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 1/3] RISC-V: Make "priv-spec" overridable Tsukasa OI
  2022-11-20  2:23 ` [PATCH v3 2/3] RISC-V: Add "arch" disassembler option Tsukasa OI
@ 2022-11-20  2:23 ` Tsukasa OI
  2 siblings, 0 replies; 4+ messages in thread
From: Tsukasa OI @ 2022-11-20  2:23 UTC (permalink / raw)
  To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils, gdb-patches

From: Tsukasa OI <research_trasio@irq.a4lg.com>

This commit adds RISC-V disassembler option tests for:

-   "no-aliases" and "numeric" options
-   overridable "priv-spec" option
-   new "arch" option

and makes sure that option override correctly works and they can be unset
thereafter (with "set disassembler-options" with no options).
---
 .../gdb.arch/riscv-disassembler-options.exp   | 129 ++++++++++++++++++
 .../gdb.arch/riscv-disassembler-options.s     |  29 ++++
 2 files changed, 158 insertions(+)
 create mode 100644 gdb/testsuite/gdb.arch/riscv-disassembler-options.exp
 create mode 100644 gdb/testsuite/gdb.arch/riscv-disassembler-options.s

diff --git a/gdb/testsuite/gdb.arch/riscv-disassembler-options.exp b/gdb/testsuite/gdb.arch/riscv-disassembler-options.exp
new file mode 100644
index 000000000000..e5548eb426d3
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-disassembler-options.exp
@@ -0,0 +1,129 @@
+# Copyright 2022 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test RISC-V disassembler options.
+
+if {![istarget "riscv*-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile .s
+set objfile [standard_output_file ${testfile}.o]
+
+set compile_flags { \
+    "additional_flags=-march=rv64i_zicsr -mabi=lp64" \
+    "additional_flags=-Xassembler -mpriv-spec=1.11" \
+    }
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object \
+      ${compile_flags}] != "" } {
+    untested "could not compile test program"
+    return
+}
+
+clean_restart ${objfile}
+
+proc riscv_set_disassembler_options { opts } {
+    gdb_test_no_output "set disassembler-options $opts"
+    gdb_test "show disassembler-options" \
+	"The current disassembler options are '$opts'\r\n.*" \
+	"show disassembler-options: $opts"
+}
+
+# We can disable disassembly using alias instructions.
+riscv_set_disassembler_options "no-aliases"
+gdb_test_sequence "disassemble test_func" "option: no-aliases" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\taddi\ta0,zero,1\r\n"
+    "\[^:\]+:\tcsrrs\ta0,0x3d0,zero\r\n"
+    "\[^:\]+:\t\\.4byte\t0x62000073\r\n"
+    "\[^:\]+:\tjalr\tzero,0\\(ra\\)\r\n"
+    "End of assembler dump\."
+}
+
+# We can disassemble using numeric register names.
+riscv_set_disassembler_options "numeric"
+gdb_test_sequence "disassemble test_func" "option: numeric" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\tli\tx10,1\r\n"
+    "\[^:\]+:\tcsrr\tx10,0x3d0\r\n"
+    "\[^:\]+:\t\\.4byte\t0x62000073\r\n"
+    "\[^:\]+:\tret\r\n"
+    "End of assembler dump\."
+}
+
+# We can switch to the privileged specification 1.11.
+riscv_set_disassembler_options "priv-spec=1.11"
+gdb_test_sequence "disassemble test_func" "privileged specification 1.11 CSRs" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\tli\ta0,1\r\n"
+    "\[^:\]+:\tcsrr\ta0,0x3d0\r\n"
+    "\[^:\]+:\t\\.4byte\t0x62000073\r\n"
+    "\[^:\]+:\tret\r\n"
+    "End of assembler dump\."
+}
+
+# We can switch to the privileged specification 1.12.
+riscv_set_disassembler_options "priv-spec=1.12"
+gdb_test_sequence "disassemble test_func" "privileged specification 1.12 CSRs" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\tli\ta0,1\r\n"
+    "\[^:\]+:\tcsrr\ta0,pmpaddr32\r\n"
+    "\[^:\]+:\t\\.4byte\t0x62000073\r\n"
+    "\[^:\]+:\tret\r\n"
+    "End of assembler dump\."
+}
+
+# We can enable the 'H'-extension support.
+riscv_set_disassembler_options "arch=rv64gch"
+gdb_test_sequence "disassemble test_func" "'H'-extension" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\tli\ta0,1\r\n"
+    "\[^:\]+:\tcsrr\ta0,0x3d0\r\n"
+    "\[^:\]+:\thfence\\.gvma\r\n"
+    "\[^:\]+:\tret\r\n"
+    "End of assembler dump\."
+}
+
+# We can set multiple comma-separated options
+riscv_set_disassembler_options "arch=rv64gch,priv-spec=1.12,no-aliases,numeric"
+gdb_test_sequence "disassemble test_func" "multiple options" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\taddi\tx10,x0,1\r\n"
+    "\[^:\]+:\tcsrrs\tx10,pmpaddr32,x0\r\n"
+    "\[^:\]+:\thfence\\.gvma\tx0,x0\r\n"
+    "\[^:\]+:\tjalr\tx0,0\\(x1\\)\r\n"
+    "End of assembler dump\."
+}
+
+# Once we set empty disassembler option, we can restore the default one.
+# Defaults:
+# - enable aliases
+# - ABI register names
+# - priv-spec=1.11
+# - arch=rv64i_zicsr
+gdb_test_no_output "set disassembler-options" "set NULL disassembler-options"
+gdb_test "show disassembler-options" \
+    "The current disassembler options are ''\r\n.*" \
+    "show NULL disassembler-options"
+gdb_test_sequence "disassemble test_func" "restore to default" {
+    "Dump of assembler code for function test_func:\r\n"
+    "\[^:\]+:\tli\ta0,1\r\n"
+    "\[^:\]+:\tcsrr\ta0,0x3d0\r\n"
+    "\[^:\]+:\t\\.4byte\t0x62000073\r\n"
+    "\[^:\]+:\tret\r\n"
+    "End of assembler dump\."
+}
diff --git a/gdb/testsuite/gdb.arch/riscv-disassembler-options.s b/gdb/testsuite/gdb.arch/riscv-disassembler-options.s
new file mode 100644
index 000000000000..fc33b03b0c00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-disassembler-options.s
@@ -0,0 +1,29 @@
+/* Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.attribute arch, "rv64i_zicsr"
+	.option nopic
+	.text
+
+	.align	1
+	.globl	test_func
+	.type	test_func, @function
+test_func:
+	li	a0, 1
+	csrr	a0, 0x3d0
+	# hfence.gvma (an alias of hfence.gvma zero,zero)
+	.insn	0x62000073
+	ret
+	.size	test_func, .-test_func
-- 
2.38.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-20  2:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-20  2:23 [PATCH v3 0/3] RISC-V: Add overridable "priv-spec" and "arch" disassembler options Tsukasa OI
2022-11-20  2:23 ` [PATCH v3 1/3] RISC-V: Make "priv-spec" overridable Tsukasa OI
2022-11-20  2:23 ` [PATCH v3 2/3] RISC-V: Add "arch" disassembler option Tsukasa OI
2022-11-20  2:23 ` [PATCH v3 3/3] gdb/testsuite: RISC-V disassembler option tests Tsukasa OI

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).