Message ID | 20220720135723.1391598-1-danielhb413@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | target/ppc: fix unreachable code in do_ldst_quad() | expand |
On 20/07/2022 10:57, Daniel Henrique Barboza 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. > > Fix it by using "#if !defined(CONFIG_USER_ONLY)" to fold the code that > shouldn't be there if we're running in a non-privileged state. This is > also how the REQUIRE_SV() macro is being used in > storage-ctrl-impl.c.inc. > > Fixes: Coverity CID 1490757 > Fixes: fc34e81acd51 ("target/ppc: add macros to check privilege level") > Cc: Matheus Ferst <matheus.ferst@eldorado.org.br> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > target/ppc/translate/fixedpoint-impl.c.inc | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc > index db14d3bebc..4a32fac4f3 100644 > --- a/target/ppc/translate/fixedpoint-impl.c.inc > +++ b/target/ppc/translate/fixedpoint-impl.c.inc > @@ -82,10 +82,14 @@ static bool do_ldst_quad(DisasContext *ctx, arg_D *a, bool store, bool prefixed) > /* lq and stq were privileged prior to V. 2.07 */ > REQUIRE_SV(ctx); > > +#if !defined(CONFIG_USER_ONLY) > if (ctx->le_mode) { > gen_align_no_le(ctx); > return true; > } > +#else > + qemu_build_not_reached(); nit: I think the indentation here is off by 1 level (missing 4 spaces)? > +#endif > } > > if (!store && unlikely(a->ra == a->rt)) { > -- > 2.36.1 > > Reviewed-by: Víctor Colombo <victor.colombo@eldorado.org.br>
On 7/20/22 19:27, Daniel Henrique Barboza 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. I think adding ifdefs isn't fantastic. This isn't actually fix a bug, so we *could* just mark this as ignore in Coverity. If you wanted to clean this up, remove the implicit control flow from REQUIRE_* and turn the macros into pure predicates, so that you get if (REQUIRE_SV(ctx)) { return true; } if (ctx->le_mode) { ... } r~
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc index db14d3bebc..4a32fac4f3 100644 --- a/target/ppc/translate/fixedpoint-impl.c.inc +++ b/target/ppc/translate/fixedpoint-impl.c.inc @@ -82,10 +82,14 @@ static bool do_ldst_quad(DisasContext *ctx, arg_D *a, bool store, bool prefixed) /* lq and stq were privileged prior to V. 2.07 */ REQUIRE_SV(ctx); +#if !defined(CONFIG_USER_ONLY) if (ctx->le_mode) { gen_align_no_le(ctx); return true; } +#else + qemu_build_not_reached(); +#endif } if (!store && unlikely(a->ra == a->rt)) {