From 3af4f6ae1fa6e06de1284fa1143cb8a485ac6437 Mon Sep 17 00:00:00 2001 From: David 'Digit' Turner 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. --- android-configure.sh | 6 ------ target-arm/translate.c | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/android-configure.sh b/android-configure.sh index 3e5d654..84e4535 100755 --- a/android-configure.sh +++ b/android-configure.sh @@ -426,12 +426,6 @@ case $OS in ;; esac -case $OS in - linux-*) - echo "#define CONFIG_IOTHREAD 1" >> $config_h - ;; -esac - echo "#define CONFIG_$CONFIG_OS 1" >> $config_h if [ $BSD = 1 ] ; then echo "#define _BSD 1" >> $config_h 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