search

BinaryAnalysisPlatform / bap

Binary Analysis Platform
MIT License
2.07k stars 273 forks source link

prevents IR block contraction on unconditional edges #1543

Closed heyitsanthony closed 2 years ago

heyitsanthony commented 2 years ago

Contraction / normalize conditions changed in 2.5.0 which causes IR normalization to drop the "when RCX <> 0 ... goto ..." for the instruction "rep cmpsb" (\xf3\xa6).

This patch adds an extra keep/weak check to can_contract that keeps the when-cnd-goto made when reifying the rep while loop.

heyitsanthony commented 2 years ago

This popped up in SE tests as a segfault. I spent some time looking at the git history trying to understand this code; there were slightly different checks on b1 that were removed. I don't know if this is the best fix (tests are passing on my side at least), but it seems worst case here will be sometimes the IR will not contract in cases where it could.

ivg commented 2 years ago

Good catch! The root of the problem was that we were contracting conditional edges, which we shouldn't do at all. In most cases, conditional branches shall result in two edges so this bug wasn't caught by our test suite, though the only exception is the rep prefix in the old lifter that generates a slightly degenerative (though still valid) IR code that has this lonely conditional branch (which will be connected to the fallthrough block by the downstream analysis). The fix that I propose is to add an additional check to the single_dst function so that we will no longer treat conditional jumps as jumps with a single destination (indeed conditional jumps are branches and have two destinations).

heyitsanthony commented 2 years ago

Ah, OK. Thanks for the explanation!