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

[Daniel Henrique Barboza <danielhb413@gmail.com>]

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