[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set |
Date: |
Mon, 6 Jul 2015 23:24:16 +0100 |
On 6 July 2015 at 19:09, <address@hidden> wrote:
> From: Meador Inge <address@hidden>
>
> This small patch adds a sanity check when disassembling
> the BLX instruction. The use case came to light when
> doing toolchain development and a similar check was
> upstreamed for Binutils:
>
> * https://sourceware.org/ml/binutils/2011-01/msg00077.html
>
> Patch by Nathan Sidwell.
>
> Signed-off-by: Meador Inge <address@hidden>
> ---
> target-arm/translate.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 69ac18c..fedc8f3 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9912,6 +9912,12 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> gen_jmp(s, offset);
> } else {
> /* blx */
> + /* The instruction must have bit zero unset, even
> + though it is part of the offset. Real hardware
> + will abort, so we do too. */
This comment isn't really correct -- bit zero in this encoding
(BLX T2) isn't part of the immediate, it's described
as a one bit field H which causes UNDEFINED if it's 1.
This is architecturally mandated, not just somewhere
we're following h/w on an IMPDEF or UNPREDICTABLE case.
> + if (insn & 1) {
> + goto illegal_op;
> + }
This check is happening too late -- we've already
done the write to R14. We need to UNDEF before that.
> offset &= ~(uint32_t)2;
The new check makes this masking unnecessary now, right?
> /* thumb2 bx, no need to check */
> gen_bx_im(s, offset);
> --
thanks
-- PMM