add uc_hook_set_user_data#2274
Conversation
|
LGTM. |
Shall I rebase this to current dev to rerun the CI or is there anything else blocking the merge? |
| uc_err uc_hook_set_user_data(uc_hook hh, void *user_data) | ||
| { | ||
| struct hook *hook = (struct hook *)hh; | ||
| if (hook->type == UC_HOOK_BLOCK || hook->type == UC_HOOK_CODE) { |
There was a problem hiding this comment.
Wait why do we exclude these two types?
There was a problem hiding this comment.
Because the callbacks are directly called by the tb with the user_data pointer written to the tb. It would require to rebuild the tb to allow change the user_data.
There was a problem hiding this comment.
Oh that's correct. Can we inline the pointer to the hook struct in the tb instead? In that case, the modification to uc_hook would take effect as well.
There was a problem hiding this comment.
No because there is no wrapper function in unicorn for these hooks. The cb function is directly called. I could write a wrapper function which just use the hook_struct. This might affect performance which I would like to avoid in the block case.
There was a problem hiding this comment.
Oh okay, I see. I forgot the inline optimization.
A wrapper would hurt performance quite a lot. Spare me some time to think about if we have better ways.
There was a problem hiding this comment.
For a bit more context, not enabling inlining for more than 1 hooks was due to a bad hack in tcg_optimize leading to segfault at that time. Since I removed the hack recently, we could extend inlining to any number of hooks.
There was a problem hiding this comment.
I somehow misinterpreted what this code is doing, thanks for explaining.
There was a problem hiding this comment.
Also I would also recommend not allowing changing data during emulation.
There was a problem hiding this comment.
So I guess this can be merged now?
There was a problem hiding this comment.
yeah, look good now except the small typo and documents above.
This allows to change the user_data for hooks. This way it's simpler to adopt the hooks for current needs. I.e. change the page table entries for the tlb hook.
PhilippTakacs
force-pushed
the
hook_set_user_data
branch
from
fd9fa29 to
dca893c
Compare
|
@wtdcode can you merge this PR or is there something missing/blocking? |
Now it is possible to change the user data of block and code hooks. The corresponding translation blocks are cleared. It is only allowed to be done while the emulation is stopped.
include/unicorn/unicorn.h
Outdated
| @user_data: user-defined data. This will be passed to callback function in its | ||
| last argument @user_data | ||
|
|
||
| @return UC_ERR_OK on success, or UC_ERR_ARGS when the hook was block or code |
There was a problem hiding this comment.
The documents are not updated here (return error during emulation) and UC_ERR_ARGS seems a typo.
2 participants
This allows to change the user_data for hooks. This way it's simpler to adopt the hooks for current needs. I.e. change the page table entries for the tlb hook.