Skip to content

[RFC] refactor: make hook callbacks sound - #17

Draft
Jvlegod wants to merge 1 commit into
unicorn-engine:mainfrom
Jvlegod:refactor/sound-hooks
Draft

Jvlegod wants to merge 1 commit into
unicorn-engine:mainfrom
Jvlegod:refactor/sound-hooks

Conversation

@Jvlegod

@Jvlegod Jvlegod commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Ref #14

  • Introduce HookContext as the callback API.
    • We need to perform CPU operations within the callback.
    • We cannot create a second unicorn owner within the callback.
  • use Box replace Rc the emulator.

what's more: Perhaps we should rename new_with_data to new and make it take only arch and mode.

@Jvlegod
Jvlegod force-pushed the refactor/sound-hooks branch from e4c04f1 to e5875d7 Compare September 7, 2026 16:30
@Jvlegod

Jvlegod commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

How about this refactor? :>
I would like to hear your suggestions.

Hook callbacks previously reconstructed Unicorn values from shared
Rc<UnsafeCell<_>> state, which could create multiple mutable aliases to
the same emulator state.

Introduce HookContext as the callback API, and use Box for the emulator.

Signed-off-by: Jvle <keke.oerv@isrc.iscas.ac.cn>
@Jvlegod
Jvlegod force-pushed the refactor/sound-hooks branch from e5875d7 to 68de01a Compare September 7, 2026 16:34
@PhilippTakacs

Copy link
Copy Markdown
Contributor

I just had a quick look. I'm not quite sure what exactly you want to make more sound. Can you explain a bit more what is the problem you want to solve.

Also with this patch the user_data is practical disabled. For small examples it is not that bad to replace the user_data with closure, but for bigger code it quickly gets ugly.

@PhilippTakacs

Copy link
Copy Markdown
Contributor

Another problem is that you need now to implement every api twiche (one time for the Unicorn struct and one time for the HookContext).

@Jvlegod

Jvlegod commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

I'm not quite sure what exactly you want to make more sound.

To be honest, the goal of this patch is to fix an aliasing problem in the old callback.

HookContext is intended to give callbacks access to the current engine handle without creating another Unicorn owner. Box provides unique ownership of UnicornInner.

We can see the an example:

In the old imp.

The callback could modify the same internal state through a second Unicorn while the original Unicorn was still in use by emu_start.

  • uc -> Rc<UnsafeCell>
  • also, callback_uc -> the same Rc<UnsafeCell>
let mut uc = Unicorn::new(Arch::X86, Mode::MODE_64).unwrap();

uc.add_code_hook(0x1000, 0x2000, |callback_uc, _, _| {
    callback_uc
        .reg_write(RegisterX86::RAX, 1)
        .unwrap();
})
.unwrap();

uc.emu_start(0x1000, 0x2000, 0, 0).unwrap();

I wonder if there have a better way?

Another problem is that you need now to implement every api twiche

Yes, I have also thought about this question. I think this PR just a draft.

@PhilippTakacs

Copy link
Copy Markdown
Contributor

It might work using uc_emu_start() as a freestanding function taking ownership over the unicorn struct and giving it back on return.

@Jvlegod

Jvlegod commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

It might work using uc_emu_start() as a freestanding function taking ownership over the unicorn struct and giving it back on return.

Yes, it's a good idea.

But from a user experience perspective, these approaches would seem very strange.

Maybe you mean:

let mut uc = Unicorn::new_with_data(Arch::X86, Mode::MODE_32, 0u64).unwrap();

// it's strange 
uc = uc_emu_start(uc, CODE_START, CODE_START + code.len() as u64, 0, 0);

// or like
uc = uc.emu_start(CODE_START, CODE_START + code.len() as u64, 0, 0);

@Jvlegod
Jvlegod marked this pull request as draft September 8, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants