target/arm/tcg/translate-a64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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 <hdijk@accesssoftek.com>
---
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;
}
--
2.47.3
On Fri, 28 Nov 2025 at 04:38, Harald van Dijk <hdijk@accesssoftek.com> 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 <hdijk@accesssoftek.com>
> ---
> 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: qemu-stable@nongnu.org
Fixes: 64678fc45d8f6 ("target/arm: Fix BTI versus CF_PCREL")
and you can have
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
© 2016 - 2025 Red Hat, Inc.