From 3af4f6ae1fa6e06de1284fa1143cb8a485ac6437 Mon Sep 17 00:00:00 2001
From: David 'Digit' Turner <digit@google.com>
Date: Fri, 9 Oct 2009 11:37:14 -0700
Subject: Fix upstream ARM emulation bug that broke singlestep mode.

This fixes a really bad bug in the Thumb/Thumb2 ARM emulation related to
conditional instructions execution. The issue was that the previous
implementation did break badly if a page fault occured during the conditional
instruction's emulation. Giving an example if probably the best way to demonstrate
this. Consider the following two instructions:

    itt eq
    streq r0,[r4, #0]

These two instructions mean, respectively:
  - If the Z flag is set, execute the next instruction. Otherwise ignore it
  - Store the value of r0 at the address pointed to by r4

In single-step mode (used when debugging the emulator), each instruction is
separately JIT-ed and executed in a different pass. The 'condexec_bits' field
of the CPU state if used to store flags corresponding to the conditional
execution of up to 4 next instructions.

When the first instruction is executed, it simply sets 'condexec_bits' to a
specific value (4).

When the second instruction is executed, things get slightly bit more funky
because what happened was the following:

 - the JIT-ed code started by clearing the 'condexec_bits' right at the
   start of its sequence (a comment says "to avoid complications trying to
   do it at the end of the block", famous last words...)

 - a conditional test, based on the current value of the Z flag was added
   to skip over the rest of the instruction sequence

 - the store itself is implemented through a call to the __stl_mmu helper
   function.

The thing is that __stl_mmu may implement a *page fault* (i.e. when the
address in r4 hasn't been commited to memory yet) which requires a switch
to kernel mode (to populate the page), then going back to the instruction's
execution.

This is done in the current implementation by re-running the JIT-er for the
same instruction, however, since 'condexec_bits' was already cleared to 0,
the new JIT-ed code sequence doesn't have the conditional test to skip over
the store. The conditional instruction has been transformed into a
non-conditional one due to the page fault ! This results in either bad behaviour
or, even a crash in the emulator.

The patch fixes the clearing of condexec_bits to happen as it should, i.e.
only when execution has really cleared it.

This is preliminary work to fix the -trace option.

Also, disable the IO Thread when running the standalone emulator. This makes
debugging much easier since everything happens in a single thread.
---
 target-arm/translate.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

(limited to 'target-arm')

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 15239d1..45dd237 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -58,6 +58,7 @@ typedef struct DisasContext {
     /* Thumb-2 condtional execution bits.  */
     int condexec_mask;
     int condexec_cond;
+    int condexec_mask_prev;  /* mask at start of instruction/block */
     struct TranslationBlock *tb;
     int singlestep_enabled;
     int thumb;
@@ -3515,6 +3516,11 @@ gen_set_condexec (DisasContext *s)
         tcg_gen_movi_i32(tmp, val);
         store_cpu_field(tmp, condexec_bits);
     }
+    else if (s->condexec_mask_prev != 0) {
+        TCGv tmp = new_tmp();
+        tcg_gen_movi_i32(tmp, 0);
+        store_cpu_field(tmp, condexec_bits);
+    }
 }
 
 static void gen_nop_hint(DisasContext *s, int val)
@@ -8789,6 +8795,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
     dc->condjmp = 0;
     dc->thumb = env->thumb;
     dc->condexec_mask = (env->condexec_bits & 0xf) << 1;
+    dc->condexec_mask_prev = dc->condexec_mask;
     dc->condexec_cond = env->condexec_bits >> 4;
 #if !defined(CONFIG_USER_ONLY)
     if (IS_M(env)) {
@@ -8813,14 +8820,6 @@ static inline void gen_intermediate_code_internal(CPUState *env,
         max_insns = CF_COUNT_MASK;
 
     gen_icount_start();
-    /* Reset the conditional execution bits immediately. This avoids
-       complications trying to do it at the end of the block.  */
-    if (env->condexec_bits)
-      {
-        TCGv tmp = new_tmp();
-        tcg_gen_movi_i32(tmp, 0);
-        store_cpu_field(tmp, condexec_bits);
-      }
 #ifdef CONFIG_TRACE
     if (tracing) {
         gen_helper_traceBB(trace_static.bb_num, (target_phys_addr_t)tb );
@@ -8880,6 +8879,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
 
         if (env->thumb) {
             disas_thumb_insn(env, dc);
+            dc->condexec_mask_prev = dc->condexec_mask;
             if (dc->condexec_mask) {
                 dc->condexec_cond = (dc->condexec_cond & 0xe)
                                    | ((dc->condexec_mask >> 4) & 1);
@@ -8892,7 +8892,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
             disas_arm_insn(env, dc);
         }
         if (num_temps) {
-            fprintf(stderr, "Internal resource leak before %08x\n", dc->pc);
+            fprintf(stderr, "Internal resource leak before %08x (%d temps)\n", dc->pc, num_temps);
+            tcg_dump_ops(&tcg_ctx, stderr);
             num_temps = 0;
         }
 
-- 
cgit v1.1