From b34ddd03f71c8637a80a264a3ea757b8295d482b Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Thu, 1 Aug 2024 17:03:39 -0400 Subject: [PATCH] Pass length to free flag conditions list callback and remove libc from rust api Allows language bindings like rust to free conditions lists sanely --- arch/msp430/Cargo.lock | 1 - arch/riscv/Cargo.lock | 1 - architecture.cpp | 2 +- binaryninjaapi.h | 2 +- binaryninjacore.h | 6 +- python/architecture.py | 2 +- rust/Cargo.lock | 1 - rust/Cargo.toml | 1 - rust/src/architecture.rs | 141 ++++++++++++++++++++-------------- rust/src/callingconvention.rs | 52 +++++++++---- rust/src/filemetadata.rs | 5 +- rust/src/interaction.rs | 11 ++- rust/src/lib.rs | 1 - view/bintxt/Cargo.lock | 1 - 14 files changed, 134 insertions(+), 93 deletions(-) diff --git a/arch/msp430/Cargo.lock b/arch/msp430/Cargo.lock index a55d6aab8..515b6ccfe 100644 --- a/arch/msp430/Cargo.lock +++ b/arch/msp430/Cargo.lock @@ -26,7 +26,6 @@ version = "0.1.0" dependencies = [ "binaryninjacore-sys", "lazy_static", - "libc", "log", ] diff --git a/arch/riscv/Cargo.lock b/arch/riscv/Cargo.lock index e25009e23..de9ca95e8 100644 --- a/arch/riscv/Cargo.lock +++ b/arch/riscv/Cargo.lock @@ -33,7 +33,6 @@ version = "0.1.0" dependencies = [ "binaryninjacore-sys", "lazy_static", - "libc", "log", "rayon", ] diff --git a/architecture.cpp b/architecture.cpp index 75ab3d0a3..450466da1 100644 --- a/architecture.cpp +++ b/architecture.cpp @@ -477,7 +477,7 @@ BNFlagConditionForSemanticClass* Architecture::GetFlagConditionsForSemanticFlagG } -void Architecture::FreeFlagConditionsForSemanticFlagGroupCallback(void*, BNFlagConditionForSemanticClass* conditions) +void Architecture::FreeFlagConditionsForSemanticFlagGroupCallback(void*, BNFlagConditionForSemanticClass* conditions, size_t) { delete[] conditions; } diff --git a/binaryninjaapi.h b/binaryninjaapi.h index c857919c3..e553aa48e 100644 --- a/binaryninjaapi.h +++ b/binaryninjaapi.h @@ -7931,7 +7931,7 @@ namespace BinaryNinja { static BNFlagConditionForSemanticClass* GetFlagConditionsForSemanticFlagGroupCallback( void* ctxt, uint32_t semGroup, size_t* count); static void FreeFlagConditionsForSemanticFlagGroupCallback( - void* ctxt, BNFlagConditionForSemanticClass* conditions); + void* ctxt, BNFlagConditionForSemanticClass* conditions, size_t count); static uint32_t* GetFlagsWrittenByFlagWriteTypeCallback(void* ctxt, uint32_t writeType, size_t* count); static uint32_t GetSemanticClassForFlagWriteTypeCallback(void* ctxt, uint32_t writeType); static size_t GetFlagWriteLowLevelILCallback(void* ctxt, BNLowLevelILOperation op, size_t size, diff --git a/binaryninjacore.h b/binaryninjacore.h index 8dcab8160..5e090d95f 100644 --- a/binaryninjacore.h +++ b/binaryninjacore.h @@ -37,14 +37,14 @@ // Current ABI version for linking to the core. This is incremented any time // there are changes to the API that affect linking, including new functions, // new types, or modifications to existing functions or types. -#define BN_CURRENT_CORE_ABI_VERSION 85 +#define BN_CURRENT_CORE_ABI_VERSION 86 // Minimum ABI version that is supported for loading of plugins. Plugins that // are linked to an ABI version less than this will not be able to load and // will require rebuilding. The minimum version is increased when there are // incompatible changes that break binary compatibility, such as changes to // existing types or functions. -#define BN_MINIMUM_CORE_ABI_VERSION 82 +#define BN_MINIMUM_CORE_ABI_VERSION 86 #ifdef __GNUC__ #ifdef BINARYNINJACORE_LIBRARY @@ -1854,7 +1854,7 @@ extern "C" uint32_t* (*getFlagsRequiredForSemanticFlagGroup)(void* ctxt, uint32_t semGroup, size_t* count); BNFlagConditionForSemanticClass* (*getFlagConditionsForSemanticFlagGroup)( void* ctxt, uint32_t semGroup, size_t* count); - void (*freeFlagConditionsForSemanticFlagGroup)(void* ctxt, BNFlagConditionForSemanticClass* conditions); + void (*freeFlagConditionsForSemanticFlagGroup)(void* ctxt, BNFlagConditionForSemanticClass* conditions, size_t count); uint32_t* (*getFlagsWrittenByFlagWriteType)(void* ctxt, uint32_t writeType, size_t* count); uint32_t (*getSemanticClassForFlagWriteType)(void* ctxt, uint32_t writeType); size_t (*getFlagWriteLowLevelIL)(void* ctxt, BNLowLevelILOperation op, size_t size, uint32_t flagWriteType, diff --git a/python/architecture.py b/python/architecture.py index abcfb0156..01ea3166a 100644 --- a/python/architecture.py +++ b/python/architecture.py @@ -914,7 +914,7 @@ def _get_flag_conditions_for_semantic_flag_group(self, ctxt, sem_group, count): count[0] = 0 return None - def _free_flag_conditions_for_semantic_flag_group(self, ctxt, conditions): + def _free_flag_conditions_for_semantic_flag_group(self, ctxt, conditions, count): try: buf = ctypes.cast(conditions, ctypes.c_void_p) if buf.value not in self._pending_condition_lists: diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 682aae2fa..4563bbff4 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -123,7 +123,6 @@ version = "0.1.0" dependencies = [ "binaryninjacore-sys", "lazy_static", - "libc", "log", "rayon", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 4f4703a39..ff016274b 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,6 @@ noexports = [] [dependencies] lazy_static = "1.4.0" log = { version = "0.4", features = ["std"] } -libc = "0.2" rayon = { version = "1.8", optional = true } binaryninjacore-sys = { path = "binaryninjacore-sys" } diff --git a/rust/src/architecture.rs b/rust/src/architecture.rs index e91925b77..027af77fd 100644 --- a/rust/src/architecture.rs +++ b/rust/src/architecture.rs @@ -1941,19 +1941,23 @@ where None => BnString::new("invalid_flag_group").into_raw(), } } - + extern "C" fn cb_registers_full_width(ctxt: *mut c_void, count: *mut usize) -> *mut u32 where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut regs = custom_arch.registers_full_width(); + let mut regs: Vec<_> = custom_arch + .registers_full_width() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: `count` is an out parameter unsafe { *count = regs.len() }; let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr } extern "C" fn cb_registers_all(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -1961,13 +1965,13 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut regs = custom_arch.registers_all(); + let mut regs: Vec<_> = custom_arch.registers_all().iter().map(|r| r.id()).collect(); // SAFETY: `count` is an out parameter unsafe { *count = regs.len() }; let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr } extern "C" fn cb_registers_global(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -1975,13 +1979,17 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut regs = custom_arch.registers_global(); + let mut regs: Vec<_> = custom_arch + .registers_global() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: `count` is an out parameter unsafe { *count = regs.len() }; let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr } extern "C" fn cb_registers_system(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -1989,13 +1997,17 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut regs = custom_arch.registers_system(); + let mut regs: Vec<_> = custom_arch + .registers_system() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: `count` is an out parameter unsafe { *count = regs.len() }; let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr } extern "C" fn cb_flags(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -2003,13 +2015,13 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut flags = custom_arch.flags(); + let mut flags: Vec<_> = custom_arch.flags().iter().map(|f| f.id()).collect(); // SAFETY: `count` is an out parameter unsafe { *count = flags.len() }; - let regs_ptr = flags.as_mut_ptr(); + let flags_ptr = flags.as_mut_ptr(); mem::forget(flags); - regs_ptr as *mut _ + flags_ptr } extern "C" fn cb_flag_write_types(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -2017,13 +2029,17 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut flag_writes = custom_arch.flag_write_types(); + let mut flag_writes: Vec<_> = custom_arch + .flag_write_types() + .iter() + .map(|f| f.id()) + .collect(); // SAFETY: `count` is an out parameter unsafe { *count = flag_writes.len() }; - let regs_ptr = flag_writes.as_mut_ptr(); + let flags_ptr = flag_writes.as_mut_ptr(); mem::forget(flag_writes); - regs_ptr as *mut _ + flags_ptr } extern "C" fn cb_semantic_flag_classes(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -2031,13 +2047,13 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut flag_classes = custom_arch.flag_classes(); + let mut flag_classes: Vec<_> = custom_arch.flag_classes().iter().map(|f| f.id()).collect(); // SAFETY: `count` is an out parameter unsafe { *count = flag_classes.len() }; - let regs_ptr = flag_classes.as_mut_ptr(); + let flags_ptr = flag_classes.as_mut_ptr(); mem::forget(flag_classes); - regs_ptr as *mut _ + flags_ptr } extern "C" fn cb_semantic_flag_groups(ctxt: *mut c_void, count: *mut usize) -> *mut u32 @@ -2045,13 +2061,13 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut flag_groups = custom_arch.flag_groups(); + let mut flag_groups: Vec<_> = custom_arch.flag_groups().iter().map(|f| f.id()).collect(); // SAFETY: `count` is an out parameter unsafe { *count = flag_groups.len() }; - let regs_ptr = flag_groups.as_mut_ptr(); + let flags_ptr = flag_groups.as_mut_ptr(); mem::forget(flag_groups); - regs_ptr as *mut _ + flags_ptr } extern "C" fn cb_flag_role(ctxt: *mut c_void, flag: u32, class: u32) -> BNFlagRole @@ -2081,13 +2097,17 @@ where { let custom_arch = unsafe { &*(ctxt as *mut A) }; let class = custom_arch.flag_class_from_id(class); - let mut flags = custom_arch.flags_required_for_flag_condition(cond, class); + let mut flags: Vec<_> = custom_arch + .flags_required_for_flag_condition(cond, class) + .iter() + .map(|f| f.id()) + .collect(); // SAFETY: `count` is an out parameter unsafe { *count = flags.len() }; - let regs_ptr = flags.as_mut_ptr(); + let flags_ptr = flags.as_mut_ptr(); mem::forget(flags); - regs_ptr as *mut _ + flags_ptr } extern "C" fn cb_flags_required_for_semantic_flag_group( @@ -2101,13 +2121,13 @@ where let custom_arch = unsafe { &*(ctxt as *mut A) }; if let Some(group) = custom_arch.flag_group_from_id(group) { - let mut flags = group.flags_required(); - + let mut flags: Vec<_> = group.flags_required().iter().map(|f| f.id()).collect(); + // SAFETY: `count` is an out parameter unsafe { *count = flags.len() }; - let regs_ptr = flags.as_mut_ptr(); + let flags_ptr = flags.as_mut_ptr(); mem::forget(flags); - regs_ptr as *mut _ + flags_ptr } else { unsafe { *count = 0; @@ -2128,23 +2148,19 @@ where if let Some(group) = custom_arch.flag_group_from_id(group) { let flag_conditions = group.flag_conditions(); + let mut flags = flag_conditions + .iter() + .map(|(&class, &condition)| BNFlagConditionForSemanticClass { + semanticClass: class.id(), + condition, + }) + .collect::>(); - unsafe { - let allocation_size = - mem::size_of::() * flag_conditions.len(); - let result = libc::malloc(allocation_size) as *mut BNFlagConditionForSemanticClass; - let out_slice = slice::from_raw_parts_mut(result, flag_conditions.len()); - - for (i, (class, cond)) in flag_conditions.iter().enumerate() { - let out = out_slice.get_unchecked_mut(i); - - out.semanticClass = class.id(); - out.condition = *cond; - } - - *count = flag_conditions.len(); - result - } + // SAFETY: `count` is an out parameter + unsafe { *count = flags.len() }; + let flags_ptr = flags.as_mut_ptr(); + mem::forget(flags); + flags_ptr } else { unsafe { *count = 0; @@ -2156,11 +2172,17 @@ where extern "C" fn cb_free_flag_conditions_for_semantic_flag_group( _ctxt: *mut c_void, conds: *mut BNFlagConditionForSemanticClass, + count: usize, ) where A: 'static + Architecture> + Send + Sync, { + if conds.is_null() { + return; + } + unsafe { - libc::free(conds as *mut _); + let flags_ptr = ptr::slice_from_raw_parts_mut(conds, count); + let _flags = Box::from_raw(flags_ptr); } } @@ -2175,13 +2197,14 @@ where let custom_arch = unsafe { &*(ctxt as *mut A) }; if let Some(write_type) = custom_arch.flag_write_from_id(write_type) { - let mut written = write_type.flags_written(); - + let mut flags_written: Vec<_> = + write_type.flags_written().iter().map(|f| f.id()).collect(); + // SAFETY: `count` is an out parameter - unsafe { *count = written.len() }; - let regs_ptr = written.as_mut_ptr(); - mem::forget(written); - regs_ptr as *mut _ + unsafe { *count = flags_written.len() }; + let flags_ptr = flags_written.as_mut_ptr(); + mem::forget(flags_written); + flags_ptr } else { unsafe { *count = 0; @@ -2387,13 +2410,17 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut regs = custom_arch.register_stacks(); + let mut regs: Vec<_> = custom_arch + .register_stacks() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: Passed in to be written unsafe { *count = regs.len() }; let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr } extern "C" fn cb_reg_stack_info( @@ -2449,13 +2476,13 @@ where A: 'static + Architecture> + Send + Sync, { let custom_arch = unsafe { &*(ctxt as *mut A) }; - let mut intrinsics = custom_arch.intrinsics(); - + let mut intrinsics: Vec<_> = custom_arch.intrinsics().iter().map(|i| i.id()).collect(); + // SAFETY: Passed in to be written unsafe { *count = intrinsics.len() }; - let regs_ptr = intrinsics.as_mut_ptr(); + let intrinsics_ptr = intrinsics.as_mut_ptr(); mem::forget(intrinsics); - regs_ptr as *mut _ + intrinsics_ptr } extern "C" fn cb_intrinsic_inputs( diff --git a/rust/src/callingconvention.rs b/rust/src/callingconvention.rs index 915bdcc21..37c5012fb 100644 --- a/rust/src/callingconvention.rs +++ b/rust/src/callingconvention.rs @@ -97,13 +97,18 @@ where { ffi_wrap!("CallingConvention::caller_saved_registers", unsafe { let ctxt = &*(ctxt as *mut CustomCallingConventionContext); - let mut regs = ctxt.cc.caller_saved_registers(); + let mut regs: Vec<_> = ctxt + .cc + .caller_saved_registers() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: `count` is an out parameter - unsafe { *count = regs.len() }; + *count = regs.len(); let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr }) } @@ -113,13 +118,18 @@ where { ffi_wrap!("CallingConvention::callee_saved_registers", unsafe { let ctxt = &*(ctxt as *mut CustomCallingConventionContext); - let mut regs = ctxt.cc.callee_saved_registers(); - + let mut regs: Vec<_> = ctxt + .cc + .callee_saved_registers() + .iter() + .map(|r| r.id()) + .collect(); + // SAFETY: `count` is an out parameter - unsafe { *count = regs.len() }; + *count = regs.len(); let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr }) } @@ -129,13 +139,13 @@ where { ffi_wrap!("CallingConvention::int_arg_registers", unsafe { let ctxt = &*(ctxt as *mut CustomCallingConventionContext); - let mut regs = ctxt.cc.int_arg_registers(); + let mut regs: Vec<_> = ctxt.cc.int_arg_registers().iter().map(|r| r.id()).collect(); // SAFETY: `count` is an out parameter - unsafe { *count = regs.len() }; + *count = regs.len(); let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr }) } @@ -145,13 +155,18 @@ where { ffi_wrap!("CallingConvention::float_arg_registers", unsafe { let ctxt = &*(ctxt as *mut CustomCallingConventionContext); - let mut regs = ctxt.cc.float_arg_registers(); + let mut regs: Vec<_> = ctxt + .cc + .float_arg_registers() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: `count` is an out parameter - unsafe { *count = regs.len() }; + *count = regs.len(); let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr }) } @@ -267,13 +282,18 @@ where { ffi_wrap!("CallingConvention::implicitly_defined_registers", unsafe { let ctxt = &*(ctxt as *mut CustomCallingConventionContext); - let mut regs = ctxt.cc.implicitly_defined_registers(); + let mut regs: Vec<_> = ctxt + .cc + .implicitly_defined_registers() + .iter() + .map(|r| r.id()) + .collect(); // SAFETY: `count` is an out parameter - unsafe { *count = regs.len() }; + *count = regs.len(); let regs_ptr = regs.as_mut_ptr(); mem::forget(regs); - regs_ptr as *mut _ + regs_ptr }) } diff --git a/rust/src/filemetadata.rs b/rust/src/filemetadata.rs index c48bc968e..15a7059d4 100644 --- a/rust/src/filemetadata.rs +++ b/rust/src/filemetadata.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::ffi::c_void; use binaryninjacore_sys::{BNBeginUndoActions, BNCloseFile, BNCommitUndoActions, BNCreateDatabase, BNCreateFileMetadata, BNFileMetadata, BNFileMetadataGetSessionId, BNFreeFileMetadata, BNGetCurrentOffset, BNGetCurrentView, BNGetFileMetadataDatabase, BNGetFileViewOfType, BNGetFilename, BNGetProjectFile, BNIsAnalysisChanged, BNIsBackedByDatabase, BNIsFileModified, BNMarkFileModified, BNMarkFileSaved, BNNavigate, BNNewFileReference, BNOpenDatabaseForConfiguration, BNOpenExistingDatabase, BNRedo, BNRevertUndoActions, BNSaveAutoSnapshot, BNSetFilename, BNUndo}; use binaryninjacore_sys::{BNCreateDatabaseWithProgress, BNOpenExistingDatabaseWithProgress}; @@ -210,7 +211,7 @@ impl FileMetadata { BNCreateDatabaseWithProgress( handle, filename_ptr, - func as *mut libc::c_void, + func as *mut c_void, Some(cb_progress_func), ptr::null_mut(), ) @@ -259,7 +260,7 @@ impl FileMetadata { BNOpenExistingDatabaseWithProgress( self.handle, filename_ptr, - func as *mut libc::c_void, + func as *mut c_void, Some(cb_progress_func), ) }, diff --git a/rust/src/interaction.rs b/rust/src/interaction.rs index 8e437d3ea..e00908320 100644 --- a/rust/src/interaction.rs +++ b/rust/src/interaction.rs @@ -16,8 +16,7 @@ use binaryninjacore_sys::*; -use std::ffi::CStr; -use std::os::raw::{c_char, c_void}; +use std::ffi::{c_char, c_void, CStr}; use std::path::PathBuf; use crate::binaryview::BinaryView; @@ -25,7 +24,7 @@ use crate::rc::Ref; use crate::string::{BnStrCompatible, BnString}; pub fn get_text_line_input(prompt: &str, title: &str) -> Option { - let mut value: *mut libc::c_char = std::ptr::null_mut(); + let mut value: *mut c_char = std::ptr::null_mut(); let result = unsafe { BNGetTextLineInput( @@ -80,7 +79,7 @@ pub fn get_address_input(prompt: &str, title: &str) -> Option { } pub fn get_open_filename_input(prompt: &str, extension: &str) -> Option { - let mut value: *mut libc::c_char = std::ptr::null_mut(); + let mut value: *mut c_char = std::ptr::null_mut(); let result = unsafe { BNGetOpenFileNameInput( @@ -98,7 +97,7 @@ pub fn get_open_filename_input(prompt: &str, extension: &str) -> Option } pub fn get_save_filename_input(prompt: &str, extension: &str, default_name: &str) -> Option { - let mut value: *mut libc::c_char = std::ptr::null_mut(); + let mut value: *mut c_char = std::ptr::null_mut(); let result = unsafe { BNGetSaveFileNameInput( @@ -117,7 +116,7 @@ pub fn get_save_filename_input(prompt: &str, extension: &str, default_name: &str } pub fn get_directory_name_input(prompt: &str, default_name: &str) -> Option { - let mut value: *mut libc::c_char = std::ptr::null_mut(); + let mut value: *mut c_char = std::ptr::null_mut(); let result = unsafe { BNGetDirectoryNameInput( diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 272af9262..eff9518d2 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -106,7 +106,6 @@ extern crate log; #[doc(hidden)] pub extern crate binaryninjacore_sys; -extern crate libc; #[cfg(feature = "rayon")] extern crate rayon; diff --git a/view/bintxt/Cargo.lock b/view/bintxt/Cargo.lock index 6b79bd48c..faf8fc998 100644 --- a/view/bintxt/Cargo.lock +++ b/view/bintxt/Cargo.lock @@ -17,7 +17,6 @@ version = "0.1.0" dependencies = [ "binaryninjacore-sys", "lazy_static", - "libc", "log", ]