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

[Nelson Chu <nelson@rivosinc.com>]

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