Re: [Qemu-ppc] [PULL 01/22] target/ppc: add external PID support

[Peter Maydell]

On 15 November 2018 at 10:22, Peter Maydell <address@hidden> 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