On 15 November 2018 at 10:22, Peter Maydell <peter.mayd...@linaro.org> wrote:
> Hi; Coverity reports an issue (CID1396864) with this function:
>
>> +/* dcbfep (external PID dcbf) */
>> +static void gen_dcbfep(DisasContext *ctx)
>> +{
>> + /* XXX: specification says this is treated as a load by the MMU */
>> + TCGv t0;
>> + CHK_SV;
>> + gen_set_access_type(ctx, ACCESS_CACHE);
>> + t0 = tcg_temp_new();
>> + gen_addr_reg_index(ctx, t0);
>> + tcg_gen_qemu_ld_tl(t0, t0, PPC_TLB_EPID_LOAD, DEF_MEMOP(MO_UB));
>> + tcg_temp_free(t0);
>> +}
>
> It says that the gen_set_access_type() call is unreachable. I think
> this is a false positive (the code is unreachable, but only if
> CONFIG_USER_ONLY is defined). On the other hand, all the other
> similar gen_* functions in this file that use CHK_SV seem to have
> a pattern of
>
> #if defined(CONFIG_USER_ONLY)
> GEN_PRIV;
> #else
> TCGv t0;
> CHK_SV;
> [etc]
> #endif
>
> so maybe we should do that here too for consistency?
I've marked the issue as a false-positive in Coverity (since it is,
and that's what we've done with some equivalent target/ppc Coverity
issues with other functions previously). I'll let you decide whether
you want to prefer "with ifdefs" or "without". (TBH I think the
"without" style is better here.)
thanks
-- PMM
