Skip to content

Commit

Permalink
More rust cleanup
Browse files Browse the repository at this point in the history
- Fixed progress executor freeing itself after one iteration
- Updated the last of the doc imports
- Moved mainthread to main_thread
- Made project creation and opening failable

We could probably AsRef<ProgressExecutor> to get around the allocation and free inside the function bodies, not high priority as those functions are long running anyways.
  • Loading branch information
emesare committed Jan 18, 2025
1 parent 90e925a commit f741ac9
Show file tree
Hide file tree
Showing 19 changed files with 138 additions and 75 deletions.
2 changes: 1 addition & 1 deletion rust/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ To specify a specific commit see the cargo documentation [here](https://doc.rust

```rust
use binaryninja::headless::Session;
use binaryninja::binaryview::{BinaryViewBase, BinaryViewExt};
use binaryninja::binary_view::{BinaryViewBase, BinaryViewExt};

fn main() {
let headless_session = Session::new().expect("Failed to initialize session");
Expand Down
12 changes: 9 additions & 3 deletions rust/src/binary_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use crate::types::{
use crate::variable::DataVariable;
use crate::Endianness;
use std::collections::HashMap;
use std::ffi::c_char;
use std::ffi::{c_char, c_void};
use std::ops::Range;
use std::path::Path;
use std::ptr::NonNull;
Expand Down Expand Up @@ -645,17 +645,20 @@ pub trait BinaryViewExt: BinaryViewBase {
.collect();
let mut result_ids: *mut *mut c_char = std::ptr::null_mut();
let mut result_names: *mut BNQualifiedName = std::ptr::null_mut();
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let result_count = unsafe {
BNDefineAnalysisTypes(
self.as_ref().handle,
types.as_mut_ptr(),
types.len(),
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
&mut result_ids as *mut _,
&mut result_names as *mut _,
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };

for ty in types {
QualifiedNameTypeAndId::free_raw(ty);
Expand Down Expand Up @@ -690,15 +693,18 @@ pub trait BinaryViewExt: BinaryViewBase {
.map(Into::into)
.map(QualifiedNameAndType::into_raw)
.collect();
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
unsafe {
BNDefineUserAnalysisTypes(
self.as_ref().handle,
types.as_mut_ptr(),
types.len(),
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
for ty in types {
QualifiedNameAndType::free_raw(ty);
}
Expand Down
7 changes: 5 additions & 2 deletions rust/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub mod undo;

use binaryninjacore_sys::*;
use std::collections::HashMap;
use std::ffi::c_char;
use std::ffi::{c_char, c_void};
use std::fmt::Debug;
use std::ptr::NonNull;

Expand Down Expand Up @@ -94,6 +94,8 @@ impl Database {
{
let name_raw = name.into_bytes_with_nul();
let name_ptr = name_raw.as_ref().as_ptr() as *const c_char;
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let new_id = unsafe {
BNWriteDatabaseSnapshotData(
self.handle.as_ptr(),
Expand All @@ -104,10 +106,11 @@ impl Database {
name_ptr,
data.handle.as_ptr(),
auto_save,
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
Some(ProgressExecutor::cb_execute),
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
SnapshotId(new_id)
}

Expand Down
22 changes: 16 additions & 6 deletions rust/src/database/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use binaryninjacore_sys::{
BNSnapshot, BNSnapshotHasAncestor, BNSnapshotHasContents, BNSnapshotHasUndo,
BNSnapshotStoreData,
};
use std::ffi::c_char;
use std::ffi::{c_char, c_void};
use std::fmt;
use std::fmt::{Debug, Display, Formatter};
use std::ptr::NonNull;
Expand Down Expand Up @@ -136,14 +136,17 @@ impl Snapshot {
) -> Array<UndoEntry> {
assert!(self.has_undo());
let mut count = 0;
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let result = unsafe {
BNGetSnapshotUndoEntriesWithProgress(
self.handle.as_ptr(),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
Some(ProgressExecutor::cb_execute),
&mut count,
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
assert!(!result.is_null());
unsafe { Array::new(result, count, ()) }
}
Expand All @@ -158,13 +161,16 @@ impl Snapshot {
&self,
progress: impl Into<ProgressExecutor>,
) -> Ref<KeyValueStore> {
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let result = unsafe {
BNReadSnapshotDataWithProgress(
self.handle.as_ptr(),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
Some(ProgressExecutor::cb_execute),
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
unsafe { KeyValueStore::ref_from_raw(NonNull::new(result).unwrap()) }
}

Expand All @@ -190,14 +196,18 @@ impl Snapshot {
data: &KeyValueStore,
progress: impl Into<ProgressExecutor>,
) -> bool {
unsafe {
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let success = unsafe {
BNSnapshotStoreData(
self.handle.as_ptr(),
data.handle.as_ptr(),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
Some(ProgressExecutor::cb_execute),
)
}
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
success
}

/// Determine if this snapshot has another as an ancestor
Expand Down
7 changes: 5 additions & 2 deletions rust/src/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ impl DebugInfoParser {
existing_debug_info: Option<&DebugInfo>,
progress: impl Into<ProgressExecutor>,
) -> Option<Ref<DebugInfo>> {
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let info: *mut BNDebugInfo = match existing_debug_info {
Some(debug_info) => unsafe {
BNParseDebugInfo(
Expand All @@ -185,7 +187,7 @@ impl DebugInfoParser {
debug_file.handle,
debug_info.handle,
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
)
},
None => unsafe {
Expand All @@ -195,10 +197,11 @@ impl DebugInfoParser {
debug_file.handle,
std::ptr::null_mut(),
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
)
},
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
if info.is_null() {
return None;
}
Expand Down
17 changes: 12 additions & 5 deletions rust/src/file_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use binaryninjacore_sys::{
BNSaveAutoSnapshot, BNSetFilename, BNUndo,
};
use binaryninjacore_sys::{BNCreateDatabaseWithProgress, BNOpenExistingDatabaseWithProgress};
use std::ffi::c_void;
use std::fmt::Debug;

use crate::rc::*;
Expand Down Expand Up @@ -226,15 +227,19 @@ impl FileMetadata {
return false;
};

unsafe {
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let success = unsafe {
BNCreateDatabaseWithProgress(
raw_view.handle,
filename_ptr,
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
Some(ProgressExecutor::cb_execute),
ptr::null_mut(),
)
}
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
success
}

pub fn save_auto_snapshot(&self) -> bool {
Expand Down Expand Up @@ -283,15 +288,17 @@ impl FileMetadata {
) -> Result<Ref<BinaryView>, ()> {
let filename = filename.into_bytes_with_nul();
let filename_ptr = filename.as_ref().as_ptr() as *mut _;

let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let view = unsafe {
BNOpenExistingDatabaseWithProgress(
self.handle,
filename_ptr,
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
Some(ProgressExecutor::cb_execute),
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };

if view.is_null() {
Err(())
Expand Down
2 changes: 1 addition & 1 deletion rust/src/headless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::path::{Path, PathBuf};
use thiserror::Error;

use crate::enterprise::release_license;
use crate::mainthread::{MainThreadAction, MainThreadHandler};
use crate::main_thread::{MainThreadAction, MainThreadHandler};
use crate::rc::Ref;
use binaryninjacore_sys::{BNInitPlugins, BNInitRepoPlugins};
use std::sync::mpsc::Sender;
Expand Down
21 changes: 14 additions & 7 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ pub mod interaction;
pub mod linear_view;
pub mod logger;
pub mod low_level_il;
pub mod mainthread;
pub mod main_thread;
pub mod medium_level_il;
pub mod metadata;
pub mod platform;
mod progress;
pub mod progress;
pub mod project;
pub mod rc;
pub mod references;
Expand Down Expand Up @@ -134,15 +134,18 @@ pub fn load_with_progress(
) -> Option<Ref<BinaryView>> {
let file_path = file_path.as_ref().into_bytes_with_nul();
let options = c"";
let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let handle = unsafe {
BNLoadFilename(
file_path.as_ptr() as *mut _,
true,
options.as_ptr() as *mut c_char,
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
if handle.is_null() {
None
} else {
Expand Down Expand Up @@ -206,16 +209,18 @@ where
.as_ref()
.to_vec()
};

let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let handle = unsafe {
BNLoadFilename(
file_path.as_ptr() as *mut _,
update_analysis_and_wait,
options_or_default.as_ptr() as *mut c_char,
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };

if handle.is_null() {
None
Expand Down Expand Up @@ -262,16 +267,18 @@ where
.as_ref()
.to_vec()
};

let boxed_progress = Box::new(progress.into());
let leaked_boxed_progress = Box::into_raw(boxed_progress);
let handle = unsafe {
BNLoadBinaryView(
bv.handle as *mut _,
update_analysis_and_wait,
options_or_default.as_ptr() as *mut c_char,
Some(ProgressExecutor::cb_execute),
progress.into().into_raw_context(),
leaked_boxed_progress as *mut c_void,
)
};
let _ = unsafe { Box::from_raw(leaked_boxed_progress) };

if handle.is_null() {
None
Expand Down
File renamed without changes.
4 changes: 2 additions & 2 deletions rust/src/medium_level_il/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl MediumLevelILFunction {
///
/// # Example
/// ```no_run
/// # use binaryninja::mlil::MediumLevelILFunction;
/// # use binaryninja::medium_level_il::MediumLevelILFunction;
/// # use binaryninja::variable::PossibleValueSet;
/// # let mlil_fun: MediumLevelILFunction = todo!();
/// let user_var_val = mlil_fun.user_var_values().iter().next().unwrap();
Expand Down Expand Up @@ -322,7 +322,7 @@ impl MediumLevelILFunction {
///
/// # Example
/// ```no_run
/// # use binaryninja::mlil::MediumLevelILFunction;
/// # use binaryninja::medium_level_il::MediumLevelILFunction;
/// # use binaryninja::variable::Variable;
/// # let mlil_fun: MediumLevelILFunction = todo!();
/// # let mlil_var: Variable = todo!();
Expand Down
9 changes: 2 additions & 7 deletions rust/src/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,12 @@ impl ProgressExecutor {
}
}

/// Leak the executor and return an opaque pointer.
pub fn into_raw_context(self) -> *mut c_void {
Box::into_raw(Box::new(self)) as *mut c_void
}

pub unsafe extern "C" fn cb_execute(ctx: *mut c_void, progress: usize, total: usize) -> bool {
if ctx.is_null() {
return true;
}
let f: Box<Self> = Box::from_raw(ctx as *mut Self);
f.execute(progress, total)
let executor: *mut Self = ctx as *mut Self;
(*executor).execute(progress, total)
}

pub fn execute(&self, progress: usize, total: usize) -> bool {
Expand Down
Loading

0 comments on commit f741ac9

Please sign in to comment.