Re: [PATCH] RISC-V: Adjust the 'print_insn' return value of disassembling.

Re: [PATCH] RISC-V: Adjust the 'print_insn' return value of disassembling.

[Xiaolin Zheng]

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

Hi Jiawei,
Thanks for fixing this issue! I didn't subscribe our mailing list so I add a "Re:" in the title and expect this mail could append to our thread of discussion.
Seems to me this patch has not yet been committed, so we have time to discuss.
My question is simple:
There seem to be two [1] [2] `memory_error_func` used in `print_insn_riscv`. Seems the first one is an opcode pre-fetch and the other one is the real one. So, should the other one be fixed as well? ;-)
[1] https://github.com/bminor/binutils-gdb/blob/9e4b2a6697006f9ff39bce13edd640330e208c9f/opcodes/riscv-dis.c#L1061 <https://github.com/bminor/binutils-gdb/blob/9e4b2a6697006f9ff39bce13edd640330e208c9f/opcodes/riscv-dis.c#L1061 >
[2] https://github.com/bminor/binutils-gdb/blob/9e4b2a6697006f9ff39bce13edd640330e208c9f/opcodes/riscv-dis.c#L1073 <https://github.com/bminor/binutils-gdb/blob/9e4b2a6697006f9ff39bce13edd640330e208c9f/opcodes/riscv-dis.c#L1073 >
Best Regards,
Xiaolin

Re: [PATCH] RISC-V: Adjust the 'print_insn' return value of disassembling.

[Nelson Chu]

OK, looks reasonable and good, please commit.

Thanks
Nelson

On Fri, Mar 17, 2023 at 9:54 AM Jiawei <jiawei@iscas.ac.cn> wrote:
>
> This bug reported in
> https://sourceware.org/bugzilla/show_bug.cgi?id=30184
> And discussed in
> https://sourceware.org/pipermail/binutils/2023-February/126213.html
>
> We also checked the implementation of return value in arm and mips.
> So this patch changes the return value to -1, that can fix bugs and maintain
> consistency with other architectures.
>
> opcodes/ChangeLog:
>
>         * riscv-dis.c (print_insn_riscv):Change the return value.
>
> ---
>  opcodes/riscv-dis.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 7baba054daa..1506dc5750b 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -1059,7 +1059,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        if (status != 0)
>         {
>           (*info->memory_error_func) (status, memaddr, info);
> -         return status;
> +         return -1;
>         }
>        insn = (insn_t) bfd_getl16 (packet);
>        dump_size = riscv_insn_length (insn);
> --
> 2.25.1
>

[PATCH] RISC-V: Adjust the 'print_insn' return value of disassembling.

[Jiawei]

This bug reported in
https://sourceware.org/bugzilla/show_bug.cgi?id=30184
And discussed in
https://sourceware.org/pipermail/binutils/2023-February/126213.html

We also checked the implementation of return value in arm and mips.
So this patch changes the return value to -1, that can fix bugs and maintain
consistency with other architectures.

opcodes/ChangeLog:

        * riscv-dis.c (print_insn_riscv):Change the return value.

---
 opcodes/riscv-dis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 7baba054daa..1506dc5750b 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1059,7 +1059,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       if (status != 0)
 	{
 	  (*info->memory_error_func) (status, memaddr, info);
-	  return status;
+	  return -1;
 	}
       insn = (insn_t) bfd_getl16 (packet);
       dump_size = riscv_insn_length (insn);
-- 
2.25.1