WARP: Fix generating lifted IL when function GUID is already cached

We do not need to consult the lifted IL if we have already cached the function GUID in the function metadata

Author: emesare

plugins/warp/src/cache/guid.rs

@@ -4,28 +4,35 @@ use crate::function_guid;
 use binaryninja::binary_view::BinaryViewExt;
 use binaryninja::function::Function as BNFunction;
 use binaryninja::low_level_il::function::{FunctionMutability, LowLevelILFunction, NonSSA};
+use binaryninja::rc::Ref as BNRef;
 use binaryninja::symbol::Symbol as BNSymbol;
 use std::collections::HashSet;
 use uuid::Uuid;
 use warp::signature::constraint::Constraint;
 use warp::signature::function::FunctionGUID;
 
+/// Try to get the cached function GUID from the metadata.
+///
+/// If not cached, we will use `lifted_il_accessor` to retrieve the lifted IL and create the GUID.
+///
+/// `lifted_il_accessor` exists as it is to allow the retrieval of a cached GUID without incurring
+/// the cost of building the IL (if it no longer exists).
 pub fn cached_function_guid<M: FunctionMutability>(
     function: &BNFunction,
-    lifted_il: &LowLevelILFunction<M, NonSSA>,
-) -> FunctionGUID {
+    lifted_il_accessor: impl Fn() -> Option<BNRef<LowLevelILFunction<M, NonSSA>>>,
+) -> Option<FunctionGUID> {
     let cached_guid = try_cached_function_guid(function);
     if let Some(cached_guid) = cached_guid {
-        return cached_guid;
+        return Some(cached_guid);
     }
 
-    let function_guid = function_guid(function, lifted_il);
+    let function_guid = function_guid(function, lifted_il_accessor()?.as_ref());
     function.store_metadata(
         "warp_function_guid",
         &function_guid.as_bytes().to_vec(),
         false,
     );
-    function_guid
+    Some(function_guid)
 }
 
 pub fn try_cached_function_guid(function: &BNFunction) -> Option<FunctionGUID> {

plugins/warp/src/lib.rs

@@ -106,13 +106,18 @@ pub fn build_variables(func: &BNFunction) -> Vec<FunctionVariable> {
 }
 
 // TODO: Get rid of the minimal bool.
+/// Build the WARP [`Function`] from the Binary Ninja [`BNFunction`].
+///
+/// The `lifted_il_accessor` is passed in such that a function with a guid already cached will not
+/// require us to regenerate the IL. This is important in the event of someone generating signatures
+/// off of an existing BNDB or when the IL is no longer present.
 pub fn build_function<M: FunctionMutability>(
     func: &BNFunction,
-    lifted_il: &LowLevelILFunction<M, NonSSA>,
+    lifted_il_accessor: impl Fn() -> Option<BNRef<LowLevelILFunction<M, NonSSA>>>,
     minimal: bool,
-) -> Function {
+) -> Option<Function> {
     let mut function = Function {
-        guid: cached_function_guid(func, lifted_il),
+        guid: cached_function_guid(func, lifted_il_accessor)?,
         symbol: from_bn_symbol(&func.symbol()),
         // NOTE: Adding adjacent only works if analysis is complete.
         // NOTE: We do not filter out adjacent functions here.
@@ -123,7 +128,7 @@ pub fn build_function<M: FunctionMutability>(
     };
 
     if minimal {
-        return function;
+        return Some(function);
     }
 
     // Currently we only store the type if its a user type.
@@ -142,7 +147,7 @@ pub fn build_function<M: FunctionMutability>(
         .map(|c| bn_comment_to_comment(func, c))
         .collect();
     function.variables = build_variables(func);
-    function
+    Some(function)
 }
 
 /// Basic blocks sorted from high to low.

plugins/warp/src/plugin/debug.rs

@@ -9,9 +9,10 @@ pub struct DebugFunction;
 
 impl FunctionCommand for DebugFunction {
     fn action(&self, _view: &BinaryView, func: &Function) {
-        if let Ok(lifted_il) = func.lifted_il() {
-            log::info!("{:#?}", build_function(func, &lifted_il, false));
-        }
+        log::info!(
+            "{:#?}",
+            build_function(func, || func.lifted_il().ok(), false)
+        );
     }
 
     fn valid(&self, _view: &BinaryView, _func: &Function) -> bool {

plugins/warp/src/plugin/ffi.rs

@@ -124,12 +124,12 @@ pub unsafe extern "C" fn BNWARPGetAnalysisFunctionGUID(
     result: *mut BNWARPFunctionGUID,
 ) -> bool {
     let function = unsafe { Function::from_raw(analysis_function) };
-    match function.lifted_il() {
-        Ok(lifted_il) => {
-            *result = cached_function_guid(&function, &lifted_il);
+    match cached_function_guid(&function, || function.lifted_il().ok()) {
+        Some(guid) => {
+            *result = guid;
             true
         }
-        Err(_) => false,
+        None => false,
     }
 }
 

plugins/warp/src/plugin/ffi/function.rs

@@ -37,11 +37,10 @@ pub unsafe extern "C" fn BNWARPGetFunction(
     analysis_function: *mut BNFunction,
 ) -> *mut BNWARPFunction {
     let function = Function::from_raw(analysis_function);
-    let Ok(lifted_il) = function.lifted_il() else {
-        return std::ptr::null_mut();
-    };
-    let function = build_function(&function, &lifted_il, false);
-    Arc::into_raw(Arc::new(function)) as *mut BNWARPFunction
+    match build_function(&function, || function.lifted_il().ok(), false) {
+        Some(function) => Arc::into_raw(Arc::new(function)) as *mut BNWARPFunction,
+        None => std::ptr::null_mut(),
+    }
 }
 
 #[no_mangle]

plugins/warp/src/plugin/function.rs

@@ -47,11 +47,10 @@ pub struct CopyFunctionGUID;
 
 impl FunctionCommand for CopyFunctionGUID {
     fn action(&self, _view: &BinaryView, func: &Function) {
-        let Ok(lifted_il) = func.lifted_il() else {
-            log::error!("Could not get lifted il for copied function");
+        let Some(guid) = cached_function_guid(func, || func.lifted_il().ok()) else {
+            log::error!("Could not get guid for copied function");
             return;
         };
-        let guid = cached_function_guid(func, &lifted_il);
         log::info!(
             "Function GUID for {:?}... {}",
             func.symbol().short_name(),

plugins/warp/src/plugin/workflow.rs

@@ -220,9 +220,7 @@ pub fn insert_workflow() -> Result<(), ()> {
 
     let guid_activity = |ctx: &AnalysisContext| {
         let function = ctx.function();
-        if let Some(lifted_il) = unsafe { ctx.lifted_il_function() } {
-            cached_function_guid(&function, &lifted_il);
-        }
+        cached_function_guid(&function, || unsafe { ctx.lifted_il_function() });
     };
 
     let guid_config = activity::Config::action(

plugins/warp/src/processor.rs

@@ -814,13 +814,12 @@ impl WarpFileProcessor {
             .filter(is_function_named)
             .filter(|f| !f.analysis_skipped())
             .filter_map(|func| {
-                let lifted_il = func.lifted_il().ok()?;
                 let target = platform_to_target(&func.platform());
                 let built_function = build_function(
                     &func,
-                    &lifted_il,
+                    || func.lifted_il().ok(),
                     self.file_data == FileDataKindField::Symbols,
-                );
+                )?;
                 Some((target, built_function))
             })
             .fold(