[PATCH v2 0/2] RISC-V: Fix abort when displaying data and add test

[PATCH v2 0/2] RISC-V: Fix abort when displaying data and add test

[Charlie Jenkins]

Commit 6a04e8230707 ("RISC-V: Fix display of partial instructions")
changed how objdump displays instructions. This enabled data of any size
to be dumped. riscv_disassemble_data() only supports data of size 1,
2, 4, or 8 bytes and any other number would cause an abort.

Instead of aborting when encountering one of the other values, dump the
data as ".<N>byte <val>", similar to what is done for instructions with
".insn <N>, <val>".

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
Changes in v2:
- Dump arbitrary data with ".<N>byte <val>"
- Link to v1: https://lore.kernel.org/r/20250211-fix_gas_abort-v1-0-afd9730f9c51@rivosinc.com

---
Charlie Jenkins (2):
      RISC-V: Fix abort when displaying .dword
      RISC-V: Add testcase for 6 byte instruction

 gas/testsuite/gas/riscv/dis-partial-insn-dword.d | 12 ++++++++++++
 gas/testsuite/gas/riscv/dis-partial-insn-dword.s |  2 ++
 gas/testsuite/gas/riscv/dis-partial-insn-word.d  |  2 +-
 opcodes/riscv-dis.c                              | 19 ++++++++++---------
 4 files changed, 25 insertions(+), 10 deletions(-)
---
base-commit: f1ffed29357a30162506c0c895f2927c7c2a4aba
change-id: 20250211-fix_gas_abort-6d1e28b4ad46
-- 
- Charlie

[PATCH v2 1/2] RISC-V: Fix abort when displaying .dword

[Charlie Jenkins]

In the normal case an instruction won't be split into 5, 6, or 7 byte
sections. However a .dword disassembled with -D can cause an instruction
to split across the 6 byte boundary. 6 byte instructions were not
supported so riscv_disassemble_data() would abort.

If data is encountered that is not a power of two, dump all of the data
with a .<N>byte directive.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
Fixes: 6a04e8230707 ("RISC-V: Fix display of partial instructions")
---
 opcodes/riscv-dis.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 3956f588f8f749935b23e6a4dc4fab95ef66d013..d02723ed38ff368e9ce0a48e49d8f87820551a50 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1317,14 +1317,6 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
       (*info->fprintf_styled_func)
 	(info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
       break;
-    case 3:
-      info->bytes_per_line = 7;
-      (*info->fprintf_styled_func)
-	(info->stream, dis_style_assembler_directive, ".word");
-      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
-      (*info->fprintf_styled_func)
-	(info->stream, dis_style_immediate, "0x%06x", (unsigned) data);
-      break;
     case 4:
       info->bytes_per_line = 8;
       (*info->fprintf_styled_func)
@@ -1344,7 +1336,16 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
 	 (unsigned long long) data);
       break;
     default:
-      abort ();
+      /* Arbitrary data so just print the bits in the shape of an .<N>byte
+	 directive.  */
+      info->bytes_per_line = info->bytes_per_chunk;
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_assembler_directive, ".%dbyte", info->bytes_per_chunk);
+      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_immediate, "0x%0llx",
+	  (unsigned long long) data);
+      break;
     }
   return info->bytes_per_chunk;
 }

-- 
2.43.0

[PATCH v2 2/2] RISC-V: Add testcase for 6 byte instruction

[Charlie Jenkins]

6-byte instructions were causing gas to abort. Add a test to ensure this
is avoided in the future.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
 gas/testsuite/gas/riscv/dis-partial-insn-dword.d | 12 ++++++++++++
 gas/testsuite/gas/riscv/dis-partial-insn-dword.s |  2 ++
 gas/testsuite/gas/riscv/dis-partial-insn-word.d  |  2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/gas/testsuite/gas/riscv/dis-partial-insn-dword.d b/gas/testsuite/gas/riscv/dis-partial-insn-dword.d
new file mode 100644
index 0000000000000000000000000000000000000000..c5f44f533fff6949409975ca7fab51a859aabfd9
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-partial-insn-dword.d
@@ -0,0 +1,12 @@
+#as:
+#source: dis-partial-insn-dword.s
+#objdump: -D -j .text
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ 	]+0:[ 	]+78f9[ 	]+.insn[ 	]+2, 0x78f9
+[ 	]+2:[ 	]+2f2f313421ff[ 	]+.6byte[ 	]+0x2f2f313421ff
diff --git a/gas/testsuite/gas/riscv/dis-partial-insn-dword.s b/gas/testsuite/gas/riscv/dis-partial-insn-dword.s
new file mode 100644
index 0000000000000000000000000000000000000000..8aa94b1a8cbd57fb00bb4197697e7a8bb94a8bcb
--- /dev/null
+++ b/gas/testsuite/gas/riscv/dis-partial-insn-dword.s
@@ -0,0 +1,2 @@
+target:
+	.dword 0x2f2f313421ff78f9
diff --git a/gas/testsuite/gas/riscv/dis-partial-insn-word.d b/gas/testsuite/gas/riscv/dis-partial-insn-word.d
index 2f52153075e77361a3dd3f1e3071949eb89c6fe6..0bb43fca2e22d0e2363c70032b988dfc96fe3585 100644
--- a/gas/testsuite/gas/riscv/dis-partial-insn-word.d
+++ b/gas/testsuite/gas/riscv/dis-partial-insn-word.d
@@ -8,4 +8,4 @@
 Disassembly of section .text:
 
 0+000 <target>:
-[ 	]+0:[ 	]+000013[ 	]+.word[ 	]+0x000013
+[ 	]+0:[ 	]+000013[ 	]+.3byte[ 	]+0x13

-- 
2.43.0

Re: [PATCH v2 2/2] RISC-V: Add testcase for 6 byte instruction

[Jan Beulich]

On 19.02.2025 04:35, Charlie Jenkins wrote:
> --- a/gas/testsuite/gas/riscv/dis-partial-insn-word.d
> +++ b/gas/testsuite/gas/riscv/dis-partial-insn-word.d
> @@ -8,4 +8,4 @@
>  Disassembly of section .text:
>  
>  0+000 <target>:
> -[ 	]+0:[ 	]+000013[ 	]+.word[ 	]+0x000013
> +[ 	]+0:[ 	]+000013[ 	]+.3byte[ 	]+0x13

Doesn't this need to be part of patch 1?

Jan

Re: [PATCH v2 2/2] RISC-V: Add testcase for 6 byte instruction

[Charlie Jenkins]

On Wed, Feb 19, 2025 at 10:25:13AM +0100, Jan Beulich wrote:
> On 19.02.2025 04:35, Charlie Jenkins wrote:
> > --- a/gas/testsuite/gas/riscv/dis-partial-insn-word.d
> > +++ b/gas/testsuite/gas/riscv/dis-partial-insn-word.d
> > @@ -8,4 +8,4 @@
> >  Disassembly of section .text:
> >  
> >  0+000 <target>:
> > -[ 	]+0:[ 	]+000013[ 	]+.word[ 	]+0x000013
> > +[ 	]+0:[ 	]+000013[ 	]+.3byte[ 	]+0x13
> 
> Doesn't this need to be part of patch 1?

Yes it does, thank you. 

- Charlie

> 
> Jan
>