Hooks: Add warning about ARM IT blocks with UC_HOOK_CODE

[wuetj]

The Hooks.md currently mentions this about UC_HOOK_CODE:

unicorn/docs/Hooks.md

Line 35 in c24c9eb

If you want to inject behaviour you might use this hook to modify the registers - including modifying the program counter, to jump to a different place.

So in my application, which emulates a Cortex M7, I ended up using this hook to modify the program counter during execution.

For some reason the changes to the program counter were sometimes ignored. After a lot of debugging I stumbled across this piece of code:

unicorn/uc.c

Lines 2153 to 2160 in c24c9eb

	// the last callback may already asked to stop emulation
	// Unicorn:
	// In an ARM IT block, we behave like the emulation continues
	// normally. No check_exit_request is generated and the hooks are
	// triggered normally. In other words, the whole IT block is treated
	// as a single instruction.
	if (not_allow_stop && uc->stop_request) {
	revert_uc_emu_stop(uc);

This was the reason for the ignored PC changes in my case. I created a simple test case to reproduce this:

Expand test case

static void test_arm_modify_pc_callback(uc_engine *uc, uint64_t address,
                                        uint32_t size, void *user_data)
{
    uint64_t *expected_pc = (uint64_t *)user_data;

    if (*expected_pc != 0) {
        TEST_CHECK(*expected_pc == address);
        exit(0);
    }

    // Modify PC wile in itt block
    if (address == code_start + 4) {
        uint32_t pc = code_start + 4 + 1;
        uc_reg_write(uc, UC_ARM_REG_PC, &pc);
        *expected_pc = pc - 1;
    }
}

static void test_arm_it_modify_pc(void)
{
    char code[] =
        "\x05\xbf" // ittet   eq
        "\x00\x18" // addeq   r0, r0, r0
        "\x01\x18" // addeq   r1, r0, r0
        "\x02\x18" // addne   r2, r0, r0
        "\x03\x18" // addeq   r3, r0, r0
        "\xf9\xe7" // b code_start
        "\xf8\xe7" // b code_start
        ;
    uc_engine *uc;
    uc_hook hook;
    uint64_t expected_pc = 0;

    uc_common_setup(&uc, UC_ARCH_ARM, UC_MODE_THUMB, code, sizeof(code) - 1,
                UC_CPU_ARM_CORTEX_M7);

    OK(uc_hook_add(uc, &hook, UC_HOOK_CODE, test_arm_modify_pc_callback,
                   &expected_pc, 1, 0));

    OK(uc_emu_start(uc, code_start | 1, code_start + sizeof(code) - 1, 0, 0));

    OK(uc_close(uc));
}

In order to avoid anyone from running into the same issue this PR adds a warning to the hooks documentation.

Some potential "fixes" I could think of:

• Allowing emulation to exit from within an IT block.
  I don't know enough about qemu/unicorn to know what is preventing it from doing that in the first place.
• Only invoking the callback once per IT block. Maybe with the size of the entire block?

[wtdcode]

I don't know enough about qemu/unicorn to know what is preventing it from doing that in the first place.

The root cause is that if we stop within an IT block, we can no longer restart emulation because the conditon flags are lost, though I agree that jumping to elsewhere by modifying PC counter is fairly safe. See #853

Only invoking the callback once per IT block. Maybe with the size of the entire block?

That's also possible but much harder to do.

[wuetj]

Wouldn't this currently also cause issues for the timeout parameter in uc_emu_start? It calls uc_emu_stop in a separate thread, which could potentially happen just before revert_uc_emu_stop during an IT block.

[wtdcode]

Wouldn't this currently also cause issues for the timeout parameter in uc_emu_start? It calls uc_emu_stop in a separate thread, which could potentially happen just before revert_uc_emu_stop during an IT block.

Not exactly because uc_emu_start only sends a signal while unicorn will ignore the signal within an IT block.