qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL v2 12/64] target/riscv: add vector amo operations


From: Peter Maydell
Subject: Re: [PULL v2 12/64] target/riscv: add vector amo operations
Date: Sun, 5 Jul 2020 19:20:11 +0100

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.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]