Need some help with implementing the post-instruction hook.

[farmdve]

Before I start I'd like to just mention that maybe we need an IRC channel to discuss things. e.g #unicorn-engine on freenode.

So, as you all of you may be aware, currently we do a pre-instruction hook. Which means our hook is called before the instruction is executed, thus it makes sense to have a post-instruction hook. The pre hook would setup some stuff, the post hook would change them back(if need be).

This past week is what I've been trying to do, but it turned out to be much more difficult than I thought.

The problem mostly stems from the way some instructions are jitted. Some are jitted to exit the translation block and enter it again. At first I placed the post instruction hook after the instruction was jitted, e.g at the end of disas_insn, this worked for a bit, until I encountered instructions like REP STOS which is jitted in a way that it checks condition first, does whatever operation it must and re-enters the translation block to check the condition again, thereby bypassing my hook. I kept this hook, but added the same code to the gen_op_jmp_v which again worked magnificently, except now instructions like ret were not being hooked. So back at square one. I decided that I'd add the post-instruction hook in the same place as the pre-instruction hook.

    if (env->uc->hook_insn) {
        trace = hook_find(env->uc, UC_HOOK_CODE, pc_start);
        if (trace) {
            if (s->last_cc_op != s->cc_op) {
                sync_eflags(s, tcg_ctx);
                s->last_cc_op = s->cc_op;
                changed_cc_op = true;
            }
            // generate code to call callback
            gen_uc_tracecode(tcg_ctx, 0xf1f1f1f1, trace->callback, env->uc, pc_start, trace->user_data);
            // the callback might want to stop emulation immediately
            check_exit_request(tcg_ctx);
        }
    }

Adding here would be more or less the same, it would be called before the next instruction is executed. But unfortunately it is also going to be plagued by other problems. The first is the early check code "// early check to see if the address of this block is the until address", our post instruction code never gets jitted if this is the last address.
The other problem is that instructions like REP STOSD/MOVS would re-enter the TB and execute our hook once again(in addition to the pre-instruction hook for the current instruction).

Suggestions?

[lunixbochs]

I just registered #unicorn-engine on Freenode, can give ownership to @aquynh (and perhaps it could go in documentation or the website?)

I support your method of running both hooks from the same code area, as it simplifies things. For REP, do you mean the post-instruction hook for the instruction before REP will be repeatedly executed?

Maybe the "until" exit-handler itself should also execute the pending post-instruction hook. The timeout exit case could also execute a pending hook as long as the instruction finished.

[farmdve]

@lunixbochs

In this case, both hooks will be called again. The REP STOS instruction is jitted to regular movs and a call to a qemu handler to store the data at the address pointed to by [EDI]. But it's jitted in such a way, that the pre-instruction hook is called one additional time.

It means for 2 ECX block operations, the hook is called 3 times, instead of 2.

The implications of this is, instruction counting can and will be wrong.

Btw, I should obviously note that adding the post instruction hook means double the time to execute some code, compared to a pre-instruction hook alone.

[aquynh]

thanks for creating the IRC channel, @lunixbochs. i can announce it and will visit it, but personally i am not sure how much time i can spend there as IRC is pretty distracting. but this is surely useful for quick discussion and can help new users.

@farmdve, could you publish your branch so people can look at that and comment? post-instruction hooking is an important missing feature for us. we can make it work first, then improve the performance later.

[ZakDanger]

This is somewhat related to my ramblings in issue #332.
Where I am suggesting having a fetch hook as well as an execution hook.
The fetch hook is a bit like a pre-pre-instruction execution hook.
So the order of hooks would be:
fetch
pre-exec
post-exec

[aquynh]

yes the calling order of hooks should be exactly like what physical machine does.

[farmdve]

I should probably mention the following. The way I've made the changes is, I've added an additional argument to the hooks, this means all projects that use UC_HOOK_CODE will need to be changed, to add the additional argument which specifies what type of callback it is, pre/post etc. This means the samples as well.

I felt as there was no choice but to add it, as it would've been difficult otherwise to discern hooks.

[JCYang]

I suggest, add a new hook type such as UC_HOOK_POST_INSTRUCTION or something like this, and also UC_HOOK_PRE_INSTRUCTION which is an alias of UC_HOOK_CODE, don't break the current codes. and a new hook type make more sense, imho.

[aquynh]

yes, it is important not to break existing code.

we can define 2 new hook types like followings in enum uc_hook_type:

UC_HOOK_PRE = 0,
UC_HOOK_POST = 1 << 31,

then any existing hooks can be combined with these two types, such as:

UC_HOOK_INSN + UC_HOOK_PRE    // hook callback is executed before instruction
UC_HOOK_CODE + UC_HOOK_POST    // hook callback is called after instruction

since UC_HOOK_PRE is 0, all existing code use pre-hook by default, thus no change is needed.

[aquynh]

with the above approach, we just extend the semantics of existing API, but do not have to add a new argument to uc_hook_add().

[farmdve]

@aquynh

I see, ok.

However my changes do not break uc_hook_add, they change the callback parameters to add one additional argument. But they still break existing code yes.

void hook_ins(uc_engine *uc, uint64_t address, int32_t type, uint32_t size, void *user_data)
{   
    if(type == 0) //pre
    {
        // prepare something
    }
    else if(type == 1) //post
    {
        instructions++;
    }
}

Like that.

So you are proposing a uc_hook_add for both PRE and POST, not a single callback for both?

[aquynh]

is there a good reason to add the type argument to the callback?

if you dont pass the hook type to the callback, then everything is all the same.

[farmdve]

Because a user will not know what type of hook it is. And no, it's not possible to track them, because the post instruction hook may not be called if an instruction fails to execute, e.g unhandled unmapped memory etc.

[aquynh]

Lets change the perspective: user registeres the callback, so he must know which callback is used. And he needs to take care his callbackS, we don't do that for him. Then everything is fine now, isn't it?

[farmdve]

If it's two user callbacks

uc_hook_add(PreCallback);
uc_hook_add(PostCallback);

Then yes.

[aquynh]

Exactly yes.