diff mbox series

[1/2] target/riscv: Quiet Coverity complains about vamo*

Message ID 20200721133742.2298-1-zhiwei_liu@c-sky.com
State New
Headers show
Series [1/2] target/riscv: Quiet Coverity complains about vamo* | expand

Commit Message

LIU Zhiwei July 21, 2020, 1:37 p.m. UTC 
Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/insn_trans/trans_rvv.inc.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Alistair Francis July 21, 2020, 3:07 p.m. UTC | #1 
On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
> index c0b7375927..7b4752b911 100644
> --- a/target/riscv/insn_trans/trans_rvv.inc.c
> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
> @@ -733,6 +733,7 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
>              g_assert_not_reached();
>  #endif
>          } else {
> +            assert(seq < ARRAY_SIZE(fnsw));
>              fn = fnsw[seq];
>          }
>      }
> --
> 2.23.0
>
>
Peter Maydell July 21, 2020, 3:30 p.m. UTC | #2 
On Tue, 21 Jul 2020 at 16:19, Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> >
> > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Interestingly Coverity's latest scan has decided all these
issues are 'fixed', even without the assert. I guess that the
online version of Coverity has had an improvement to its
checking and so is now able to figure out that it's not going
to overrun the array? Still I think the assert is worth having.

thanks
-- PMM
Alistair Francis July 22, 2020, 4:40 p.m. UTC | #3 
On Tue, Jul 21, 2020 at 8:30 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 21 Jul 2020 at 16:19, Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> > >
> > > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> >
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Interestingly Coverity's latest scan has decided all these
> issues are 'fixed', even without the assert. I guess that the
> online version of Coverity has had an improvement to its
> checking and so is now able to figure out that it's not going
> to overrun the array? Still I think the assert is worth having.

Strange.

I'll still apply these two patches.

Alistair

>
> thanks
> -- PMM
diff mbox series

Patch

diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
index c0b7375927..7b4752b911 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -733,6 +733,7 @@  static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
             g_assert_not_reached();
 #endif
         } else {
+            assert(seq < ARRAY_SIZE(fnsw));
             fn = fnsw[seq];
         }
     }