qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Alistair Francis <alistair.francis@wdc.com>
Cc: Alistair Francis <alistair23@gmail.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	LIU Zhiwei <zhiwei_liu@c-sky.com>
Subject: Re: [PULL v2 12/64] target/riscv: add vector amo operations
Date: Sun, 5 Jul 2020 19:20:11 +0100	[thread overview]
Message-ID: <CAFEAcA8b7Rk8ZDDu689tRDCqcSKRbCGVf=75e0f0WNd2GJ9dmg@mail.gmail.com> (raw)
In-Reply-To: <20200702162354.928528-13-alistair.francis@wdc.com>

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	other threads:[~2020-07-05 18:21 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-02 16:22 [PULL v2 00/64] riscv-to-apply queue Alistair Francis
2020-07-02 16:22 ` [PULL v2 01/64] riscv: plic: Honour source priorities Alistair Francis
2020-07-02 16:22 ` [PULL v2 02/64] riscv: plic: Add a couple of mising sifive_plic_update calls Alistair Francis
2020-07-02 16:22 ` [PULL v2 03/64] hw/riscv: Allow 64 bit access to SiFive CLINT Alistair Francis
2020-07-02 16:22 ` [PULL v2 04/64] target/riscv: add vector extension field in CPURISCVState Alistair Francis
2020-07-02 16:22 ` [PULL v2 05/64] target/riscv: implementation-defined constant parameters Alistair Francis
2020-07-02 16:22 ` [PULL v2 06/64] target/riscv: support vector extension csr Alistair Francis
2020-07-02 16:22 ` [PULL v2 07/64] target/riscv: add vector configure instruction Alistair Francis
2020-07-02 16:22 ` [PULL v2 08/64] target/riscv: add an internals.h header Alistair Francis
2020-07-02 16:22 ` [PULL v2 09/64] target/riscv: add vector stride load and store instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 10/64] target/riscv: add vector index " Alistair Francis
2020-07-02 16:23 ` [PULL v2 11/64] target/riscv: add fault-only-first unit stride load Alistair Francis
2020-07-02 16:23 ` [PULL v2 12/64] target/riscv: add vector amo operations Alistair Francis
2020-07-05 18:20   ` Peter Maydell [this message]
2020-07-06 20:48     ` Richard Henderson
2020-07-07 14:26       ` LIU Zhiwei
2020-07-07 14:33         ` Richard Henderson
2020-07-06 23:36     ` Alistair Francis
2020-07-07  2:35       ` LIU Zhiwei
2020-07-02 16:23 ` [PULL v2 13/64] target/riscv: vector single-width integer add and subtract Alistair Francis
2020-07-02 16:23 ` [PULL v2 14/64] target/riscv: vector widening " Alistair Francis
2020-07-02 16:23 ` [PULL v2 15/64] target/riscv: vector integer add-with-carry / subtract-with-borrow instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 16/64] target/riscv: vector bitwise logical instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 17/64] target/riscv: vector single-width bit shift instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 18/64] target/riscv: vector narrowing integer right " Alistair Francis
2020-07-02 16:23 ` [PULL v2 19/64] target/riscv: vector integer comparison instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 20/64] target/riscv: vector integer min/max instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 21/64] target/riscv: vector single-width integer multiply instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 22/64] target/riscv: vector integer divide instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 23/64] target/riscv: vector widening integer multiply instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 24/64] target/riscv: vector single-width integer multiply-add instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 25/64] target/riscv: vector widening " Alistair Francis
2020-07-02 16:23 ` [PULL v2 26/64] target/riscv: vector integer merge and move instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 27/64] target/riscv: vector single-width saturating add and subtract Alistair Francis
2020-07-02 16:23 ` [PULL v2 28/64] target/riscv: vector single-width averaging " Alistair Francis
2020-07-02 16:23 ` [PULL v2 29/64] target/riscv: vector single-width fractional multiply with rounding and saturation Alistair Francis
2020-07-02 16:23 ` [PULL v2 30/64] target/riscv: vector widening saturating scaled multiply-add Alistair Francis
2020-07-02 16:23 ` [PULL v2 31/64] target/riscv: vector single-width scaling shift instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 32/64] target/riscv: vector narrowing fixed-point clip instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 33/64] target/riscv: vector single-width floating-point add/subtract instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 34/64] target/riscv: vector widening " Alistair Francis
2020-07-02 16:23 ` [PULL v2 35/64] target/riscv: vector single-width floating-point multiply/divide instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 36/64] target/riscv: vector widening floating-point multiply Alistair Francis
2020-07-02 16:23 ` [PULL v2 37/64] target/riscv: vector single-width floating-point fused multiply-add instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 38/64] target/riscv: vector widening " Alistair Francis
2020-07-02 16:23 ` [PULL v2 39/64] target/riscv: vector floating-point square-root instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 40/64] target/riscv: vector floating-point min/max instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 41/64] target/riscv: vector floating-point sign-injection instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 42/64] target/riscv: vector floating-point compare instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 43/64] target/riscv: vector floating-point classify instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 44/64] target/riscv: vector floating-point merge instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 45/64] target/riscv: vector floating-point/integer type-convert instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 46/64] target/riscv: widening " Alistair Francis
2020-07-02 16:23 ` [PULL v2 47/64] target/riscv: narrowing " Alistair Francis
2020-07-02 16:23 ` [PULL v2 48/64] target/riscv: vector single-width integer reduction instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 49/64] target/riscv: vector wideing " Alistair Francis
2020-07-02 16:23 ` [PULL v2 50/64] target/riscv: vector single-width floating-point " Alistair Francis
2020-07-02 16:23 ` [PULL v2 51/64] target/riscv: vector widening " Alistair Francis
2020-07-02 16:23 ` [PULL v2 52/64] target/riscv: vector mask-register logical instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 53/64] target/riscv: vector mask population count vmpopc Alistair Francis
2020-07-02 16:23 ` [PULL v2 54/64] target/riscv: vmfirst find-first-set mask bit Alistair Francis
2020-07-02 16:23 ` [PULL v2 55/64] target/riscv: set-X-first " Alistair Francis
2020-07-02 16:23 ` [PULL v2 56/64] target/riscv: vector iota instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 57/64] target/riscv: vector element index instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 58/64] target/riscv: integer extract instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 59/64] target/riscv: integer scalar move instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 60/64] target/riscv: floating-point scalar move instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 61/64] target/riscv: vector slide instructions Alistair Francis
2020-07-02 16:23 ` [PULL v2 62/64] target/riscv: vector register gather instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 63/64] target/riscv: vector compress instruction Alistair Francis
2020-07-02 16:23 ` [PULL v2 64/64] target/riscv: configure and turn on vector extension from command line Alistair Francis
2020-07-02 17:27 ` [PULL v2 00/64] riscv-to-apply queue no-reply
2020-07-03 16:55 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFEAcA8b7Rk8ZDDu689tRDCqcSKRbCGVf=75e0f0WNd2GJ9dmg@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=alistair.francis@wdc.com \
    --cc=alistair23@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=zhiwei_liu@c-sky.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).