Re: [PATCH] target/riscv: Fix PC sync in trans_sspopchk for CFI exception handling

On 7/30/2026 3:18 PM, A-Shehab wrote:
> From: Max Chou <[email protected]>
> 
> Move gen_update_pc call before conditional logic to ensure consistent
> PC state regardless of execution path.
> 
> Previously, the host instructions generated to update the cpu_pc were
> only executed in the failure path when shadow stack validation failed.
> This created inconsistent PC synchronization.
> 
> This inconsistency caused issues in CF_PCREL mode where subsequent
> instructions calculated wrong relative offsets from stale pc_save
> values, and could lead to incorrect exception return addresses.
> 
> This fix ensures PC is always synchronized before any helper that
> might raise an exception, maintaining consistent translator state
> across all execution paths.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4118
> Signed-off-by: Max Chou <[email protected]>
> [ahshehab: rebased on current master; file moved to
>   target/riscv/tcg/insn_trans/ and the ssp load is now 64-bit wide]
> Tested-by: A-Shehab <[email protected]>
> Signed-off-by: A-Shehab <[email protected]>
> ---

Reviewed-by: Daniel Henrique Barboza <[email protected]>

> This is a repost of Max Chou's patch from 2025-11-05 [1], which did not
> receive any review. Rebased onto current master: the file moved to
> target/riscv/tcg/insn_trans/ and the ssp load is now a 64-bit load, so
> the original patch no longer applies.
> 
> I opened a GitLab issue (#4118) with a minimal, self-contained bare-metal
> reproducer for this bug. It runs an sspopchk that matches the shadow stack
> (the common case) followed by an auipc in the same translation block; on
> current master the auipc returns an address 4 bytes too low (exit 42) and
> with this patch it is correct (exit 0). The reproducer is included in the
> issue.
> 
> [1] https://lore.kernel.org/qemu-devel/[email protected]/
> 
>   target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc
> index a813232887..d47a9f9c7d 100644
> --- a/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc
> +++ b/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc
> @@ -32,6 +32,7 @@ static bool trans_sspopchk(DisasContext *ctx, arg_sspopchk *a)
>       TCGLabel *skip = gen_new_label();
>       uint32_t tmp = (get_xl(ctx) == MXL_RV64) ? 8 : 4;
>       TCGv data = tcg_temp_new();
> +    gen_update_pc(ctx, 0);
>       TCGv_i64 wide_addr = tcg_temp_new_i64();
>       tcg_gen_ld_i64(wide_addr, tcg_env, offsetof(CPURISCVState, ssp));
>       tcg_gen_trunc_i64_tl(addr, wide_addr);
> @@ -42,7 +43,6 @@ static bool trans_sspopchk(DisasContext *ctx, arg_sspopchk *a)
>       tcg_gen_brcond_tl(TCG_COND_EQ, data, rs1, skip);
>       tcg_gen_st8_i32(tcg_constant_i32(RISCV_EXCP_SW_CHECK_BCFI_TVAL),
>                       tcg_env, offsetof(CPURISCVState, sw_check_code));
> -    gen_update_pc(ctx, 0);
>       gen_helper_raise_exception(tcg_env,
>                     tcg_constant_i32(RISCV_EXCP_SW_CHECK));
>       gen_set_label(skip);