Fix 3 bugs: backward fixed-point, PPC calling convention, ashr >64-bit#125
Merged
Conversation
…4-bit - Backward fixed-point analysis started from CFG entry instead of exit, causing it to visit only 1-2 locations instead of the entire function. Added FixedPointRequiresExit error variant for clarity. - PPC calling convention had return_address_type (lr) and return_register (r3) swapped, misidentifying branch targets and return value data flow. - Constant::ashr sign-extension used u64::MAX (64 1-bits) which is insufficient for >64-bit values; now scales to any bit width. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second-pass code review of the Falcon binary analysis framework, finding 3 bugs with test coverage for each.
fixed_point.rs:157):fixed_point_backward_optionscalled.entry()on the CFG instead of.exit(), causing backward analysis to visit only the entry block (which has no predecessors) and terminate immediately. The variable was correctly namedexit_index— the author intended.exit(). AddedError::FixedPointRequiresExitvariant for the correct error message.calling_convention.rs:380,389):return_address_typewas set tor3andreturn_registertolr, but per PPC32 SVR4 ABI,lrholds the return address andr3holds the return value. This would cause analysis to misidentify return branch targets and miss return value data flow.Constant::ashrincorrect for >64-bit values (constant.rs:309): Sign-extension fill mask was hardcoded tou64::MAX(64 1-bits). For 128-bit values shifted right by more than 64, the fill pattern was too short, leaving upper bits as 0 instead of 1. Now constructs a properly-sized all-ones mask matching the value's bit width, consistent with howConstant::sexthandles this.Test plan
backward_analysis_starts_from_exit— builds a 2-block CFG (entry→exit), runs backward analysis, asserts exit block is visitedppc_return_address_is_lr— creates PPC calling convention, asserts return address register islrand return value register isr3constant_ashr_128bit— shifts0x80...0:128right by 96, asserts correct 96-bit sign extensioncargo test)🤖 Generated with Claude Code