Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions include/unicorn/unicorn.h
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,22 @@ uc_err uc_hook_add(uc_engine *uc, uc_hook *hh, int type, void *callback,
UNICORN_EXPORT
uc_err uc_hook_del(uc_engine *uc, uc_hook hh);

/*
change the user data from a hook callback.
This change the user-defined data for a given hook.
NOTE: It's undefinde behavior when called on a hook which was not initialized
by uc_hook_add or deleted by uc_hook_delete
@uc: handle returned by uc_open()
@hh: handle returned by uc_hook_add()
@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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documents are not updated here (return error during emulation) and UC_ERR_ARGS seems a typo.

hook
*/
UNICORN_EXPORT
uc_err uc_hook_set_user_data(uc_engine *uc, uc_hook hh, void *user_data);

/*
Variables to control which state should be stored in the context.
Defaults to UC_CTL_CONTEXT_CPU. The options are used in a bitfield
Expand Down
18 changes: 18 additions & 0 deletions uc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2060,6 +2060,24 @@ uc_err uc_hook_del(uc_engine *uc, uc_hook hh)
return UC_ERR_OK;
}

UNICORN_EXPORT
uc_err uc_hook_set_user_data(uc_engine *uc, uc_hook hh, void *user_data)
{
struct hook *hook = (struct hook *)hh;
if (hook->type == UC_HOOK_BLOCK || hook->type == UC_HOOK_CODE) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait why do we exclude these two types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I somehow misinterpreted what this code is doing, thanks for explaining.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I would also recommend not allowing changing data during emulation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess this can be merged now?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, look good now except the small typo and documents above.

if (uc->nested_level) {
return UC_ERR_ARG;
}
if (hook->end < hook->begin) {
uc->tb_flush(uc);
} else {
uc->uc_invalidate_tb(uc, hook->begin, hook->end - hook->begin);
}
}
hook->user_data = user_data;
return UC_ERR_OK;
}

// TCG helper
// 2 arguments are enough for most opcodes. Load/Store needs 3 arguments but we
// have memory hooks already. We may exceed the maximum arguments of a tcg
Expand Down
Loading