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

[PATCH 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 had the unintented
side-effect that it allowed 6 byte instructions to be displayed which
happens when a .dword is attempted to be split into instructions. Add
support for 5, 6, and 7 byte instructions to remedy this
issue.

Signed-off-by: Charlie Jenkins <charlie@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 ++
 opcodes/riscv-dis.c                              | 29 +++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)
---
base-commit: 815d9a14cbbb3b81843f7566222c87fb22e7255d
change-id: 20250211-fix_gas_abort-6d1e28b4ad46
-- 
- Charlie

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

Forcing instructions to be at most 4 bytes causes other unintented
side-effects, so instead add entries to the switch statement to handle
instructions that are 5, 6, or 7 bytes.

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

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 367004d3341e46a5f72253cd70c7c2941912e84d..21bcbd32c7f226d13f1637ad192d8fe3c52e0d7a 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1325,6 +1325,33 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
 	(info->stream, dis_style_immediate, "0x%08lx",
 	 (unsigned long) data);
       break;
+    case 5:
+      info->bytes_per_line = 8;
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_assembler_directive, ".dword");
+      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_immediate, "0x%010lx",
+	 (unsigned long) data);
+      break;
+    case 6:
+      info->bytes_per_line = 8;
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_assembler_directive, ".dword");
+      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_immediate, "0x%012lx",
+	 (unsigned long) data);
+      break;
+    case 7:
+      info->bytes_per_line = 8;
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_assembler_directive, ".dword");
+      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+      (*info->fprintf_styled_func)
+	(info->stream, dis_style_immediate, "0x%014lx",
+	 (unsigned long) data);
+      break;
     case 8:
       info->bytes_per_line = 8;
       (*info->fprintf_styled_func)
@@ -1332,7 +1359,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
       (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
       (*info->fprintf_styled_func)
 	(info->stream, dis_style_immediate, "0x%016llx",
-	 (unsigned long long) data);
+	 (unsigned long) data);
       break;
     default:
       abort ();

-- 
2.43.0

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

[Jiawei]

在 2025/2/12 13:08, 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.
>
> Forcing instructions to be at most 4 bytes causes other unintented
> side-effects, so instead add entries to the switch statement to handle
> instructions that are 5, 6, or 7 bytes.
>
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> Fixes: 6a04e8230707 ("RISC-V: Fix display of partial instructions")
> ---
>   opcodes/riscv-dis.c | 29 ++++++++++++++++++++++++++++-
>   1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 367004d3341e46a5f72253cd70c7c2941912e84d..21bcbd32c7f226d13f1637ad192d8fe3c52e0d7a 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -1325,6 +1325,33 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>   	(info->stream, dis_style_immediate, "0x%08lx",
>   	 (unsigned long) data);
>         break;
> +    case 5:
> +      info->bytes_per_line = 8;
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_assembler_directive, ".dword");
> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_immediate, "0x%010lx",
> +	 (unsigned long) data);
> +      break;
> +    case 6:
> +      info->bytes_per_line = 8;
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_assembler_directive, ".dword");
> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_immediate, "0x%012lx",
> +	 (unsigned long) data);
> +      break;
> +    case 7:
> +      info->bytes_per_line = 8;
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_assembler_directive, ".dword");
> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_immediate, "0x%014lx",
> +	 (unsigned long) data);
> +      break;
>       case 8:
>         info->bytes_per_line = 8;
>         (*info->fprintf_styled_func)
> @@ -1332,7 +1359,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>         (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>         (*info->fprintf_styled_func)
>   	(info->stream, dis_style_immediate, "0x%016llx",
> -	 (unsigned long long) data);
> +	 (unsigned long) data);
>         break;
>       default:
>         abort ();

LGTM, thanks.

Jiawei

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

[Alan Modra]

On Tue, Feb 11, 2025 at 09:08:30PM -0800, Charlie Jenkins wrote:
> 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.

I think it is worse than just needing to cope with 5, 6, or 7 bytes.
riscv-dis.c:1444 sets dump_size from riscv_insn_length which
looks like it can be up to 22 bytes.  A carefully constructed testcase
that gets this value from riscv_insn_length but then has only 20
bytes in the section will result in riscv_disassemble_data being
called with bytes_per_chunk of 20.

cat > xxx.s <<EOF
 .byte 0x7f,0xef,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20
EOF
gas/as-new -o xxx.o xxx.s
binutils/objcopy --strip-symbol '$d' xxx.o xx.o
binutils/objdump -d xx.o

-- 
Alan Modra

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

[Jan Beulich]

On 12.02.2025 06:08, Charlie Jenkins wrote:
> 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.
> 
> Forcing instructions to be at most 4 bytes causes other unintented
> side-effects, so instead add entries to the switch statement to handle
> instructions that are 5, 6, or 7 bytes.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> Fixes: 6a04e8230707 ("RISC-V: Fix display of partial instructions")
> ---
>  opcodes/riscv-dis.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 367004d3341e46a5f72253cd70c7c2941912e84d..21bcbd32c7f226d13f1637ad192d8fe3c52e0d7a 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -1325,6 +1325,33 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>  	(info->stream, dis_style_immediate, "0x%08lx",
>  	 (unsigned long) data);
>        break;
> +    case 5:
> +      info->bytes_per_line = 8;
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_assembler_directive, ".dword");
> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_immediate, "0x%010lx",
> +	 (unsigned long) data);

This isn't going to have the intended effect on a 32-bit host.

> +      break;
> +    case 6:
> +      info->bytes_per_line = 8;
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_assembler_directive, ".dword");
> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_immediate, "0x%012lx",
> +	 (unsigned long) data);
> +      break;
> +    case 7:
> +      info->bytes_per_line = 8;
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_assembler_directive, ".dword");
> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> +      (*info->fprintf_styled_func)
> +	(info->stream, dis_style_immediate, "0x%014lx",
> +	 (unsigned long) data);
> +      break;

While this follows what's done in the 3-byte case, I don't consider this
(or the 3-byte logic) correct. When I see .dword, I expect what's printed
covers full 8 bytes. Imo fake .<N>byte directives (which the assembler
doesn't recognize for non-power-of-2 N) would be more logical to use.

Also, question to the maintainers: Can we perhaps stop this unnecessary
decoration of indirect function calls? E.g. (taking part of the above)

    case 5:
      info->bytes_per_line = 8;
      info->fprintf_styled_func
	(info->stream, dis_style_assembler_directive, ".dword");
      info->fprintf_styled_func (info->stream, dis_style_text, "\t");
      info->fprintf_styled_func
	(info->stream, dis_style_immediate, "0x%010lx",
	 (unsigned long) data);
      break;

has been perfectly fine to write for several decades, and is imo quite
a bit easier to read.

> @@ -1332,7 +1359,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>        (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>        (*info->fprintf_styled_func)
>  	(info->stream, dis_style_immediate, "0x%016llx",
> -	 (unsigned long long) data);
> +	 (unsigned long) data);

This can't be correct; the compiler will complain about format specifier
disagreeing with type of the value to format, on 32-bit hosts.

Jan

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

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

On Wed, Feb 12, 2025 at 4:06 PM Jan Beulich <jbeulich@suse.com> wrote:

> While this follows what's done in the 3-byte case, I don't consider this
> (or the 3-byte logic) correct. When I see .dword, I expect what's printed
> covers full 8 bytes. Imo fake .<N>byte directives (which the assembler
> doesn't recognize for non-power-of-2 N) would be more logical to use.
>
> Also, question to the maintainers: Can we perhaps stop this unnecessary
> decoration of indirect function calls? E.g. (taking part of the above)
>
>     case 5:
>       info->bytes_per_line = 8;
>       info->fprintf_styled_func
>         (info->stream, dis_style_assembler_directive, ".dword");
>       info->fprintf_styled_func (info->stream, dis_style_text, "\t");
>       info->fprintf_styled_func
>         (info->stream, dis_style_immediate, "0x%010lx",
>          (unsigned long) data);
>       break;
>
> has been perfectly fine to write for several decades, and is imo quite
> a bit easier to read.
>

Well if I see .word, I also expect to print the full 4 bytes, so probably
just remove the case 3, and then make sure that we only have 1/2/4/8 for
data, which means .byte/.short/.word/.dword.  That is -

 opcodes/riscv-dis.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index d67b2c2aaf0..fc774760f8c 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1308,14 +1308,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)
@@ -1497,6 +1489,10 @@ print_insn_riscv (bfd_vma memaddr, struct
disassemble_info *info)
     }
   else if (bytes_fetched != dump_size)
     {
+      bytes_fetched = bytes_fetched >= 8
+                     ? 8 : bytes_fetched >= 4
+                           ? 4 : bytes_fetched >= 2
+                                 ? 2 : bytes_fetched;
       dump_size = bytes_fetched;
       info->bytes_per_chunk = dump_size;
       riscv_disassembler = riscv_disassemble_data;

This may make stuff easier, and the logic is similar to riscv_data_length
when the length is 3.

Nelson

[-- Attachment #2: Type: text/html, Size: 3467 bytes --]

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

[Jan Beulich]

On 12.02.2025 09:57, Nelson Chu wrote:
> On Wed, Feb 12, 2025 at 4:06 PM Jan Beulich <jbeulich@suse.com> wrote:
> 
>> While this follows what's done in the 3-byte case, I don't consider this
>> (or the 3-byte logic) correct. When I see .dword, I expect what's printed
>> covers full 8 bytes. Imo fake .<N>byte directives (which the assembler
>> doesn't recognize for non-power-of-2 N) would be more logical to use.
> 
> Well if I see .word, I also expect to print the full 4 bytes, so probably
> just remove the case 3, and then make sure that we only have 1/2/4/8 for
> data, which means .byte/.short/.word/.dword.

Except that there is no 4th byte to print in that case. That wants expressing
somehow, without taking a value out of thin air.

Jan

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

[Maciej W. Rozycki]

On Wed, 12 Feb 2025, Jan Beulich wrote:

> >> While this follows what's done in the 3-byte case, I don't consider this
> >> (or the 3-byte logic) correct. When I see .dword, I expect what's printed
> >> covers full 8 bytes. Imo fake .<N>byte directives (which the assembler
> >> doesn't recognize for non-power-of-2 N) would be more logical to use.
> > 
> > Well if I see .word, I also expect to print the full 4 bytes, so probably
> > just remove the case 3, and then make sure that we only have 1/2/4/8 for
> > data, which means .byte/.short/.word/.dword.
> 
> Except that there is no 4th byte to print in that case. That wants expressing
> somehow, without taking a value out of thin air.

 They could just do what backends for other fixed-length-instruction-word 
targets do, such as Alpha or MIPS one:

$ dd if=/dev/zero of=partial-insn.img bs=1 count=7
$ alpha-linux-gnu-objdump -b binary -m alpha -D partial-insn.img

partial-insn.img:     file format binary


Disassembly of section .data:

0000000000000000 <.data>:
   0:	00 00 00 00 	halt
   4:	Address 0x0000000000000004 is out of bounds.

$ mips-linux-gnu-objdump -b binary -m mips -D partial-insn.img

partial-insn.img:     file format binary


Disassembly of section .data:

00000000 <.data>:
   0:	00000000 	nop
   4:	Address 0x4 is out of bounds.

$ 

There are other means available, such as the `-s' option or `readelf', for 
dumping trailing partial data that is not a complete instruction and thus 
has no meaning.  I'm not sure if this is a corner case worth putting much 
effort into, beyond just making sure the tools don't crash or produce 
utter rubbish.

 FWIW,

  Maciej

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

[Charlie Jenkins]

On Wed, Feb 12, 2025 at 10:15:31PM +0000, Maciej W. Rozycki wrote:
> On Wed, 12 Feb 2025, Jan Beulich wrote:
> 
> > >> While this follows what's done in the 3-byte case, I don't consider this
> > >> (or the 3-byte logic) correct. When I see .dword, I expect what's printed
> > >> covers full 8 bytes. Imo fake .<N>byte directives (which the assembler
> > >> doesn't recognize for non-power-of-2 N) would be more logical to use.
> > > 
> > > Well if I see .word, I also expect to print the full 4 bytes, so probably
> > > just remove the case 3, and then make sure that we only have 1/2/4/8 for
> > > data, which means .byte/.short/.word/.dword.
> > 
> > Except that there is no 4th byte to print in that case. That wants expressing
> > somehow, without taking a value out of thin air.
> 
>  They could just do what backends for other fixed-length-instruction-word 
> targets do, such as Alpha or MIPS one:
> 
> $ dd if=/dev/zero of=partial-insn.img bs=1 count=7
> $ alpha-linux-gnu-objdump -b binary -m alpha -D partial-insn.img
> 
> partial-insn.img:     file format binary
> 
> 
> Disassembly of section .data:
> 
> 0000000000000000 <.data>:
>    0:	00 00 00 00 	halt
>    4:	Address 0x0000000000000004 is out of bounds.
> 
> $ mips-linux-gnu-objdump -b binary -m mips -D partial-insn.img
> 
> partial-insn.img:     file format binary
> 
> 
> Disassembly of section .data:
> 
> 00000000 <.data>:
>    0:	00000000 	nop
>    4:	Address 0x4 is out of bounds.
> 
> $ 
> 
> There are other means available, such as the `-s' option or `readelf', for 
> dumping trailing partial data that is not a complete instruction and thus 
> has no meaning.  I'm not sure if this is a corner case worth putting much 
> effort into, beyond just making sure the tools don't crash or produce 
> utter rubbish.

It came up because there was a Linux test that was using objdump to
check that instructions in memory was equal to the intructions in the
binary. It was using --stop-address to bound a region to look at and
that was sometimes splitting an instruction down the middle. This used
to work fine with binutils 2.41 on riscv, but a change was made that
caused this throw an out of bounds on 2.42. My original change was to
bring the behavior back in line with other architectures and what riscv
used to do. I feel like it's a somewhat useful feature.

- Charlie

> 
>  FWIW,
> 
>   Maciej

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

[Charlie Jenkins]

On Wed, Feb 12, 2025 at 09:06:43AM +0100, Jan Beulich wrote:
> On 12.02.2025 06:08, Charlie Jenkins wrote:
> > 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.
> > 
> > Forcing instructions to be at most 4 bytes causes other unintented
> > side-effects, so instead add entries to the switch statement to handle
> > instructions that are 5, 6, or 7 bytes.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > Fixes: 6a04e8230707 ("RISC-V: Fix display of partial instructions")
> > ---
> >  opcodes/riscv-dis.c | 29 ++++++++++++++++++++++++++++-
> >  1 file changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> > index 367004d3341e46a5f72253cd70c7c2941912e84d..21bcbd32c7f226d13f1637ad192d8fe3c52e0d7a 100644
> > --- a/opcodes/riscv-dis.c
> > +++ b/opcodes/riscv-dis.c
> > @@ -1325,6 +1325,33 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
> >  	(info->stream, dis_style_immediate, "0x%08lx",
> >  	 (unsigned long) data);
> >        break;
> > +    case 5:
> > +      info->bytes_per_line = 8;
> > +      (*info->fprintf_styled_func)
> > +	(info->stream, dis_style_assembler_directive, ".dword");
> > +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> > +      (*info->fprintf_styled_func)
> > +	(info->stream, dis_style_immediate, "0x%010lx",
> > +	 (unsigned long) data);
> 
> This isn't going to have the intended effect on a 32-bit host.
> 
> > +      break;
> > +    case 6:
> > +      info->bytes_per_line = 8;
> > +      (*info->fprintf_styled_func)
> > +	(info->stream, dis_style_assembler_directive, ".dword");
> > +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> > +      (*info->fprintf_styled_func)
> > +	(info->stream, dis_style_immediate, "0x%012lx",
> > +	 (unsigned long) data);
> > +      break;
> > +    case 7:
> > +      info->bytes_per_line = 8;
> > +      (*info->fprintf_styled_func)
> > +	(info->stream, dis_style_assembler_directive, ".dword");
> > +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> > +      (*info->fprintf_styled_func)
> > +	(info->stream, dis_style_immediate, "0x%014lx",
> > +	 (unsigned long) data);
> > +      break;
> 
> While this follows what's done in the 3-byte case, I don't consider this
> (or the 3-byte logic) correct. When I see .dword, I expect what's printed
> covers full 8 bytes. Imo fake .<N>byte directives (which the assembler
> doesn't recognize for non-power-of-2 N) would be more logical to use.

That's a good point. I think the simplest thing to do would be to have
.short/.word/.dword for the 2/4/8 cases and then for anything outside of
that print .<N>byte. Similar to what is done with insn with printing
.insn <size>, <insn> when it is not identified.

- Charlie

> 
> Also, question to the maintainers: Can we perhaps stop this unnecessary
> decoration of indirect function calls? E.g. (taking part of the above)
> 
>     case 5:
>       info->bytes_per_line = 8;
>       info->fprintf_styled_func
> 	(info->stream, dis_style_assembler_directive, ".dword");
>       info->fprintf_styled_func (info->stream, dis_style_text, "\t");
>       info->fprintf_styled_func
> 	(info->stream, dis_style_immediate, "0x%010lx",
> 	 (unsigned long) data);
>       break;
> 
> has been perfectly fine to write for several decades, and is imo quite
> a bit easier to read.
> 
> > @@ -1332,7 +1359,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
> >        (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> >        (*info->fprintf_styled_func)
> >  	(info->stream, dis_style_immediate, "0x%016llx",
> > -	 (unsigned long long) data);
> > +	 (unsigned long) data);
> 
> This can't be correct; the compiler will complain about format specifier
> disagreeing with type of the value to format, on 32-bit hosts.
> 
> Jan

[PATCH 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 ++
 2 files changed, 14 insertions(+)

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..79aada549c5a69c8e79a2a0b8009e08a903ff43b
--- /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[ 	]+.dword[ 	]+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

-- 
2.43.0

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

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

I think this resolved the segfault when dumping partial data which size
between 4 and 8.  Before this series, we used to dump "out of bounds
address", so after adding this last piece of the puzzle will be perfect for
now.  The only thing I can imagine is that we probably need to handle the
dump_size between 8 and 16 if supporting rv128, but we can handle that
later at that time.

Hi Nick, Hi Alan, Hi Jan,

I think we should also backport this to 2.44 branch, is that good?

Thanks a lot
Nelson


On Wed, Feb 12, 2025 at 1:08 PM Charlie Jenkins <charlie@rivosinc.com>
wrote:

> Commit 6a04e8230707 ("RISC-V: Fix display of partial instructions")
> changed how objdump displays instructions. This had the unintented
> side-effect that it allowed 6 byte instructions to be displayed which
> happens when a .dword is attempted to be split into instructions. Add
> support for 5, 6, and 7 byte instructions to remedy this
> issue.
>
> Signed-off-by: Charlie Jenkins <charlie@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 ++
>  opcodes/riscv-dis.c                              | 29
> +++++++++++++++++++++++-
>  3 files changed, 42 insertions(+), 1 deletion(-)
> ---
> base-commit: 815d9a14cbbb3b81843f7566222c87fb22e7255d
> change-id: 20250211-fix_gas_abort-6d1e28b4ad46
> --
> - Charlie
>
>

[-- Attachment #2: Type: text/html, Size: 2098 bytes --]