On 7/5/20 11:20 AM, Peter Maydell wrote:
> On Thu, 2 Jul 2020 at 17:33, Alistair Francis <alistair.fran...@wdc.com>
> wrote:
>>
>> From: LIU Zhiwei <zhiwei_...@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.
FWIW, I think it would be better to have the gen_helper_amo *fn symbol here in
the macro than a magic "seq" number.
r~
