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

  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.

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(-)

      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);
  

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
  

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
>
  

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
>