Move instruction filters out of basic_block_guid and make them public

emesare · emesare · commit 1a45f7a3cfb5 · 2025-02-02T22:10:59.000-05:00
Need to do this for exposing to FFI and things like render layers
diff --git a/plugins/warp/src/lib.rs b/plugins/warp/src/lib.rs
@@ -6,7 +6,7 @@ use binaryninja::architecture::{
     Architecture, ImplicitRegisterExtend, Register as BNRegister, RegisterInfo,
 };
 use binaryninja::basic_block::BasicBlock as BNBasicBlock;
-use binaryninja::binary_view::BinaryViewExt;
+use binaryninja::binary_view::{BinaryView, BinaryViewExt};
 use binaryninja::confidence::MAX_CONFIDENCE;
 use binaryninja::function::{Function as BNFunction, NativeBlock};
 use binaryninja::low_level_il::expression::{ExpressionHandler, LowLevelILExpressionKind};
@@ -18,6 +18,7 @@ use binaryninja::low_level_il::instruction::{
 };
 use binaryninja::low_level_il::{LowLevelILRegister, VisitorAction};
 use binaryninja::rc::Ref as BNRef;
+use std::ops::Range;
 use std::path::PathBuf;
 use warp::signature::basic_block::BasicBlockGUID;
 use warp::signature::function::constraints::FunctionConstraints;
@@ -80,15 +81,18 @@ pub fn function_guid<A: Architecture, M: FunctionMutability>(
     func: &BNFunction,
     llil: &LowLevelILFunction<A, M, NonSSA<RegularNonSSA>>,
 ) -> FunctionGUID {
+    // TODO: We might want to make this configurable, or otherwise _not_ retrieve from the view here.
+    let relocatable_regions = relocatable_regions(&func.view());
     let basic_blocks = sorted_basic_blocks(func);
     let basic_block_guids = basic_blocks
         .iter()
-        .map(|bb| basic_block_guid(bb, llil))
+        .map(|bb| basic_block_guid(&relocatable_regions, bb, llil))
         .collect::<Vec<_>>();
     FunctionGUID::from_basic_blocks(&basic_block_guids)
 }
 
 pub fn basic_block_guid<A: Architecture, M: FunctionMutability>(
+    relocatable_regions: &[Range<u64>],
     basic_block: &BNBasicBlock<NativeBlock>,
     llil: &LowLevelILFunction<A, M, NonSSA<RegularNonSSA>>,
 ) -> BasicBlockGUID {
@@ -97,67 +101,6 @@ pub fn basic_block_guid<A: Architecture, M: FunctionMutability>(
     let arch = func.arch();
     let max_instr_len = arch.max_instr_len();
 
-    // NOPs and useless moves are blacklisted to allow for hot-patchable functions.
-    let is_blacklisted_instr = |instr: &LowLevelILInstruction<A, M, NonSSA<RegularNonSSA>>| {
-        match instr.kind() {
-            LowLevelILInstructionKind::Nop(_) => true,
-            LowLevelILInstructionKind::SetReg(op) => {
-                match op.source_expr().kind() {
-                    LowLevelILExpressionKind::Reg(source_op)
-                        if op.dest_reg() == source_op.source_reg() =>
-                    {
-                        match op.dest_reg() {
-                            LowLevelILRegister::ArchReg(r) => {
-                                // If this register has no implicit extend then we can safely assume it's a NOP.
-                                // Ex. on x86_64 we don't want to remove `mov edi, edi` as it will zero the upper 32 bits.
-                                // Ex. on x86 we do want to remove `mov edi, edi` as it will not have a side effect like above.
-                                matches!(
-                                    r.info().implicit_extend(),
-                                    ImplicitRegisterExtend::NoExtend
-                                )
-                            }
-                            LowLevelILRegister::Temp(_) => false,
-                        }
-                    }
-                    _ => false,
-                }
-            }
-            _ => false,
-        }
-    };
-
-    let is_variant_instr = |instr: &LowLevelILInstruction<A, M, NonSSA<RegularNonSSA>>| {
-        let is_variant_expr = |expr: &LowLevelILExpressionKind<A, M, NonSSA<RegularNonSSA>>| {
-            // TODO: Checking the section here is slow, we should gather all section ranges outside of this.
-            match expr {
-                LowLevelILExpressionKind::ConstPtr(op)
-                    if !view.sections_at(op.value()).is_empty() =>
-                {
-                    // Constant Pointer must be in a section for it to be relocatable.
-                    // NOTE: We cannot utilize segments here as there will be a zero based segment.
-                    true
-                }
-                LowLevelILExpressionKind::ExternPtr(_) => true,
-                LowLevelILExpressionKind::Const(op) if !view.sections_at(op.value()).is_empty() => {
-                    // Constant value must be in a section for it to be relocatable.
-                    // NOTE: We cannot utilize segments here as there will be a zero based segment.
-                    true
-                }
-                _ => false,
-            }
-        };
-
-        // Visit instruction expressions looking for variant expression, [VisitorAction::Halt] means variant.
-        instr.visit_tree(&mut |expr| {
-            if is_variant_expr(&expr.kind()) {
-                // Found a variant expression
-                VisitorAction::Halt
-            } else {
-                VisitorAction::Descend
-            }
-        }) == VisitorAction::Halt
-    };
-
     let basic_block_range = basic_block.start_index()..basic_block.end_index();
     let mut basic_block_bytes = Vec::with_capacity(basic_block_range.count());
     for instr_addr in basic_block.into_iter() {
@@ -166,8 +109,8 @@ pub fn basic_block_guid<A: Architecture, M: FunctionMutability>(
             instr_bytes.truncate(instr_info.length);
             if let Some(instr_llil) = llil.instruction_at(instr_addr) {
                 // If instruction is blacklisted don't include the bytes.
-                if !is_blacklisted_instr(&instr_llil) {
-                    if is_variant_instr(&instr_llil) {
+                if !is_blacklisted_instruction(&instr_llil) {
+                    if is_variant_instruction(relocatable_regions, &instr_llil) {
                         // Found a variant instruction, mask off entire instruction.
                         instr_bytes.fill(0);
                     }
@@ -181,6 +124,96 @@ pub fn basic_block_guid<A: Architecture, M: FunctionMutability>(
     BasicBlockGUID::from(basic_block_bytes.as_slice())
 }
 
+/// Is the instruction not included in the masked byte sequence?
+///
+/// Blacklisted instructions will make an otherwise identical function GUID fail to match.
+///
+/// Example: NOPs and useless moves are blacklisted to allow for hot-patchable functions.
+pub fn is_blacklisted_instruction<A: Architecture, M: FunctionMutability>(
+    instr: &LowLevelILInstruction<A, M, NonSSA<RegularNonSSA>>,
+) -> bool {
+    match instr.kind() {
+        LowLevelILInstructionKind::Nop(_) => true,
+        LowLevelILInstructionKind::SetReg(op) => {
+            match op.source_expr().kind() {
+                LowLevelILExpressionKind::Reg(source_op)
+                    if op.dest_reg() == source_op.source_reg() =>
+                {
+                    match op.dest_reg() {
+                        LowLevelILRegister::ArchReg(r) => {
+                            // If this register has no implicit extend then we can safely assume it's a NOP.
+                            // Ex. on x86_64 we don't want to remove `mov edi, edi` as it will zero the upper 32 bits.
+                            // Ex. on x86 we do want to remove `mov edi, edi` as it will not have a side effect like above.
+                            matches!(r.info().implicit_extend(), ImplicitRegisterExtend::NoExtend)
+                        }
+                        LowLevelILRegister::Temp(_) => false,
+                    }
+                }
+                _ => false,
+            }
+        }
+        _ => false,
+    }
+}
+
+pub fn is_variant_instruction<A: Architecture, M: FunctionMutability>(
+    relocatable_regions: &[Range<u64>],
+    instr: &LowLevelILInstruction<A, M, NonSSA<RegularNonSSA>>,
+) -> bool {
+    let is_variant_expr = |expr: &LowLevelILExpressionKind<A, M, NonSSA<RegularNonSSA>>| {
+        match expr {
+            LowLevelILExpressionKind::ConstPtr(op)
+                if is_address_relocatable(relocatable_regions, op.value()) =>
+            {
+                // Constant Pointer must be in a section for it to be relocatable.
+                // NOTE: We cannot utilize segments here as there will be a zero based segment.
+                true
+            }
+            LowLevelILExpressionKind::Const(op)
+                if is_address_relocatable(relocatable_regions, op.value()) =>
+            {
+                // Constant value must be in a section for it to be relocatable.
+                // NOTE: We cannot utilize segments here as there will be a zero based segment.
+                true
+            }
+            LowLevelILExpressionKind::ExternPtr(_) => true,
+            _ => false,
+        }
+    };
+
+    // Visit instruction expressions looking for variant expression, [VisitorAction::Halt] means variant.
+    instr.visit_tree(&mut |expr| {
+        if is_variant_expr(&expr.kind()) {
+            // Found a variant expression.
+            VisitorAction::Halt
+        } else {
+            // Keep looking for a variant expression.
+            VisitorAction::Descend
+        }
+    }) == VisitorAction::Halt
+}
+
+/// If the address is inside any of the given ranges we will assume the address to be relocatable.
+pub fn is_address_relocatable(relocatable_regions: &[Range<u64>], address: u64) -> bool {
+    relocatable_regions
+        .iter()
+        .any(|range| range.contains(&address))
+}
+
+// TODO: This might need to be configurable, in that case we better remove this function.
+/// Get the relocatable regions of the view.
+///
+/// Currently, this is all the sections, however this might be refined later.
+pub fn relocatable_regions(view: &BinaryView) -> Vec<Range<u64>> {
+    view.sections()
+        .iter()
+        .map(|s| Range {
+            start: s.start(),
+            end: s.end(),
+        })
+        .collect()
+}
+
 #[cfg(test)]
 mod tests {
     use crate::cache::cached_function_guid;
diff --git a/plugins/warp/src/plugin/ffi.rs b/plugins/warp/src/plugin/ffi.rs
@@ -1,7 +1,7 @@
-use crate::basic_block_guid;
 use crate::cache::{cached_function_guid, insert_cached_function_match, try_cached_function_match};
 use crate::convert::{to_bn_symbol_at_address, to_bn_type};
 use crate::matcher::cached_possible_function_matches;
+use crate::{basic_block_guid, relocatable_regions};
 use binaryninja::basic_block::BasicBlock;
 use binaryninja::function::{Function, NativeBlock};
 use binaryninja::platform::Platform;
@@ -32,7 +32,9 @@ pub extern "C" fn BNWARPGetBasicBlockGUID(basic_block: *mut BNBasicBlock) -> *co
     let Ok(llil) = function.low_level_il() else {
         return std::ptr::null();
     };
-    let basic_block_guid = basic_block_guid(&basic_block, &llil);
+    // TODO: This should be the callers responsibility IMO to get relocatable ranges.
+    let relocatable_regions = relocatable_regions(&function.view());
+    let basic_block_guid = basic_block_guid(&relocatable_regions, &basic_block, &llil);
     let basic_block_guid_str = BnString::new(basic_block_guid.to_string());
     // NOTE: Leak the guid string to be freed by BNFreeString
     BnString::into_raw(basic_block_guid_str)
