diff --git a/plugins/warp/src/cache/function.rs b/plugins/warp/src/cache/function.rs index 2fef05fdab..a953f51453 100644 --- a/plugins/warp/src/cache/function.rs +++ b/plugins/warp/src/cache/function.rs @@ -1,17 +1,42 @@ +use crate::convert::{comment_to_bn_comment, to_bn_symbol_at_address}; +use binaryninja::binary_view::BinaryViewExt; use binaryninja::function::{Function as BNFunction, FunctionUpdateType}; +use binaryninja::symbol::SymbolType; use warp::signature::function::Function; -/// Inserts a function match into the cache. +// TODO: Rename this? +/// Inserts a function match into the cache. This also has the side effect of setting persisted function +/// information, such as the matched function symbol. /// /// IMPORTANT: This will mark the function as needing updates, if you intend to fill in functions with /// no match (i.e. `None`), then you must change this function to prevent marking that as needing updates. /// However, it's perfectly valid to remove a match and need to update the function still, so be careful. pub fn insert_cached_function_match(function: &BNFunction, matched_function: Option) { + let view = function.view(); + let function_start = function.start(); // NOTE: If we expect to run match_function multiple times on a function, we should move this elsewhere. // Mark the function as needing updates so that reanalysis occurs on the function, and we apply the match. function.mark_updates_required(FunctionUpdateType::FullAutoFunctionUpdate); + if let Some(auto_sym) = view.symbol_by_address(function_start) { + // TODO: If we ever create non library function symbols we will need to remove this check (see: `to_bn_symbol_at_address`). + if auto_sym.sym_type() == SymbolType::LibraryFunction { + // NOTE: This will also mark for full auto function update, one thing to note is that the + // requirement to call this is that this function not be called in the associated function's analysis. + view.undefine_auto_symbol(&auto_sym); + } + } match matched_function { Some(matched_function) => { + // Define the new matched function symbol, this can safely be done here as the symbol itself + // will be persisted, unlike function type information or variable information. + let new_sym = to_bn_symbol_at_address(&view, &matched_function.symbol, function_start); + view.define_auto_symbol(&new_sym); + // TODO: How to clear the comments? They are just persisted. + // TODO: Also they generate an undo action, i hate implicit undo actions so much. + for comment in &matched_function.comments { + let bn_comment = comment_to_bn_comment(&function, comment.clone()); + function.set_comment_at(bn_comment.addr, &bn_comment.comment); + } function.store_metadata("warp_matched_function", &matched_function.to_bytes(), false); } None => { diff --git a/plugins/warp/src/plugin/ffi/function.rs b/plugins/warp/src/plugin/ffi/function.rs index a7ec6593ac..7563bd969f 100644 --- a/plugins/warp/src/plugin/ffi/function.rs +++ b/plugins/warp/src/plugin/ffi/function.rs @@ -67,7 +67,7 @@ pub unsafe extern "C" fn BNWARPFunctionApply( let analysis_function = Function::from_raw(analysis_function); match function.is_null() { false => { - // Set the matched function to `function` and return previous. + // Set the matched function to `function`. let matched_function = ManuallyDrop::new(Arc::from_raw(function)); insert_cached_function_match( &analysis_function, @@ -75,7 +75,7 @@ pub unsafe extern "C" fn BNWARPFunctionApply( ) } true => { - // We are removing the previous match and returning it. + // We are removing the previous match. insert_cached_function_match(&analysis_function, None) } }; diff --git a/plugins/warp/src/plugin/workflow.rs b/plugins/warp/src/plugin/workflow.rs index 624fe0ef96..2a8f6287eb 100644 --- a/plugins/warp/src/plugin/workflow.rs +++ b/plugins/warp/src/plugin/workflow.rs @@ -3,9 +3,7 @@ use crate::cache::{ cached_function_guid, insert_cached_function_match, try_cached_function_guid, try_cached_function_match, }; -use crate::convert::{ - comment_to_bn_comment, platform_to_target, to_bn_symbol_at_address, to_bn_type, -}; +use crate::convert::{platform_to_target, to_bn_type}; use crate::matcher::{Matcher, MatcherSettings}; use crate::{get_warp_tag_type, relocatable_regions}; use binaryninja::architecture::RegisterId; @@ -192,16 +190,13 @@ pub fn run_matcher(view: &BinaryView) { } pub fn insert_workflow() { + // TODO: Note: because of symbol persistence function symbol is applied in `insert_cached_function_match`. + // TODO: Comments are also applied there, they are "user" like, persisted and make undo actions. // "Hey look, it's a plier" ~ Josh 2025 let apply_activity = |ctx: &AnalysisContext| { let view = ctx.view(); let function = ctx.function(); if let Some(matched_function) = try_cached_function_match(&function) { - view.define_auto_symbol(&to_bn_symbol_at_address( - &view, - &matched_function.symbol, - function.symbol().address(), - )); // core.function.propagateAnalysis will assign user type info to auto, so we must not apply // otherwise we will wipe over user type info. if !function.has_user_type() { @@ -209,12 +204,6 @@ pub fn insert_workflow() { function.set_auto_type(&to_bn_type(&function.arch(), func_ty)); } } - // TODO: How to clear the comments? They are just persisted. - // TODO: Also they generate an undo action, i hate implicit undo actions so much. - for comment in matched_function.comments { - let bn_comment = comment_to_bn_comment(&function, comment); - function.set_comment_at(bn_comment.addr, &bn_comment.comment); - } if let Some(mlil) = ctx.mlil_function() { for variable in matched_function.variables { let decl_addr = ((function.start() as i64) + variable.offset) as u64;