On Sat, 30 Aug 2025 at 18:04, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/arm/tcg/translate-a64.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index 8230ac0fab..20e18687d5 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -1842,20 +1842,20 @@ static bool trans_BRAZ(DisasContext *s, arg_braz *a)
>
> static bool trans_BLRAZ(DisasContext *s, arg_braz *a)
> {
> - TCGv_i64 dst, lr;
> + TCGv_i64 dst, link;
>
> if (!dc_isar_feature(aa64_pauth, s)) {
> return false;
> }
> -
> dst = auth_branch_target(s, cpu_reg(s, a->rn), tcg_constant_i64(0),
> !a->m);
auth_branch_target() can return its 'dst' argument
directly if pauth is disabled...
> - lr = cpu_reg(s, 30);
> - if (dst == lr) {
...which is why we had this check for dst and lr being
the same thing. Now we don't, but...
> - TCGv_i64 tmp = tcg_temp_new_i64();
> - tcg_gen_mov_i64(tmp, dst);
> - dst = tmp;
> +
> + link = tcg_temp_new_i64();
> + gen_pc_plus_diff(s, link, 4);
> + if (s->gcs_en) {
> + gen_add_gcs_record(s, link);
> }
> - gen_pc_plus_diff(s, lr, curr_insn_len(s));
> + tcg_gen_mov_i64(cpu_reg(s, 30), link);
...here we update X30 with the link pointer before we
set the PC from dst, so if Xn is X30 and pauth is
disabled we'll jump to the wrong target, I think.
> +
> gen_a64_set_pc(s, dst);
We could fix this by (like trans_BLR in the previous patch)
writing the new PC before we write link to X30.
> set_btype_for_blr(s);
> s->base.is_jmp = DISAS_JUMP;
> @@ -1892,19 +1892,20 @@ static bool trans_BRA(DisasContext *s, arg_bra *a)
>
> static bool trans_BLRA(DisasContext *s, arg_bra *a)
> {
> - TCGv_i64 dst, lr;
> + TCGv_i64 dst, link;
>
> if (!dc_isar_feature(aa64_pauth, s)) {
> return false;
> }
> dst = auth_branch_target(s, cpu_reg(s, a->rn), cpu_reg_sp(s, a->rm),
> !a->m);
> - lr = cpu_reg(s, 30);
> - if (dst == lr) {
> - TCGv_i64 tmp = tcg_temp_new_i64();
> - tcg_gen_mov_i64(tmp, dst);
> - dst = tmp;
> +
> + link = tcg_temp_new_i64();
> + gen_pc_plus_diff(s, link, 4);
> + if (s->gcs_en) {
> + gen_add_gcs_record(s, link);
> }
> - gen_pc_plus_diff(s, lr, curr_insn_len(s));
> + tcg_gen_mov_i64(cpu_reg(s, 30), link);
> +
> gen_a64_set_pc(s, dst);
Similarly here.
> set_btype_for_blr(s);
> s->base.is_jmp = DISAS_JUMP;
> --
thanks
-- PMM
