Skip to content

fix(intel): restore dead reachable-collision cleanup in getBlocks#194

Open
r0ny123 wants to merge 1 commit into
danielplohmann:masterfrom
r0ny123:fix/audit-getblocks-collision-cleanup-wave4
Open

fix(intel): restore dead reachable-collision cleanup in getBlocks#194
r0ny123 wants to merge 1 commit into
danielplohmann:masterfrom
r0ny123:fix/audit-getblocks-collision-cleanup-wave4

Conversation

@r0ny123

@r0ny123 r0ny123 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Restores a dead cleanup step in FunctionAnalysisState.getBlocks() that a prior refactor accidentally made unreachable.

Motivation

A 2025-07-28 refactor collapsed two independent checks -- both originally gated by the same outer condition (current[0] in self.code_refs_from) -- into a single flattened boolean and-chain terminating in one break. That orphaned the second check, a "reachable collision" cleanup, as dead code sitting textually after the break: it removed a stale cross-function code reference and ended the block whenever the fall-through address was already known to belong to another function (state.addCollision(), raised by the recursive engine on a gap-function-revert collision). Verified via git show on the refactor commit that the pre-refactor code ran both checks independently.

Changed behavior

  • getBlocks() now removes the stale code reference and ends the block at the instruction whose fall-through collides with another function, instead of silently retaining the cross-function edge.
  • No change to the ordinary "jump reference points elsewhere" cut path, and no change to mnemonic-prefix normalization.
  • AArch64 inherits the fix automatically (its FunctionAnalysisState subclasses this one and does not override getBlocks()). CIL and Dalvik were swept and are unaffected: neither backend's disassembly driver ever calls addCollision(), so the corresponding cleanup is a structural no-op in both regardless of this change.

Accuracy evidence

Ran a before/after comparison on Bao byteweight msvc10-64 (15 binaries, same set both runs):

  • The cleanup path fires 27-35 times across the sample (not a rare edge case).
  • Function PPV/TPR/F1 identical before and after (99.48 / 99.72 / 99.60), and a full per-binary diff shows 0 of 15 predicted function sets differ.

Interpretation: this is a CFG-edge correctness fix (removes a stale cross-function reference), not a function-boundary change -- it fires often but does not alter which functions get recovered.

Validation

  • make lint
  • python -m ruff format --check .
  • make test -- 530 passed, 1 skipped
  • New regression test: tests/testIntelDisassembler.py::test_reachable_collision_ends_block_and_removes_code_ref (fails against the pre-fix flattened condition, passes against this fix)

No report schema, serialized field, hash format, or status value changes are introduced.

A 2025-07-28 refactor (0c8499b) collapsed two independent checks that were
both gated by `current[0] in self.code_refs_from` into a single flattened
and-chain with one break, orphaning the reachable-collision cleanup as
unreachable code sitting after the break. This dropped a real cleanup:
when a code ref's fall-through address is a known colliding address
(state.addCollision(), fired from RecursiveDisassembler on a gap-function-
revert collision), the stale cross-function code ref should be removed and
the block should end there.

Restores the original pre-refactor nesting while keeping the later
mnemonic-prefix normalization (LSN-009/LSN-010) intact.

Accuracy eval (Bao msvc10-64, 15 binaries, before/after): the cleanup path
fires 27-35 times but produces zero function-set deltas -- a CFG-edge
correctness fix, not a function-boundary change.

Pattern: PAT-SMDA-021
Instances: 1 real (this fix); aarch64 inherits it automatically; cil/dalvik
benign (colliding_addresses never populated in either backend)
Validation: make lint, make test (530 passed, 1 skipped)
@r0ny123
r0ny123 force-pushed the fix/audit-getblocks-collision-cleanup-wave4 branch from 5996d77 to 495368b Compare July 21, 2026 16:30
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.

1 participant