[PATCH v2] RISC-V: Fix disassemble fetch fail return value.

[PATCH v2] RISC-V: Fix disassemble fetch fail return value.

[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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 7baba054daa..f431124b423 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);
@@ -1071,7 +1071,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_get_bits (packet, dump_size * 8, false);
 
-- 
2.25.1

Re: [PATCH v2] RISC-V: Fix disassemble fetch fail return value.

[Xiaolin Zheng]

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

Thank you Jiawei. this version LGTM though I am not a Binutils developer. Do Binutils reviewers have thoughts on this new version? Thanks in advance.
------------------------------------------------------------------
From:Jiawei <jiawei@iscas.ac.cn>
Send Time:2023年3月20日(星期一) 11:35
To:binutils <binutils@sourceware.org>
Cc:nelson <nelson@rivosinc.com>; palmer <palmer@dabbelt.com>; christoph.muellner <christoph.muellner@vrull.eu>; 郑孝林(云矅) <yunyao.zxl@alibaba-inc.com>; wuwei2016 <wuwei2016@iscas.ac.cn>; Jiawei <jiawei@iscas.ac.cn>
Subject:[PATCH v2] RISC-V: Fix disassemble fetch fail return value.
This bug reported in
https://sourceware.org/bugzilla/show_bug.cgi?id=30184 <https://sourceware.org/bugzilla/show_bug.cgi?id=30184 >
And discussed in
https://sourceware.org/pipermail/binutils/2023-February/126213.html <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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 7baba054daa..f431124b423 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);
@@ -1071,7 +1071,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_get_bits (packet, dump_size * 8, false);
-- 
2.25.1

Re: Fwd: [PATCH v2] RISC-V: Fix disassemble fetch fail return value.

[Nelson Chu]

Thanks, committed.

On Tue, Mar 21, 2023 at 5:22 PM Andrew Burgess <aburgess@redhat.com> wrote:
>
> Nelson Chu <nelson@rivosinc.com> writes:
>
> > Hey gdb, Hey Andrew,
> >
> > We plan to accept this patch in binutils upstream since it will fix
> > the pr30184, but not really sure if that will cause problems for
> > risc-v gdb dis-assembler, or other stuff.  It would be great if you
> > guys also think it's safe enough to commit.
>
> This shouldn't cause any problems for GDB, so long as the returned value
> is negative then GDB will treat this as an error.
>
> Fine to commit as far as I'm concerned.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
>
> Thanks,
> Andrew
>
> >
> > Thanks
> > Nelson
> >
> > ---------- Forwarded message ---------
> > From: Xiaolin Zheng <yunyao.zxl@alibaba-inc.com>
> > Date: Mon, Mar 20, 2023 at 1:52 PM
> > Subject: Re: [PATCH v2] RISC-V: Fix disassemble fetch fail return value.
> > To: Jiawei <jiawei@iscas.ac.cn>, binutils <binutils@sourceware.org>
> > Cc: nelson <nelson@rivosinc.com>, palmer <palmer@dabbelt.com>,
> > christoph.muellner <christoph.muellner@vrull.eu>, wuwei2016
> > <wuwei2016@iscas.ac.cn>
> >
> >
> > Thank you Jiawei. this version LGTM though I am not a Binutils
> > developer. Do Binutils reviewers have thoughts on this new version?
> > Thanks in advance.
> >
> > ------------------------------------------------------------------
> > From:Jiawei <jiawei@iscas.ac.cn>
> > Send Time:2023年3月20日(星期一) 11:35
> > To:binutils <binutils@sourceware.org>
> > Cc:nelson <nelson@rivosinc.com>; palmer <palmer@dabbelt.com>;
> > christoph.muellner <christoph.muellner@vrull.eu>; 郑孝林(云矅)
> > <yunyao.zxl@alibaba-inc.com>; wuwei2016 <wuwei2016@iscas.ac.cn>;
> > Jiawei <jiawei@iscas.ac.cn>
> > Subject:[PATCH v2] RISC-V: Fix disassemble fetch fail return value.
> >
> > 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 | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> > index 7baba054daa..f431124b423 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);
> > @@ -1071,7 +1071,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_get_bits (packet, dump_size * 8, false);
> >
> > --
> > 2.25.1
>

Re: Fwd: [PATCH v2] RISC-V: Fix disassemble fetch fail return value.

[Xiaolin Zheng]

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

Thanks for the help, Jiawei and Nelson!
Marked the issue [1] as closed.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=30184 <https://sourceware.org/bugzilla/show_bug.cgi?id=30184 >
Best Regards,
Xiaolin
------------------------------------------------------------------
From:Nelson Chu <nelson@rivosinc.com>
Send Time:2023年3月21日(星期二) 17:55
To:Andrew Burgess <aburgess@redhat.com>; jiawei <jiawei@iscas.ac.cn>; 郑孝林(云矅) <yunyao.zxl@alibaba-inc.com>
Cc:gdb-patches <gdb-patches@sourceware.org>; Binutils <binutils@sourceware.org>
Subject:Re: Fwd: [PATCH v2] RISC-V: Fix disassemble fetch fail return value.
Thanks, committed.
On Tue, Mar 21, 2023 at 5:22 PM Andrew Burgess <aburgess@redhat.com> wrote:
>
> Nelson Chu <nelson@rivosinc.com> writes:
>
> > Hey gdb, Hey Andrew,
> >
> > We plan to accept this patch in binutils upstream since it will fix
> > the pr30184, but not really sure if that will cause problems for
> > risc-v gdb dis-assembler, or other stuff. It would be great if you
> > guys also think it's safe enough to commit.
>
> This shouldn't cause any problems for GDB, so long as the returned value
> is negative then GDB will treat this as an error.
>
> Fine to commit as far as I'm concerned.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
>
> Thanks,
> Andrew
>
> >
> > Thanks
> > Nelson
> >
> > ---------- Forwarded message ---------
> > From: Xiaolin Zheng <yunyao.zxl@alibaba-inc.com>
> > Date: Mon, Mar 20, 2023 at 1:52 PM
> > Subject: Re: [PATCH v2] RISC-V: Fix disassemble fetch fail return value.
> > To: Jiawei <jiawei@iscas.ac.cn>, binutils <binutils@sourceware.org>
> > Cc: nelson <nelson@rivosinc.com>, palmer <palmer@dabbelt.com>,
> > christoph.muellner <christoph.muellner@vrull.eu>, wuwei2016
> > <wuwei2016@iscas.ac.cn>
> >
> >
> > Thank you Jiawei. this version LGTM though I am not a Binutils
> > developer. Do Binutils reviewers have thoughts on this new version?
> > Thanks in advance.
> >
> > ------------------------------------------------------------------
> > From:Jiawei <jiawei@iscas.ac.cn>
> > Send Time:2023年3月20日(星期一) 11:35
> > To:binutils <binutils@sourceware.org>
> > Cc:nelson <nelson@rivosinc.com>; palmer <palmer@dabbelt.com>;
> > christoph.muellner <christoph.muellner@vrull.eu>; 郑孝林(云矅)
> > <yunyao.zxl@alibaba-inc.com>; wuwei2016 <wuwei2016@iscas.ac.cn>;
> > Jiawei <jiawei@iscas.ac.cn>
> > Subject:[PATCH v2] RISC-V: Fix disassemble fetch fail return value.
> >
> > 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 | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> > index 7baba054daa..f431124b423 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);
> > @@ -1071,7 +1071,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_get_bits (packet, dump_size * 8, false);
> >
> > --
> > 2.25.1
>