[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL v2 12/64] target/riscv: add vector amo operations
|
From: |
Alistair Francis |
|
Subject: |
Re: [PULL v2 12/64] target/riscv: add vector amo operations |
|
Date: |
Mon, 6 Jul 2020 16:36:57 -0700 |
On Sun, Jul 5, 2020 at 11:20 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 2 Jul 2020 at 17:33, Alistair Francis <alistair.francis@wdc.com>
> wrote:
> >
> > From: LIU Zhiwei <zhiwei_liu@c-sky.com>
> >
> > Vector AMOs operate as if aq and rl bits were zero on each element
> > with regard to ordering relative to other instructions in the same hart.
> > Vector AMOs provide no ordering guarantee between element operations
> > in the same vector AMO instruction
>
> Hi; Coverity thinks (probably wrongly) that there might be an array
> overflow here:
>
> > +static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
> > +{
> > + uint32_t data = 0;
> > + gen_helper_amo *fn;
> > + static gen_helper_amo *const fnsw[9] = {
>
> This is a 9-element array...
>
> > + /* no atomic operation */
> > + gen_helper_vamoswapw_v_w,
> > + gen_helper_vamoaddw_v_w,
> > + gen_helper_vamoxorw_v_w,
> > + gen_helper_vamoandw_v_w,
> > + gen_helper_vamoorw_v_w,
> > + gen_helper_vamominw_v_w,
> > + gen_helper_vamomaxw_v_w,
> > + gen_helper_vamominuw_v_w,
> > + gen_helper_vamomaxuw_v_w
> > + };
>
> > + if (tb_cflags(s->base.tb) & CF_PARALLEL) {
> > + gen_helper_exit_atomic(cpu_env);
> > + s->base.is_jmp = DISAS_NORETURN;
> > + return true;
> > + } else {
> > + if (s->sew == 3) {
> > +#ifdef TARGET_RISCV64
> > + fn = fnsd[seq];
> > +#else
> > + /* Check done in amo_check(). */
> > + g_assert_not_reached();
> > +#endif
> > + } else {
> > + fn = fnsw[seq];
>
> ...which we here index via 'seq'...
>
>
> > +#ifdef TARGET_RISCV64
> > +GEN_VEXT_TRANS(vamoswapd_v, 9, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamoaddd_v, 10, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamoxord_v, 11, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamoandd_v, 12, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamoord_v, 13, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamomind_v, 14, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamomaxd_v, 15, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamominud_v, 16, rwdvm, amo_op, amo_check)
> > +GEN_VEXT_TRANS(vamomaxud_v, 17, rwdvm, amo_op, amo_check)
> > +#endif
>
> ...which in the calls that these macros expand out to can
> be 9 or greater.
>
> If it's in fact impossible to get into that code path
> with a value of seq that's larger than the array, it
> would help Coverity if we asserted so, maybe
> assert(seq < ARRAY_SIZE(fnsw));
>
> This is CID 1430177, 1430178, 1430179, 1430180, 1430181,
> 1430182, 1430183, 1430184, 1430185, 14305186.
@ LIU Zhiwei can you please look into this and send a patch with a fix?
Alistair
>
> thanks
> -- PMM
- [PULL v2 06/64] target/riscv: support vector extension csr, (continued)
- [PULL v2 06/64] target/riscv: support vector extension csr, Alistair Francis, 2020/07/02
- [PULL v2 14/64] target/riscv: vector widening integer add and subtract, Alistair Francis, 2020/07/02
- [PULL v2 10/64] target/riscv: add vector index load and store instructions, Alistair Francis, 2020/07/02
- [PULL v2 22/64] target/riscv: vector integer divide instructions, Alistair Francis, 2020/07/02
- [PULL v2 04/64] target/riscv: add vector extension field in CPURISCVState, Alistair Francis, 2020/07/02
- [PULL v2 12/64] target/riscv: add vector amo operations, Alistair Francis, 2020/07/02
- Re: [PULL v2 12/64] target/riscv: add vector amo operations,
Alistair Francis <=
- Re: [PULL v2 12/64] target/riscv: add vector amo operations, LIU Zhiwei, 2020/07/06
[PULL v2 09/64] target/riscv: add vector stride load and store instructions, Alistair Francis, 2020/07/02
[PULL v2 17/64] target/riscv: vector single-width bit shift instructions, Alistair Francis, 2020/07/02
[PULL v2 13/64] target/riscv: vector single-width integer add and subtract, Alistair Francis, 2020/07/02
[PULL v2 15/64] target/riscv: vector integer add-with-carry / subtract-with-borrow instructions, Alistair Francis, 2020/07/02
[PULL v2 19/64] target/riscv: vector integer comparison instructions, Alistair Francis, 2020/07/02
[PULL v2 18/64] target/riscv: vector narrowing integer right shift instructions, Alistair Francis, 2020/07/02
[PULL v2 11/64] target/riscv: add fault-only-first unit stride load, Alistair Francis, 2020/07/02
[PULL v2 20/64] target/riscv: vector integer min/max instructions, Alistair Francis, 2020/07/02
[PULL v2 21/64] target/riscv: vector single-width integer multiply instructions, Alistair Francis, 2020/07/02