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