[Qemu-devel] Re: qemu-arm: wrong execution of post-indexed loads when Rm and Rd are the same register

[Lauro Ramos Venancio <lauro.venancio@indt.org.br>]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

Now sending the attachment. :)

Lauro 

On Thu, 2007-03-15 at 16:35 -0300, Lauro Ramos Venancio wrote:
> Qemu-arm is wrongly executing post-indexed loads when Rm and Rd are
> the same register. For example:
> 
> ldr r0, [r1], +r0
> 
> Current behavior:
> r0 <- [r1]
> r1 <- r1 + r0
> 
> Expected behavior:
> addr <- r1
> r1 <- r1 + r0
> r0 <- [addr]
> 
> The attached patch fixes this bug. Patched by me and Rodrigo Vivi.
> This patch was made based on qemu 0.9.
> 
> 
> Lauro Venancio

[-- Attachment #2: 00_ldr_writeback.patch --]
[-- Type: text/x-patch, Size: 3917 bytes --]

--- target-arm/op.c.orig	2007-03-09 18:40:02.000000000 -0300
+++ target-arm/op.c	2007-03-09 18:40:27.000000000 -0300
@@ -106,6 +106,11 @@ void OPPROTO op_movl_T0_T1(void)
     T0 = T1;
 }
 
+void OPPROTO op_movl_T1_T0(void)
+{
+    T1 = T0;
+}
+
 void OPPROTO op_movl_T1_im(void)
 {
     T1 = PARAM1;
--- target-arm/translate.c.orig	2007-03-09 18:40:02.000000000 -0300
+++ target-arm/translate.c	2007-03-09 18:40:32.000000000 -0300
@@ -383,23 +383,19 @@ static inline void gen_add_data_offset(D
     }
 }
 
-static inline void gen_add_datah_offset(DisasContext *s, unsigned int insn,
-                                        int extra)
+static inline void gen_add_datah_offset(DisasContext *s, unsigned int insn)
 {
     int val, rm;
     
     if (insn & (1 << 22)) {
         /* immediate */
         val = (insn & 0xf) | ((insn >> 4) & 0xf0);
-        val += extra;
         if (!(insn & (1 << 23)))
             val = -val;
         if (val != 0)
             gen_op_addl_T1_im(val);
     } else {
         /* register */
-        if (extra)
-            gen_op_addl_T1_im(extra);
         rm = (insn) & 0xf;
         gen_movl_T2_reg(s, rm);
         if (!(insn & (1 << 23)))
@@ -1534,14 +1530,17 @@ static void disas_arm_insn(CPUState * en
                     }
                 }
             } else {
-                int address_offset;
                 /* Misc load/store */
                 rn = (insn >> 16) & 0xf;
                 rd = (insn >> 12) & 0xf;
                 gen_movl_T1_reg(s, rn);
-                if (insn & (1 << 24))
-                    gen_add_datah_offset(s, insn, 0);
-                address_offset = 0;
+                gen_movl_T0_reg(s, rn);
+                gen_add_datah_offset(s, insn);
+                /* writeback */
+                if (!(insn & (1 << 24))||(insn & (1 << 21)))
+                  gen_movl_reg_T1(s, rn);
+                if (!(insn & (1 << 24))) /* pos-indexed */
+                  gen_op_movl_T1_T0();
                 if (insn & (1 << 20)) {
                     /* load */
                     switch(sh) {
@@ -1574,20 +1573,11 @@ static void disas_arm_insn(CPUState * en
                         gen_ldst(ldl, s);
                         gen_movl_reg_T0(s, rd + 1);
                     }
-                    address_offset = -4;
                 } else {
                     /* store */
                     gen_movl_T0_reg(s, rd);
                     gen_ldst(stw, s);
                 }
-                if (!(insn & (1 << 24))) {
-                    gen_add_datah_offset(s, insn, address_offset);
-                    gen_movl_reg_T1(s, rn);
-                } else if (insn & (1 << 21)) {
-                    if (address_offset)
-                        gen_op_addl_T1_im(address_offset);
-                    gen_movl_reg_T1(s, rn);
-                }
             }
             break;
         case 0x4:
@@ -1607,9 +1597,14 @@ static void disas_arm_insn(CPUState * en
             rn = (insn >> 16) & 0xf;
             rd = (insn >> 12) & 0xf;
             gen_movl_T1_reg(s, rn);
+	    gen_movl_T0_reg(s, rn);
             i = (IS_USER(s) || (insn & 0x01200000) == 0x00200000);
-            if (insn & (1 << 24))
                 gen_add_data_offset(s, insn);
+	    /* writeback */
+	    if (!(insn & (1 << 24))||(insn & (1 << 21)))
+	      gen_movl_reg_T1(s, rn);
+	    if (!(insn & (1 << 24))) /* pos-indexed */
+	      gen_op_movl_T1_T0();
             if (insn & (1 << 20)) {
                 /* load */
 #if defined(CONFIG_USER_ONLY)
@@ -1656,12 +1651,6 @@ static void disas_arm_insn(CPUState * en
                 }
 #endif
             }
-            if (!(insn & (1 << 24))) {
-                gen_add_data_offset(s, insn);
-                gen_movl_reg_T1(s, rn);
-            } else if (insn & (1 << 21))
-                gen_movl_reg_T1(s, rn); {
-            }
             break;
         case 0x08:
         case 0x09: