* [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set
@ 2015-07-06 18:09 meadori
2015-07-06 22:24 ` Peter Maydell
2015-09-01 16:28 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: meadori @ 2015-07-06 18:09 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Meador Inge
From: Meador Inge <meadori@codesourcery.com>
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 <meadori@codesourcery.com>
---
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. */
+ if (insn & 1) {
+ goto illegal_op;
+ }
offset &= ~(uint32_t)2;
/* thumb2 bx, no need to check */
gen_bx_im(s, offset);
--
1.8.1.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set
2015-07-06 18:09 [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set meadori
@ 2015-07-06 22:24 ` Peter Maydell
2015-09-01 16:28 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2015-07-06 22:24 UTC (permalink / raw)
To: Meador Inge; +Cc: QEMU Developers
On 6 July 2015 at 19:09, <meadori@codesourcery.com> wrote:
> From: Meador Inge <meadori@codesourcery.com>
>
> 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 <meadori@codesourcery.com>
> ---
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set
2015-07-06 18:09 [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set meadori
2015-07-06 22:24 ` Peter Maydell
@ 2015-09-01 16:28 ` Peter Maydell
2015-09-01 22:01 ` Meador Inge
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2015-09-01 16:28 UTC (permalink / raw)
To: Meador Inge; +Cc: QEMU Developers
On 6 July 2015 at 19:09, <meadori@codesourcery.com> wrote:
> From: Meador Inge <meadori@codesourcery.com>
>
> 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 <meadori@codesourcery.com>
Hi; are you planning to send a v2 of this patch?
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set
2015-09-01 16:28 ` Peter Maydell
@ 2015-09-01 22:01 ` Meador Inge
0 siblings, 0 replies; 4+ messages in thread
From: Meador Inge @ 2015-09-01 22:01 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On Tue, Sep 01, 2015 at 05:28:10PM +0100, Peter Maydell wrote:
> On 6 July 2015 at 19:09, <meadori@codesourcery.com> wrote:
> > From: Meador Inge <meadori@codesourcery.com>
> >
> > 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 <meadori@codesourcery.com>
>
> Hi; are you planning to send a v2 of this patch?
I am. Thank you for the reminder. I will send a v2 some time tomorrow.
Thanks,
-- Meador
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-01 22:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-06 18:09 [Qemu-devel] [PATCH] arm: Ensure LSB of BLX is set meadori
2015-07-06 22:24 ` Peter Maydell
2015-09-01 16:28 ` Peter Maydell
2015-09-01 22:01 ` Meador Inge
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.