Improve FileAccessor in Rust API

- Add unit tests
- Expose read/write functionality
- Add some much needed documentation when passing to memory map

Author: emesare

rust/src/binary_view.rs

@@ -32,7 +32,7 @@ use crate::confidence::Conf;
 use crate::data_buffer::DataBuffer;
 use crate::debuginfo::DebugInfo;
 use crate::external_library::{ExternalLibrary, ExternalLocation};
-use crate::file_accessor::FileAccessor;
+use crate::file_accessor::{Accessor, FileAccessor};
 use crate::file_metadata::FileMetadata;
 use crate::flowgraph::FlowGraph;
 use crate::function::{Function, NativeBlock};
@@ -1832,8 +1832,11 @@ impl BinaryView {
         unsafe { Ok(Ref::new(Self { handle })) }
     }
 
-    pub fn from_accessor(meta: &FileMetadata, file: &mut FileAccessor) -> Result<Ref<Self>> {
-        let handle = unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut file.api_object) };
+    pub fn from_accessor<A: Accessor>(
+        meta: &FileMetadata,
+        file: &mut FileAccessor<A>,
+    ) -> Result<Ref<Self>> {
+        let handle = unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut file.raw) };
 
         if handle.is_null() {
             return Err(());
@@ -1875,8 +1878,8 @@ impl BinaryView {
     ///
     /// To avoid the above issue use [`crate::main_thread::execute_on_main_thread_and_wait`] to verify there
     /// are no queued up main thread actions.
-    pub fn save_to_accessor(&self, file: &mut FileAccessor) -> bool {
-        unsafe { BNSaveToFile(self.handle, &mut file.api_object) }
+    pub fn save_to_accessor<A: Accessor>(&self, file: &mut FileAccessor<A>) -> bool {
+        unsafe { BNSaveToFile(self.handle, &mut file.raw) }
     }
 }
 

rust/src/binary_view/memory_map.rs

@@ -1,6 +1,6 @@
 use crate::binary_view::BinaryView;
 use crate::data_buffer::DataBuffer;
-use crate::file_accessor::FileAccessor;
+use crate::file_accessor::{Accessor, FileAccessor};
 use crate::rc::Ref;
 use crate::segment::SegmentFlags;
 use crate::string::{BnString, IntoCStr};
@@ -59,6 +59,9 @@ impl MemoryMap {
         }
     }
 
+    /// Adds the memory region using a [`DataBuffer`].
+    ///
+    /// This will add the contents of the [`DataBuffer`] to the database.
     pub fn add_data_memory_region(
         &mut self,
         name: &str,
@@ -78,11 +81,20 @@ impl MemoryMap {
         }
     }
 
-    pub fn add_remote_memory_region(
+    // TODO: This really cant be safe until BNFileAccessor is ARC'd and can be freed. Probably need another thing
+    // TODO: Ontop of a file accessor in the core that would manage it. (I.e. BNFileAccessorHandle) or something.
+    /// Adds the memory region using a [`FileAccessor`].
+    ///
+    /// This does not add the region contents to the database, instead accesses to the contents
+    /// are done "remotely" to a [`FileAccessor`].
+    ///
+    /// NOTE: The [`FileAccessor`] MUST live as long as the region is available, currently there is no gurentee by
+    /// the type checker that the file accessor is tied to that of the memory region.
+    pub fn add_remote_memory_region<A: Accessor>(
         &mut self,
         name: &str,
         start: u64,
-        accessor: &mut FileAccessor,
+        accessor: &mut FileAccessor<A>,
         segment_flags: Option<SegmentFlags>,
     ) -> bool {
         let name_raw = name.to_cstr();
@@ -91,7 +103,7 @@ impl MemoryMap {
                 self.view.handle,
                 name_raw.as_ptr(),
                 start,
-                &mut accessor.api_object,
+                &mut accessor.raw,
                 segment_flags.unwrap_or_default().into_raw(),
             )
         }

rust/src/file_accessor.rs

@@ -13,41 +13,36 @@
 // limitations under the License.
 
 use binaryninjacore_sys::BNFileAccessor;
-use std::io::{Read, Seek, SeekFrom, Write};
+use std::io::{ErrorKind, Read, Seek, SeekFrom, Write};
 use std::marker::PhantomData;
 use std::slice;
 
-pub struct FileAccessor<'a> {
-    pub(crate) api_object: BNFileAccessor,
-    _ref: PhantomData<&'a mut ()>,
+pub trait Accessor: Read + Write + Seek + Sized {}
+
+impl<T: Read + Write + Seek + Sized> Accessor for T {}
+
+pub struct FileAccessor<A: Accessor> {
+    pub(crate) raw: BNFileAccessor,
+    accessor: PhantomData<A>,
 }
 
-impl<'a> FileAccessor<'a> {
-    pub fn new<F>(f: &'a mut F) -> Self
-    where
-        F: 'a + Read + Write + Seek + Sized,
-    {
+impl<A: Accessor> FileAccessor<A> {
+    pub fn new(accessor: A) -> Self {
         use std::os::raw::c_void;
 
-        extern "C" fn cb_get_length<F>(ctxt: *mut c_void) -> u64
-        where
-            F: Read + Write + Seek + Sized,
-        {
-            let f = unsafe { &mut *(ctxt as *mut F) };
+        extern "C" fn cb_get_length<A: Accessor>(ctxt: *mut c_void) -> u64 {
+            let f = unsafe { &mut *(ctxt as *mut A) };
 
             f.seek(SeekFrom::End(0)).unwrap_or(0)
         }
 
-        extern "C" fn cb_read<F>(
+        extern "C" fn cb_read<A: Accessor>(
             ctxt: *mut c_void,
             dest: *mut c_void,
             offset: u64,
             len: usize,
-        ) -> usize
-        where
-            F: Read + Write + Seek + Sized,
-        {
-            let f = unsafe { &mut *(ctxt as *mut F) };
+        ) -> usize {
+            let f = unsafe { &mut *(ctxt as *mut A) };
             let dest = unsafe { slice::from_raw_parts_mut(dest as *mut u8, len) };
 
             if f.seek(SeekFrom::Start(offset)).is_err() {
@@ -58,16 +53,13 @@ impl<'a> FileAccessor<'a> {
             }
         }
 
-        extern "C" fn cb_write<F>(
+        extern "C" fn cb_write<A: Accessor>(
             ctxt: *mut c_void,
             offset: u64,
             src: *const c_void,
             len: usize,
-        ) -> usize
-        where
-            F: Read + Write + Seek + Sized,
-        {
-            let f = unsafe { &mut *(ctxt as *mut F) };
+        ) -> usize {
+            let f = unsafe { &mut *(ctxt as *mut A) };
             let src = unsafe { slice::from_raw_parts(src as *const u8, len) };
 
             if f.seek(SeekFrom::Start(offset)).is_err() {
@@ -77,14 +69,52 @@ impl<'a> FileAccessor<'a> {
             }
         }
 
+        let boxed_accessor = Box::new(accessor);
+        let leaked_accessor = Box::leak(boxed_accessor);
+
         Self {
-            api_object: BNFileAccessor {
-                context: f as *mut F as *mut _,
-                getLength: Some(cb_get_length::<F>),
-                read: Some(cb_read::<F>),
-                write: Some(cb_write::<F>),
+            raw: BNFileAccessor {
+                context: leaked_accessor as *mut A as *mut _,
+                getLength: Some(cb_get_length::<A>),
+                read: Some(cb_read::<A>),
+                write: Some(cb_write::<A>),
             },
-            _ref: PhantomData,
+            accessor: PhantomData,
+        }
+    }
+
+    pub fn read(&self, addr: u64, len: usize) -> Result<Vec<u8>, ErrorKind> {
+        let cb_read = self.raw.read.unwrap();
+        let mut buf = vec![0; len];
+        let read_len = unsafe { cb_read(self.raw.context, buf.as_mut_ptr() as *mut _, addr, len) };
+        if read_len != len {
+            return Err(ErrorKind::UnexpectedEof);
+        }
+        Ok(buf)
+    }
+
+    pub fn write(&self, addr: u64, data: &[u8]) -> usize {
+        let cb_write = self.raw.write.unwrap();
+        unsafe {
+            cb_write(
+                self.raw.context,
+                addr,
+                data.as_ptr() as *const _,
+                data.len(),
+            )
+        }
+    }
+
+    pub fn length(&self) -> u64 {
+        let cb_get_length = self.raw.getLength.unwrap();
+        unsafe { cb_get_length(self.raw.context) }
+    }
+}
+
+impl<A: Accessor> Drop for FileAccessor<A> {
+    fn drop(&mut self) {
+        unsafe {
+            let _ = Box::from_raw(self.raw.context as *mut A);
         }
     }
 }

rust/tests/file_accessor.rs

@@ -0,0 +1,22 @@
+use binaryninja::file_accessor::FileAccessor;
+use std::io::Cursor;
+
+#[test]
+fn test_file_accessor() {
+    let mut mock_data = Cursor::new(vec![0u8; 100]);
+    let accessor = FileAccessor::new(&mut mock_data);
+    assert_eq!(
+        accessor.length(),
+        100,
+        "File accessor length does not match"
+    );
+    assert_eq!(
+        accessor.write(0x10, &[0xff]),
+        1,
+        "Failed to write to file accessor"
+    );
+    let read_value = accessor
+        .read(0x10, 1)
+        .expect("Failed to read from file accessor");
+    assert_eq!(read_value, &[0xff], "Read value does not match");
+}