Re: [PATCH v2 1/1] target/ppc: fix unreachable code in do_ldst_quad()

[Daniel Henrique Barboza]

On 8/4/22 15:05, Peter Maydell wrote:
> On Mon, 25 Jul 2022 at 21:24, Daniel Henrique Barboza
> <danielhb413@gmail.com> wrote:
> > Coverity reports that commit fc34e81acd51 ("target/ppc: add macros to
> > check privilege level") turned the following code unreachable:
> >
> > if (!prefixed && !(ctx->insns_flags2 & PPC2_LSQ_ISA207)) {
> >      /* lq and stq were privileged prior to V. 2.07 */
> >      REQUIRE_SV(ctx);
> >
> > > > >      CID 1490757:  Control flow issues  (UNREACHABLE)
> > > > >      This code cannot be reached: "if (ctx->le_mode) {
> >      if (ctx->le_mode) {
> >          gen_align_no_le(ctx);
> >          return true;
> >      }
> > }
> >
> > This happens because the macro REQUIRE_SV(), in CONFIG_USER_MODE, will
> > always result in a 'return true' statement. In fact, all REQUIRE_*
> > macros for target/ppc/translate.c behave the same way: if a condition
> > isn't met, an exception is generated and a 'return' statement is issued.
> >
> > The difference is that all other callers are using it in insns that are
> > not implemented in user mode. do_ldst_quad(), on the other hand, is user
> > mode compatible.
>
> This is a Coverity false positive, and I'd already marked it that way
> in the Coverity UI back on the 20th. Coverity gets confused sometimes
> by ifdeffery.
>
> So you don't need this patch, unless you think the code is genuinely
> better (more readable to humans, etc) this way.

The idea was to make Coverity happy. If Coverity is already happy then
let's drop this patch - there's no particular improvement made here that
justifies it.


Thanks,

Daniel

> thanks
> -- PMM