From e7ad2fda91652e0a8c1af8b806265863731594f0 Mon Sep 17 00:00:00 2001 From: mio Date: Tue, 18 Feb 2025 23:33:51 +0800 Subject: [PATCH] Further fix MIPS delay slot --- include/uc_priv.h | 8 +++++ qemu/include/tcg/tcg-op.h | 2 +- qemu/target/mips/translate.c | 57 +++++++++++++++++++++--------------- tests/unit/test_mips.c | 6 +++- uc.c | 14 +++++++-- 5 files changed, 59 insertions(+), 28 deletions(-) diff --git a/include/uc_priv.h b/include/uc_priv.h index 07b5c244..484fc53e 100644 --- a/include/uc_priv.h +++ b/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, ...) \ diff --git a/qemu/include/tcg/tcg-op.h b/qemu/include/tcg/tcg-op.h index b57f96b1..93026d1d 100644 --- a/qemu/include/tcg/tcg-op.h +++ b/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) { diff --git a/qemu/target/mips/translate.c b/qemu/target/mips/translate.c index 383fcb1b..4d0b3eb0 100644 --- a/qemu/target/mips/translate.c +++ b/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) { - // the callback might want to stop emulation immediately - check_exit_request(tcg_ctx); - } + + // TODO: Memory hooks, maybe use icount_decr.low? + TCGLabel *skip_label = gen_new_label(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); } diff --git a/tests/unit/test_mips.c b/tests/unit/test_mips.c index 33011818..41325a34 100644 --- a/tests/unit/test_mips.c +++ b/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)); } diff --git a/uc.c b/uc.c index e1dcd4d5..52ce2227 100644 --- a/uc.c +++ b/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; } }