Skip to content

BUG: Some basic blocks have been incorrectly removed. #80

Open
@wallds

Description

Hello.

DOCTEST_TEST_CASE("dummy")
{
    vtil::logger::log("\n\n>> %s \n", __FUNCTION__);
    auto block = vtil::basic_block::begin(0);
    auto [t0, t1, t2, t3] = block->tmp(64, 64, 1, 64);
    auto rtn = block->owner;
    block->mov(t0, vtil::REG_FLAGS);
    block->bnot(t0);
    block->ifs(t1, t0.select(1, 2), 0x1000);
    block->mov(t2, t0.select(1, 2));
    block->bnot(t2);
    block->ifs(t3, t2, 0x2000);
    block->add(t1, t3);
    block->add(t1, vtil::REG_IMGBASE);
    block->jmp(t1);

    if (auto block_1000 = block->fork(0x1000)) {
        block_1000->jmp(0x3000);
        block_1000->fork(0x3000);
    }
    if (auto block_2000 = block->fork(0x2000)) {
        block_2000->jmp(0x3000);
        block_2000->fork(0x3000);
    }
    if (auto block_3000 = rtn->get_block(0x3000)) {
        block_3000->vexit(uintptr_t(0xdeadc0de));
    }

    vtil::logger::log(":: Before:\n");
    vtil::debug::dump(rtn);
    vtil::optimizer::bblock_thunk_removal_pass{}(rtn);
    vtil::optimizer::branch_correction_pass{}(rtn);
    vtil::logger::log(":: After:\n");
    vtil::debug::dump(rtn);
    vtil::logger::log(":: Over:\n");
    CHECK(1 == 1);
}

Result

image

Relevant code

for ( auto it = blk->next.begin(); it != blk->next.end(); )
{
// Check if this destination is plausible or not.
//
vip_t target = ( *it )->entry_vip;
bool plausible = false;
for ( auto& branch : branch_info.destinations )
plausible |= ( branch == target ).get<bool>().value_or( true );
// If it is not:
//
if ( !plausible )
{
// Delete prev and next links.
//
( *it )->prev.erase( std::remove( ( *it )->prev.begin(), ( *it )->prev.end(), blk ), ( *it )->prev.end() );
it = blk->next.erase( it );
// Increment counter and continue.
//
cnt++;
continue;
}
// Otherwise increment iterator and continue.
//
++it;
}

Description

The branch_correction_pass will remove branches that do not exist in the branch_info.
I think this is correct, but the bblock_thunk_removal_pass is too aggressive and also handles jump instructions that have not been correctly converted to js instructions.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions