unicorn
unicorn copied to clipboard
unicorn-engine
Need some help with implementing the post-instruction hook.
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?
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.
@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.
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.
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
yes the calling order of hooks should be exactly like what physical machine does.
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.
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.
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.
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().
@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?
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.
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.
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?
If it's two user callbacks
uc_hook_add(PreCallback); uc_hook_add(PostCallback);
Then yes.
Then, I'll have it rework it. Meanwhile, this is unrelated but I plan to launch an online emulation platform to test out Unicorn. This way I can more easily show you quirks that I've discovered without cluttering the tests/regress folder.
And it will be casually used for x86 emulation. It won't be anything fancy, and I have not fully planned it.
It is still very important to have the testcases in tests/ so we can take care the regression.
Because of the post instruction hook, and depending on whether either are active or both, the argument patching for instruction size(for the pre hook) has become more difficult, and I dare say error prone.
Do you suppose we should use some pointer instead? Like store a pointer to that argument and then patch only it? This way we avoid lots of logic to check for changed_cc_op, cc_op_dirty and so forth.
Moreover, I've come to the conclusion that if the post-instruction hook is active, we should avoid syncing the flags in the pre-hook, as the post-hook would do that for us, and it also made it even more difficult to find the correct argument.
can you post the code, so i can look at it? it is hard to discuss without the code.
Here is the commit. Although git pushed to my master rather than new branch :/
https://github.com/farmdve/unicorn/commit/6648e41ab987bbf46301e1458fa1d4a169cae1df
I tested the code as much as I could, this is why it took me so long. The logic involved in the argument patching for size was difficult to nail down. I compiled the code both in MinGW32 and MinGW64, the pre/post hooks work.
Both can be on or off or either of them. However, note that I've only introduced the change in target-i386 as I am much more familiar with that target, not sure about the rest(arm,mips,sparc etc). This is why I have not submitted a pull request.
can you write a sample on this new post hook and put it under samples/ ?
i will look closer, but dont worry about the rest architectures, you can do your pull request now.
thanks.
Sure, ok. Will do it tomorrow though.
In the meantime, all is well, but like I said, REP prefixed instructions, due to the way they are jitted will cause the hooks to execute one additional time.
The code is roughly jitted like so: 1.Check if ECX > 0 2. If true, execute qemu_st instruction 2.2 goto 1. 3. If false, continue/exit TB.
So you can see the code is jitted so that ECX is first checked if it's > 0. So basically the hook will be called again, but the instruction itself will not be executed if ECX <= 0, but the pre and post instruction hooks will execute, sadly. This will make instruction counting incorrect.
I see no easy way to fix this.
for any problem, there is always a solution. firstly you should decide what is the best place to put the pre & post hooks for special cases like this, then implement them.
so even current pre hook has problem with REP? if so, please send a pull request with a testcase, i will look at that.
see the new regress test https://github.com/unicorn-engine/unicorn/blob/master/tests/regress/rep_hook.py
this tests REP STOSB instruction, with ECX=3. the output is:
$ ./rep_hook.py
hook called at 0
hook called at 0
hook called at 0
hook called at 0
.
----------------------------------------------------------------------
Ran 1 test in 0.001s
OK
if you want to monitor every iterations of STOSB, this is currently fine. but if you consider REP STOSB as one instruction and want to get called only once, this is not. so this still depends on what you want.
Personally, I believe we should monitor all instructions. Even when single-stepping in a debugger, the REP prefixed instruction executes as many times as they need without EIP changing.
And yes, pre/post hooks will both be affected.
if so, currently the pre-hook works for you in the REP case, and there is no need to change anything.
But you see it's called one additional time. ECX = 3, then the hook should be called only three times, but in your test it's called 4 times.
I've checked the behaviour of the REP instruction and it executes only ECX times, but the way it's jitted invokes the hook one more time without doing anything.
because callback is called before the instruction is executed, it should be 4 times, no? the last time ECX=0, so the instruction is not executed itself, but the callback is still called. remember this is the pre-hook case.
however, for the post-hook case, the callback should be called 3 times.
You would be correct usually, but it is because of the way the REP instruction is jitted by QEMU, it exits the TB and re-enters it. Think about it, if the instruction is never executed because ECX <= 0, then the pre-hook must not execute, because this instruction would've never executed either way.
This behaviour also affects post hook, because it runs before the pre hook for the current instruction.
ok, for the sake of simplicity, lets change the perspective. we can document our pre-hook callback like this:
execute the pre-hook callback
execute the hooked instruction - or may not in special cases, like when ECX=0 for REPxx
this is actually how the callback works at the moment. while this is not perfect like in the case of REP , as long as we know every cases and document all the potential issues so users can handle them properly, that will be fine.
overengineering in some cases causes too much overhead, thus overkill i think.
The user will have great difficulty in achieving that. They would have to specifically craft the hook to check for these special edge cases.
And that is only for the pre-callback.
this case is actually not that hard to handle. you can safely ignore this special case for now. i will come back to this when i have more time.
any updates on this? i have some code that would be greatly simplified by separate pre/post code hooks :)
@xorstream
A long time ago, however only for x86 for now.
https://github.com/farmdve/unicorn/commit/6648e41ab987bbf46301e1458fa1d4a169cae1df
@farmdve, please clear all the issues that i commented there, and do a pull request. x86-only is fine. if nobody does the rest archs, i will when i have more free time.
@aquynh
If the hook API is going to be revamped, I will wait as I will need to change the code either way. Plus right now I am also busy with some life-matters and I can't do anything.
@farmdve do you mind if I take 6648e41ab987bbf46301e1458fa1d4a169cae1df, port it to the hook refactor, and PR it? is there anything missing from that commit?
I don't mind, but the code for the size patching needs to made so that curly braces are after the if statement, not a new line. The post hook needs to be added to the other arches and finally, in unicorn.h when adding UC_HOOK_PRE, the other members of the enum must not be changed to be +1 as I've mistakenly done in my commit.
does post-hook support arm32 at this moment? I want to print a register value after the instruction has run... so need a post-hook call back...
Back when I worked on/with Unicorn instruction hooks got called more than once and not when you wanted them to due to the way JIT worked at the time. Not sure how it is now.
On Sat, 5 Sep 2020 at 06:04, my [email protected] wrote:
does post-hook support arm32 at this moment? I want to print a register value after the instruction has run... so need a post-hook call back...
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/unicorn-engine/unicorn/issues/346#issuecomment-687537154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3DGSGETKVH6BG7WH6Q5WLSEGTDNANCNFSM4BXOQ5HA .
