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

[David Gibson]

On Mon, Nov 19, 2018 at 04:39:29PM +0000, Peter Maydell wrote:
> 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.)

I tend to agree, but are you ok dealing with the whole slew of
Coverity false positives it will create if we move to that style?
AFAIK, I don't have the power to deal with it from my end.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature