On Fri, 28 Nov 2025 at 04:38, Harald van Dijk <[email protected]> wrote:
>
> trans_BRA does
>
> gen_a64_set_pc(s, dst);
> set_btype_for_br(s, a->rn);
>
> gen_a64_set_pc does
>
> s->pc_save = -1;
>
> set_btype_for_br (if aa64_bti is enabled and the register is not x16 or
> x17) does
>
> gen_pc_plus_diff(s, pc, 0);
>
> gen_pc_plus_diff does
>
> assert(s->pc_save != -1);
>
> Hence, this assert is getting hit. We need to call set_btype_for_br
> before gen_a64_set_pc, and there is nothing in set_btype_for_br that
> depends on gen_a64_set_pc having already been called, so this commit
> simply swaps the calls.
>
> Signed-off-by: Harald van Dijk <[email protected]>
> ---
> target/arm/tcg/translate-a64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index 08b21d7dbf..cde22a5cca 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -1916,8 +1916,8 @@ static bool trans_BRA(DisasContext *s, arg_bra *a)
> return false;
> }
> dst = auth_branch_target(s, cpu_reg(s,a->rn), cpu_reg_sp(s, a->rm),
> !a->m);
> - gen_a64_set_pc(s, dst);
> set_btype_for_br(s, a->rn);
> + gen_a64_set_pc(s, dst);
> s->base.is_jmp = DISAS_JUMP;
> return true;
> }
The commit message on commit 64678fc45d8f6 says
The set_btype_for_br call must be moved after the gen_a64_set_pc
call to ensure the current pc can still be computed.
but I think that is incorrect and it meant to say "moved before",
because the actual code changes it makes to trans_BR() and
trans_BRAZ() are
@@ -1521,8 +1528,8 @@ static void set_btype_for_blr(DisasContext *s)
static bool trans_BR(DisasContext *s, arg_r *a)
{
- gen_a64_set_pc(s, cpu_reg(s, a->rn));
set_btype_for_br(s, a->rn);
+ gen_a64_set_pc(s, cpu_reg(s, a->rn));
s->base.is_jmp = DISAS_JUMP;
return true;
}
@@ -1581,8 +1588,8 @@ static bool trans_BRAZ(DisasContext *s, arg_braz *a)
}
dst = auth_branch_target(s, cpu_reg(s, a->rn), tcg_constant_i64(0), !a->m);
- gen_a64_set_pc(s, dst);
set_btype_for_br(s, a->rn);
+ gen_a64_set_pc(s, dst);
s->base.is_jmp = DISAS_JUMP;
return true;
}
which move the set_btype_for_br() call to before gen_a64_set_pc().
So I think that we just forgot to also include trans_BRA() in
that change, and your patch here fixes that.
Richard, does that sound right?
If so, this should be:
Cc: [email protected]
Fixes: 64678fc45d8f6 ("target/arm: Fix BTI versus CF_PCREL")
and you can have
Reviewed-by: Peter Maydell <[email protected]>
thanks
-- PMM
