[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

8 files changed, 39 insertions, 31 deletions

diff --git a/plugins/warp/src/cache/guid.rs b/plugins/warp/src/cache/guid.rs
index f1788a33..df215554 100644
--- a/plugins/warp/src/cache/guid.rs
+++ b/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> {
diff --git a/plugins/warp/src/lib.rs b/plugins/warp/src/lib.rs
index be6f8bc3..56915983 100644
--- a/plugins/warp/src/lib.rs
+++ b/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.
diff --git a/plugins/warp/src/plugin/debug.rs b/plugins/warp/src/plugin/debug.rs
index 24cdf1c3..399dba50 100644
--- a/plugins/warp/src/plugin/debug.rs
+++ b/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 {
diff --git a/plugins/warp/src/plugin/ffi.rs b/plugins/warp/src/plugin/ffi.rs
index d78ca3c5..c3a8051c 100644
--- a/plugins/warp/src/plugin/ffi.rs
+++ b/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,
     }
 }
 
diff --git a/plugins/warp/src/plugin/ffi/function.rs b/plugins/warp/src/plugin/ffi/function.rs
index 7563bd96..b37d613b 100644
--- a/plugins/warp/src/plugin/ffi/function.rs
+++ b/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]
diff --git a/plugins/warp/src/plugin/function.rs b/plugins/warp/src/plugin/function.rs
index 130e8582..a4fd2c10 100644
--- a/plugins/warp/src/plugin/function.rs
+++ b/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(),
diff --git a/plugins/warp/src/plugin/workflow.rs b/plugins/warp/src/plugin/workflow.rs
index 735706cf..8da69342 100644
--- a/plugins/warp/src/plugin/workflow.rs
+++ b/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(
diff --git a/plugins/warp/src/processor.rs b/plugins/warp/src/processor.rs
index 75e8569e..1611dff3 100644
--- a/plugins/warp/src/processor.rs
+++ b/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(