Patchwork target-arm: fix LDMIA bug on page boundary

login
register
mail settings
Submitter 오유연
Date April 25, 2011, 1:23 a.m.
Message ID <18082259.13471303694638277.JavaMail.weblogic@epv6ml05>
Download mbox | patch
Permalink /patch/92689/
State New
Headers show

Comments

오유연 - April 25, 2011, 1:23 a.m.
target-arm: fix LDMIA bug on page boundary

When consecutive memory locations are on page boundary, a base register may be
loaded before page fault occurs. After page fault handling, it losts the memory
location information. To solve this problem, loading a base register has to put back.

Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>
---
 target-arm/translate.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
Peter Maydell - April 26, 2011, 5:17 p.m.
On 25 April 2011 02:23, YuYeon Oh <yuyeon.oh@samsung.com> wrote:
> target-arm: fix LDMIA bug on page boundary

(You don't need to repeat the Subject summary line in the body, it makes the
git changelog look a bit odd when the patch is applied with 'git am').

> When consecutive memory locations are on page boundary, a base register may be
> loaded before page fault occurs. After page fault handling, it losts the memory
> location information. To solve this problem, loading a base register has to put back.
>
> Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I've tested this and confirmed that it fixes this bug for the
Thumb T2 encoding. However, the same problem still exists for the
T1 (16 bit) encoding; I'll send a patch for that in a moment.
(The ARM encoding did not have this bug.)

-- PMM
Aurelien Jarno - April 27, 2011, 6:18 p.m.
On Mon, Apr 25, 2011 at 01:23:58AM +0000, YuYeon Oh wrote:
> target-arm: fix LDMIA bug on page boundary
> 
> When consecutive memory locations are on page boundary, a base register may be
> loaded before page fault occurs. After page fault handling, it losts the memory
> location information. To solve this problem, loading a base register has to put back.
> 
> Signed-off-by: Yuyeon Oh <yuyeon.oh@samsung.com>
> ---
>  target-arm/translate.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index e1bda57..410e7c4 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7967,7 +7967,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                      }
>                  }
>              } else {
> -                int i;
> +                int i, loaded_base = 0;
> +                TCGv loaded_var;
>                  /* Load/store multiple.  */
>                  addr = load_reg(s, rn);
>                  offset = 0;
> @@ -7979,6 +7980,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                      tcg_gen_addi_i32(addr, addr, -offset);
>                  }
>  
> +                TCGV_UNUSED(loaded_var);
>                  for (i = 0; i < 16; i++) {
>                      if ((insn & (1 << i)) == 0)
>                          continue;
> @@ -7987,6 +7989,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                          tmp = gen_ld32(addr, IS_USER(s));
>                          if (i == 15) {
>                              gen_bx(s, tmp);
> +                        } else if (i == rn) {
> +                            loaded_var = tmp;
> +                            loaded_base = 1;
>                          } else {
>                              store_reg(s, i, tmp);
>                          }
> @@ -7997,6 +8002,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                      }
>                      tcg_gen_addi_i32(addr, addr, 4);
>                  }
> +                if (loaded_base) {
> +                    store_reg(s, rn, loaded_var);
> +                }
>                  if (insn & (1 << 21)) {
>                      /* Base register writeback.  */
>                      if (insn & (1 << 24)) {
> -- 
> 1.7.4.msysgit.0

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index e1bda57..410e7c4 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7967,7 +7967,8 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                     }
                 }
             } else {
-                int i;
+                int i, loaded_base = 0;
+                TCGv loaded_var;
                 /* Load/store multiple.  */
                 addr = load_reg(s, rn);
                 offset = 0;
@@ -7979,6 +7980,7 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                     tcg_gen_addi_i32(addr, addr, -offset);
                 }
 
+                TCGV_UNUSED(loaded_var);
                 for (i = 0; i < 16; i++) {
                     if ((insn & (1 << i)) == 0)
                         continue;
@@ -7987,6 +7989,9 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                         tmp = gen_ld32(addr, IS_USER(s));
                         if (i == 15) {
                             gen_bx(s, tmp);
+                        } else if (i == rn) {
+                            loaded_var = tmp;
+                            loaded_base = 1;
                         } else {
                             store_reg(s, i, tmp);
                         }
@@ -7997,6 +8002,9 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                     }
                     tcg_gen_addi_i32(addr, addr, 4);
                 }
+                if (loaded_base) {
+                    store_reg(s, rn, loaded_var);
+                }
                 if (insn & (1 << 21)) {
                     /* Base register writeback.  */
                     if (insn & (1 << 24)) {