Patchwork target-arm: Reinsert SRS missing return statements.

login
register
mail settings
Submitter Peter Chubb
Date April 15, 2013, 4:50 a.m.
Message ID <84sj2s75yv.wl%peter@chubb.wattle.id.au>
Download mbox | patch
Permalink /patch/236493/
State New
Headers show

Comments

Peter Chubb - April 15, 2013, 4:50 a.m.
Since patch
   81465888c5306cd94abb9847e560796fd13d3c2f
   target-arm: factor out handling of SRS instruction
the SRS instruction has not worked in QEMU.

The problem is a return directive that was removed in the
refactoring, so after decoding the instruction, qemu would fall
through to do stuff that it should not have done.

Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>
Peter Maydell - April 15, 2013, 7:23 a.m.
On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au> wrote:
>
>
> Since patch
>    81465888c5306cd94abb9847e560796fd13d3c2f
>    target-arm: factor out handling of SRS instruction
> the SRS instruction has not worked in QEMU.
>
> The problem is a return directive that was removed in the
> refactoring, so after decoding the instruction, qemu would fall
> through to do stuff that it should not have done.

Nice catch for the ARM decoder, but not needed for thumb2
I think?

thanks
-- PMM
Peter Chubb - April 15, 2013, 7:29 a.m.
>>>>> "Peter" == Peter Maydell <peter.maydell@linaro.org> writes:

Peter> On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au>
Peter> wrote:
>> 
>> 
>> Since patch 81465888c5306cd94abb9847e560796fd13d3c2f target-arm:
>> factor out handling of SRS instruction the SRS instruction has not
>> worked in QEMU.
>> 
>> The problem is a return directive that was removed in the
>> refactoring, so after decoding the instruction, qemu would fall
>> through to do stuff that it should not have done.

Peter> Nice catch for the ARM decoder, but not needed for thumb2 I
Peter> think?

It was there in the code that was removed.
I didn't analyse too deeply, as nothing we do uses the thumb version.

Peter C
--
Dr Peter Chubb				        peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au          Software Systems Research Group/NICTA
Peter Maydell - April 15, 2013, 7:32 a.m.
On 15 April 2013 08:29, Peter Chubb <peter.chubb@nicta.com.au> wrote:
>>>>>> "Peter" == Peter Maydell <peter.maydell@linaro.org> writes:
> Peter> On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au>
> Peter> wrote:
>>> The problem is a return directive that was removed in the
>>> refactoring, so after decoding the instruction, qemu would fall
>>> through to do stuff that it should not have done.
>
> Peter> Nice catch for the ARM decoder, but not needed for thumb2 I
> Peter> think?
>
> It was there in the code that was removed.

No it wasn't, I just looked at the broken commit which is why I
pointed it out. (Also you can see that the gen_rfe() above isn't
followed by a return, so I checked code flow to be sure it was OK.)

thanks
-- PMM

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 35a21be..c870246 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6762,6 +6762,7 @@  static void disas_arm_insn(CPUARMState * env, DisasContext *s)
             }
             ARCH(6);
             gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
+            return;
         } else if ((insn & 0x0e50ffe0) == 0x08100a00) {
             /* rfe */
             int32_t offset;
@@ -8209,6 +8210,7 @@  static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                     /* srs */
                     gen_srs(s, (insn & 0x1f), (insn & (1 << 24)) ? 1 : 2,
                             insn & (1 << 21));
+                    return;
                 }
             } else {
                 int i, loaded_base = 0;