Skip to content

Commit

Permalink
Pass length to free flag conditions list callback and remove libc fro…
Browse files Browse the repository at this point in the history
…m rust api

Allows language bindings like rust to free conditions lists sanely

# Conflicts:
#	rust/Cargo.lock
  • Loading branch information
emesare committed Aug 1, 2024
1 parent ec43516 commit ce5995c
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 36 deletions.
1 change: 0 additions & 1 deletion arch/riscv/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion architecture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ BNFlagConditionForSemanticClass* Architecture::GetFlagConditionsForSemanticFlagG
}


void Architecture::FreeFlagConditionsForSemanticFlagGroupCallback(void*, BNFlagConditionForSemanticClass* conditions)
void Architecture::FreeFlagConditionsForSemanticFlagGroupCallback(void*, BNFlagConditionForSemanticClass* conditions, size_t)
{
delete[] conditions;
}
Expand Down
2 changes: 1 addition & 1 deletion binaryninjaapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -7743,7 +7743,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,
Expand Down
2 changes: 1 addition & 1 deletion binaryninjacore.h
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,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,
Expand Down
2 changes: 1 addition & 1 deletion python/architecture.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ noexports = []
[dependencies]
lazy_static = "1.4.0"
log = "0.4"
libc = "0.2"
rayon = { version = "1.8", optional = true }
binaryninjacore-sys = { path = "binaryninjacore-sys" }

Expand Down
38 changes: 17 additions & 21 deletions rust/src/architecture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1940,7 +1940,7 @@ where
None => BnString::new("invalid_flag_group").into_raw(),
}
}

extern "C" fn cb_registers_full_width<A>(ctxt: *mut c_void, count: *mut usize) -> *mut u32
where
A: 'static + Architecture<Handle = CustomArchitectureHandle<A>> + Send + Sync,
Expand Down Expand Up @@ -2101,7 +2101,7 @@ where

if let Some(group) = custom_arch.flag_group_from_id(group) {
let mut flags = group.flags_required();

// SAFETY: `count` is an out parameter
unsafe { *count = flags.len() };
let regs_ptr = flags.as_mut_ptr();
Expand All @@ -2127,23 +2127,13 @@ where

if let Some(group) = custom_arch.flag_group_from_id(group) {
let flag_conditions = group.flag_conditions();
let mut flags = flag_conditions.values().collect::<Vec<_>>();

unsafe {
let allocation_size =
mem::size_of::<BNFlagConditionForSemanticClass>() * 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 regs_ptr = flags.as_mut_ptr();
mem::forget(flags);
regs_ptr as *mut _
} else {
unsafe {
*count = 0;
Expand All @@ -2155,11 +2145,17 @@ where
extern "C" fn cb_free_flag_conditions_for_semantic_flag_group<A>(
_ctxt: *mut c_void,
conds: *mut BNFlagConditionForSemanticClass,
count: usize,
) where
A: 'static + Architecture<Handle = CustomArchitectureHandle<A>> + Send + Sync,
{
if conds.is_null() {
return;
}

unsafe {
libc::free(conds as *mut _);
let regs_ptr = ptr::slice_from_raw_parts_mut(conds, count);
let _regs = Box::from_raw(regs_ptr);
}
}

Expand All @@ -2175,7 +2171,7 @@ where

if let Some(write_type) = custom_arch.flag_write_from_id(write_type) {
let mut written = write_type.flags_written();

// SAFETY: `count` is an out parameter
unsafe { *count = written.len() };
let regs_ptr = written.as_mut_ptr();
Expand Down Expand Up @@ -2449,7 +2445,7 @@ where
{
let custom_arch = unsafe { &*(ctxt as *mut A) };
let mut intrinsics = custom_arch.intrinsics();

// SAFETY: Passed in to be written
unsafe { *count = intrinsics.len() };
let regs_ptr = intrinsics.as_mut_ptr();
Expand Down
5 changes: 3 additions & 2 deletions rust/src/filemetadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -224,7 +225,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(),
)
Expand Down Expand Up @@ -273,7 +274,7 @@ impl FileMetadata {
BNOpenExistingDatabaseWithProgress(
self.handle,
filename_ptr,
func as *mut libc::c_void,
func as *mut c_void,
Some(cb_progress_func),
)
},
Expand Down
11 changes: 5 additions & 6 deletions rust/src/interaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,15 @@

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;
use crate::rc::Ref;
use crate::string::{BnStrCompatible, BnString};

pub fn get_text_line_input(prompt: &str, title: &str) -> Option<String> {
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(
Expand Down Expand Up @@ -80,7 +79,7 @@ pub fn get_address_input(prompt: &str, title: &str) -> Option<u64> {
}

pub fn get_open_filename_input(prompt: &str, extension: &str) -> Option<PathBuf> {
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(
Expand All @@ -98,7 +97,7 @@ pub fn get_open_filename_input(prompt: &str, extension: &str) -> Option<PathBuf>
}

pub fn get_save_filename_input(prompt: &str, title: &str, default_name: &str) -> Option<PathBuf> {
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(
Expand All @@ -117,7 +116,7 @@ pub fn get_save_filename_input(prompt: &str, title: &str, default_name: &str) ->
}

pub fn get_directory_name_input(prompt: &str, default_name: &str) -> Option<PathBuf> {
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(
Expand Down
1 change: 0 additions & 1 deletion rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@
extern crate log;
#[doc(hidden)]
pub extern crate binaryninjacore_sys;
extern crate libc;
#[cfg(feature = "rayon")]
extern crate rayon;

Expand Down

1 comment on commit ce5995c

@emesare
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit message looks suspicious, when we force push on this branch again please fix.

Please sign in to comment.