Re: [PATCH 3/5] target/riscv/csr.c: fix deadcode in rmw_xiregi()

[Alistair Francis]

On Wed, Jan 22, 2025 at 4:49 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Coverity found a DEADCODE issue in rmw_xiregi() claiming that we can't
> reach 'RISCV_EXCP_VIRT_INSTRUCTION_FAULT' at the 'done' label:
>
>  > 2652     done:
>  >>>>      CID 1590357:  Control flow issues  (DEADCODE)
>  >>>>      Execution cannot reach the expression 
> "RISCV_EXCP_VIRT_INSTRUCTION_FAULT"
>      inside this statement: "return (env->virt_enabled &...".
>  > 2653         return (env->virt_enabled && virt) ?
>  > 2654                 RISCV_EXCP_VIRT_INSTRUCTION_FAULT : 
> RISCV_EXCP_ILLEGAL_INST;
>
> This happens because 'virt' is being set to 'false' and it will remain
> as 'false' in any code path where 'done' will be called. The label can
> be safely reduced to:
>
> done:
>      return RISCV_EXCP_ILLEGAL_INST;
>
> And that will leave us with the following usage of a 'goto' skipping a
> single 'return' to do another single 'return':
>
>      } else {
>         goto done;
>      }
>
>      return rmw_xireg_csrind(env, csrno, isel, val, new_val, wr_mask);
>
> done:
>      return RISCV_EXCP_ILLEGAL_INST;
>
> Which we will eliminate it and just do 'return RISCV_EXCP_ILLEGAL_INST'
> instead.
>
> Resolves: Coverity CID 1590357
> Fixes: 5e33a20827 ("target/riscv: Support generic CSR indirect access")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/csr.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0e83c3b045..75f21ccabb 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2621,7 +2621,6 @@ static int rmw_xireg_csrind(CPURISCVState *env, int 
> csrno,
>  static int rmw_xiregi(CPURISCVState *env, int csrno, target_ulong *val,
>                        target_ulong new_val, target_ulong wr_mask)
>  {
> -    bool virt = false;
>      int ret = -EINVAL;
>      target_ulong isel;
>
> @@ -2642,16 +2641,11 @@ static int rmw_xiregi(CPURISCVState *env, int csrno, 
> target_ulong *val,
>      } else if (CSR_VSIREG <= csrno && csrno <= CSR_VSIREG6 &&
>                 csrno != CSR_VSIREG4 - 1) {
>          isel = env->vsiselect;
> -        virt = true;
>      } else {
> -        goto done;
> +        return RISCV_EXCP_ILLEGAL_INST;
>      }
>
>      return rmw_xireg_csrind(env, csrno, isel, val, new_val, wr_mask);
> -
> -done:
> -    return (env->virt_enabled && virt) ?
> -            RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
>  }
>
>  static RISCVException rmw_xireg(CPURISCVState *env, int csrno,
> --
> 2.47.1
>
>