From 0d0d8b6db967d964f1c22f3cb40352a8fb9bb9d3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 3 Sep 2018 13:59:47 -0400 Subject: [PATCH] target/i386/translate: Report proper instruction size in code hooks This was intentionally broken to make updating qemu as quick as possible when it was woefully out of date, particularly because the interface of qemu's TCG changed quite a bit, so this code would have needed to be changed anyways. Now that qemu is up to date for this variant of Unicorn, we can repair this functionality and also--and I put massive emphasis on this, since this wasn't done in the original Unicorn repo--*actually document what the heck we're doing in this case*, so it's not a pain to change in the future if we actually need to do that. It makes it much, much, simpler for people not involved with qemu to understand what is going on in this case. --- qemu/target/i386/translate.c | 61 +++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/qemu/target/i386/translate.c b/qemu/target/i386/translate.c index a845eec3..b6886f9f 100644 --- a/qemu/target/i386/translate.c +++ b/qemu/target/i386/translate.c @@ -5053,8 +5053,6 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) TCGv cpu_T1 = tcg_ctx->cpu_T1; TCGv *cpu_regs = tcg_ctx->cpu_regs; TCGv *cpu_seg_base = tcg_ctx->cpu_seg_base; - //TCGArg* save_opparam_ptr = tcg_ctx->gen_opparam_buf + tcg_ctx->gen_op_buf[tcg_ctx->gen_op_buf[0].prev].args; - //bool cc_op_dirty = s->cc_op_dirty; bool changed_cc_op = false; s->pc_start = s->pc = pc_start; @@ -5070,13 +5068,40 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) } // Unicorn: trace this instruction on request + TCGOp* pre_hook_code_op = NULL; if (HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_CODE, pc_start)) { if (s->last_cc_op != s->cc_op) { sync_eflags(s, tcg_ctx); s->last_cc_op = s->cc_op; changed_cc_op = true; } + + // Unicorn: We get the address of the TCGOp *before* the tracecode operation + // as we'll need to patch it again further down with the proper size + // (as opposed to saying we have an instruction 0xF1F1F1F1 in size). + // + // This is necessary as the TCGOps are stored within a tail queue, + // and so, we can't simply retrieve the op in particular after we've + // generated it, as a tail queue doesn't allow that behavior. + // + // Within gen_uc_tracecode(), the first thing that occurs is the following: + // + // TCGv_i32 tsize = tcg_const_i32(tcg_ctx, size); + // + // This is the instruction size that we want to patch later on. + // tcg_const_i32 is as follows: + // + // TCGv_i32 t0 = tcg_temp_new_i32(s); + // tcg_gen_movi_i32(s, t0, val); + // return t0; + // + // And so the op *following the pre_hook_code_op* will be the move + // instruction that we want to patch with the correct size, which + // as stated, is done further down in the function. + // + pre_hook_code_op = tcg_last_op(tcg_ctx); gen_uc_tracecode(tcg_ctx, 0xf1f1f1f1, UC_HOOK_CODE_IDX, env->uc, pc_start); + // the callback might want to stop emulation immediately check_exit_request(tcg_ctx); } @@ -9012,30 +9037,16 @@ case 0x101: goto unknown_op; } - // FIXME: Amend this non-conforming garbage -#if 0 - // Unicorn: patch the callback for the instruction size - if (HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_CODE, pc_start)) { - // int i; - // for(i = 0; i < 20; i++) - // printf("=== [%u] = %x\n", i, *(save_opparam_ptr + i)); - // printf("\n"); - if (changed_cc_op) { - if (cc_op_dirty) -#if TCG_TARGET_REG_BITS == 32 - *(save_opparam_ptr + 16) = s->pc - pc_start; - else - *(save_opparam_ptr + 14) = s->pc - pc_start; -#else - *(save_opparam_ptr + 12) = s->pc - pc_start; - else - *(save_opparam_ptr + 10) = s->pc - pc_start; -#endif - } else { - *(save_opparam_ptr + 1) = s->pc - pc_start; - } + // Unicorn: patch the callback to have the proper instruction size. + if (pre_hook_code_op && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_CODE, pc_start)) { + // As explained further up in the function where pre_hook_code_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). + TCGOp* const op = QTAILQ_NEXT(pre_hook_code_op, link); + TCGArg* const args = op->args; + args[1] = s->pc - pc_start; } -#endif return s->pc; illegal_op: