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
<danielhb...@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