Further fix MIPS delay slot

include/uc_priv.h

@@ -534,6 +534,14 @@ static inline uc_err break_translation_loop(uc_engine *uc)
     return UC_ERR_OK;
 }
 
+static inline void revert_uc_emu_stop(uc_engine *uc)
+{
+    uc->stop_request = 0;
+    uc->cpu->exit_request = 0;
+    uc->cpu->tcg_exit_req = 0;
+    uc->cpu->icount_decr_ptr->u16.high = 0;
+}
+
 #ifdef UNICORN_TRACER
 #define UC_TRACE_START(loc) trace_start(get_tracer(), loc)
 #define UC_TRACE_END(loc, fmt, ...)                                            \

qemu/include/tcg/tcg-op.h

@@ -47,7 +47,7 @@ static inline void gen_uc_tracecode(TCGContext *tcg_ctx, int32_t size, int32_t t
     };
 
     const int hook_type = type & UC_HOOK_IDX_MASK;
-    if (puc->hooks_count[hook_type] == 1) {
+    if (puc->hooks_count[hook_type] == 1 && !(type & UC_HOOK_FLAG_NO_STOP)) {
         cur = puc->hook[hook_type].head;
         
         while (cur) {

qemu/target/mips/translate.c

@@ -30924,7 +30924,7 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     struct uc_struct *uc = cs->uc;
     TCGContext *tcg_ctx = uc->tcg_ctx;
-    TCGOp *tcg_op, *prev_op = NULL;
+    TCGOp *tcg_op, *prev_op = NULL, *slot_op = NULL;
     int insn_bytes;
     int is_slot;
     bool hook_insn = false;
@@ -30949,11 +30949,17 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
         prev_op = tcg_last_op(tcg_ctx);
         hook_insn = true;
         gen_uc_tracecode(tcg_ctx, 4, UC_HOOK_CODE_IDX, uc, ctx->base.pc_next);
-        // Don't let unicorn stop at the branch delay slot.
+        
-        if (!is_slot) {
+        // TODO: Memory hooks, maybe use icount_decr.low?
-            // the callback might want to stop emulation immediately
+        TCGLabel *skip_label = gen_new_label(tcg_ctx);
-            check_exit_request(tcg_ctx);
+        TCGv_i32 dyn_is_slot = tcg_const_i32(tcg_ctx, 0);
-        }
+        slot_op = tcg_last_op(tcg_ctx);
+        // if slot, skip exit_request
+        // tcg is smart enough to optimize this branch away
+        tcg_gen_brcondi_i32(tcg_ctx, TCG_COND_GT, dyn_is_slot, 0, skip_label); 
+        tcg_temp_free_i32(tcg_ctx, dyn_is_slot);
+        check_exit_request(tcg_ctx);
+        gen_set_label(tcg_ctx, skip_label);
     }
     
     if (ctx->insn_flags & ISA_NANOMIPS32) {
@@ -30975,23 +30981,6 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
         return;
     }
 
-    if (hook_insn) {
-        // Unicorn: patch the callback to have the proper instruction size.
-        if (prev_op) {
-            // As explained further up in the function where prev_op is
-            // assigned, we move forward in the tail queue, so we're modifying the
-            // move instruction generated by gen_uc_tracecode() that contains
-            // the instruction size to assign the proper size (replacing 0xF1F1F1F1).
-            tcg_op = QTAILQ_NEXT(prev_op, link);
-        } else {
-            // this instruction is the first emulated code ever,
-            // so the instruction operand is the first operand
-            tcg_op = QTAILQ_FIRST(&tcg_ctx->ops);
-        }
-
-        tcg_op->args[1] = insn_bytes;
-    }
-
     if (ctx->hflags & MIPS_HFLAG_BMASK) {
         if (!(ctx->hflags & (MIPS_HFLAG_BDS16 | MIPS_HFLAG_BDS32 |
                              MIPS_HFLAG_FBNSLOT))) {
@@ -31010,6 +30999,28 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
             is_slot = 1;
         }
     }
+
+    if (hook_insn) {
+        // Unicorn: patch the callback to have the proper instruction size.
+        if (prev_op) {
+            // As explained further up in the function where prev_op is
+            // assigned, we move forward in the tail queue, so we're modifying the
+            // move instruction generated by gen_uc_tracecode() that contains
+            // the instruction size to assign the proper size (replacing 0xF1F1F1F1).
+            tcg_op = QTAILQ_NEXT(prev_op, link);
+        } else {
+            // this instruction is the first emulated code ever,
+            // so the instruction operand is the first operand
+            tcg_op = QTAILQ_FIRST(&tcg_ctx->ops);
+        }
+
+        tcg_op->args[1] = insn_bytes;
+    }
+
+    if (slot_op) {
+        slot_op->args[1] = is_slot;
+    }
+
     if (is_slot) {
         gen_branch(ctx, insn_bytes);
     }

tests/unit/test_mips.c

@@ -53,20 +53,24 @@ static void test_mips_stop_at_branch(void)
 {
     uc_engine *uc;
     char code[] =
-        "\x02\x00\x00\x08\x00\x00\x00\x00\x00\x00\x00\x00"; // j 0x8; nop;
+        "\x02\x00\x00\x08\x21\x10\x62\x00"; // j 0x8; addu $v0, $v1, $v0;
     int r_pc = 0x0;
+    uint32_t v1 = 5;
 
     uc_common_setup(&uc, UC_ARCH_MIPS, UC_MODE_MIPS32 | UC_MODE_LITTLE_ENDIAN,
                     code, sizeof(code) - 1);
 
+    OK(uc_reg_write(uc, UC_MIPS_REG_V1, &v1));
     // Execute one instruction with branch delay slot.
     OK(uc_emu_start(uc, code_start, code_start + sizeof(code) - 1, 0, 1));
 
     OK(uc_reg_read(uc, UC_MIPS_REG_PC, &r_pc));
+    OK(uc_reg_read(uc, UC_MIPS_REG_V0, &v1));
 
     // Even if we just execute one instruction, the instruction in the
     // delay slot would also be executed.
     TEST_CHECK(r_pc == code_start + 0x8);
+    TEST_CHECK(v1 == 0x5);
 
     OK(uc_close(uc));
 }

uc.c

@@ -1978,8 +1978,12 @@ void helper_uc_tracecode(int32_t size, uc_hook_idx index, void *handle,
         index &
         UC_HOOK_FLAG_MASK; // The index here may contain additional flags. See
                            // the comments of uc_hook_idx for details.
-
+    // bool not_allow_stop = (size & UC_HOOK_FLAG_NO_STOP) || (hook_flags & UC_HOOK_FLAG_NO_STOP);
+    bool not_allow_stop = hook_flags & UC_HOOK_FLAG_NO_STOP;
+    
     index = index & UC_HOOK_IDX_MASK;
+    // // Like hook index, only low 6 bits of size is used for representing sizes.
+    // size = size & UC_HOOK_IDX_MASK;
 
     // This has been done in tcg code.
     // sync PC in CPUArchState with address
@@ -1988,8 +1992,10 @@ void helper_uc_tracecode(int32_t size, uc_hook_idx index, void *handle,
     // }
 
     // the last callback may already asked to stop emulation
-    if (uc->stop_request && !(hook_flags & UC_HOOK_FLAG_NO_STOP)) {
+    if (uc->stop_request && !not_allow_stop) {
         return;
+    } else if (not_allow_stop && uc->stop_request) {
+        revert_uc_emu_stop(uc);
     }
 
     for (cur = uc->hook[index].head;
@@ -2021,7 +2027,9 @@ void helper_uc_tracecode(int32_t size, uc_hook_idx index, void *handle,
         //   normally. No check_exit_request is generated and the hooks are
         //   triggered normally. In other words, the whole IT block is treated
         //   as a single instruction.
-        if (uc->stop_request && !(hook_flags & UC_HOOK_FLAG_NO_STOP)) {
+        if (not_allow_stop && uc->stop_request) {
+            revert_uc_emu_stop(uc);
+        } else if (!not_allow_stop && uc->stop_request) {
             break;
         }
     }