On Wed, Jan 22, 2025 at 4:50 AM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
> Coverity found a second DEADCODE issue in rmw_xireg() claiming that we can't
> reach 'RISCV_EXCP_NONE' at the 'done' label:
>
>  > 2706     done:
>  > 2707         if (ret) {
>  > 2708             return (env->virt_enabled && virt) ?
>  > 2709                    RISCV_EXCP_VIRT_INSTRUCTION_FAULT : 
> RISCV_EXCP_ILLEGAL_INST;
>  > 2710         }
>  >>>>      CID 1590356:  Control flow issues  (DEADCODE)
>  >>>>      Execution cannot reach this statement: "return RISCV_EXCP_NONE;".
>  > 2711         return RISCV_EXCP_NONE;
>
> Our label is now reduced after fixing another deadcode in the previous
> patch but the problem reported here still remains:
>
>  done:
>     if (ret) {
>         return RISCV_EXCP_ILLEGAL_INST;
>     }
>     return RISCV_EXCP_NONE;
>
> This happens because 'ret' changes only once at the start of the
> function:
>
>     ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT);
>     if (ret != RISCV_EXCP_NONE) {
>         return ret;
>     }
>
> So it's a guarantee that ret will be RISCV_EXCP_NONE (-1) if we ever
> reach the label, i.e. "if (ret)" will always be true, and  the label can
> be even further reduced to:
>
> done:
>     return RISCV_EXCP_ILLEGAL_INST;
>
> To make a better use of the label, remove the 'else' from the
> xiselect_aia_range() chain and let it fall-through to the 'done' label
> since they are now both returning RISCV_EXCP_ILLEGAL_INST.
>
> Resolves: Coverity CID 1590356
> Fixes: dc0280723d ("target/riscv: Decouple AIA processing from xiselect and 
> xireg")
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>

Alistair

> ---
>  target/riscv/csr.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ab209d0cda..0e83c3b045 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2697,15 +2697,10 @@ static RISCVException rmw_xireg(CPURISCVState *env, 
> int csrno,
>      } else if (riscv_cpu_cfg(env)->ext_smcsrind ||
>                 riscv_cpu_cfg(env)->ext_sscsrind) {
>          return rmw_xireg_csrind(env, csrno, isel, val, new_val, wr_mask);
> -    } else {
> -        return RISCV_EXCP_ILLEGAL_INST;
>      }
>
>  done:
> -    if (ret) {
> -        return RISCV_EXCP_ILLEGAL_INST;
> -    }
> -    return RISCV_EXCP_NONE;
> +    return RISCV_EXCP_ILLEGAL_INST;
>  }
>
>  static RISCVException rmw_xtopei(CPURISCVState *env, int csrno,
> --
> 2.47.1
>
>

Reply via email to