[Rust] Misc documentation and cleanup

Author: emesare

plugins/bntl_utils/src/dump.rs

@@ -47,8 +47,7 @@ impl TILDump {
 
     pub fn dump(&self, type_lib: &TypeLibrary) -> Result<String, TILDumpError> {
         let empty_file = FileMetadata::new();
-        let empty_bv = BinaryView::from_data(&empty_file, &[])
-            .map_err(|_| TILDumpError::ViewCreationFailed)?;
+        let empty_bv = BinaryView::from_data(&empty_file, &[]);
 
         let type_lib_plats = type_lib.platform_names();
         let platform_name = type_lib_plats

plugins/svd/tests/mapper.rs

@@ -18,7 +18,7 @@ fn insta_snapshots() {
         let path = out_dir.join(file_name);
         let svd_str = std::fs::read_to_string(&path).expect("Failed to read svd file");
         let device = svd_parser::parse(&svd_str).expect("Failed to parse svd file");
-        let view = BinaryView::from_data(&FileMetadata::new(), &[]).expect("Failed to create view");
+        let view = BinaryView::from_data(&FileMetadata::new(), &[]);
         let address_size = view.address_size();
         DeviceMapper::new(LoadSettings::default(), address_size, device).map_to_view(&view);
 
@@ -46,7 +46,7 @@ fn test_bitfield_unions() {
     let path = out_dir.join("ARM_Sample.svd");
     let svd_str = std::fs::read_to_string(&path).expect("Failed to read svd file");
     let device = svd_parser::parse(&svd_str).expect("Failed to parse svd file");
-    let view = BinaryView::from_data(&FileMetadata::new(), &[]).expect("Failed to create view");
+    let view = BinaryView::from_data(&FileMetadata::new(), &[]);
     let address_size = view.address_size();
     let mapper = DeviceMapper::new(LoadSettings::default(), address_size, device.clone());
 

plugins/warp/src/processor.rs

@@ -29,6 +29,7 @@ use binaryninja::rc::{Guard, Ref};
 use crate::cache::cached_type_references;
 use crate::convert::platform_to_target;
 use crate::{build_function, INCLUDE_TAG_ICON, INCLUDE_TAG_NAME};
+use binaryninja::file_metadata::{SaveOption, SaveSettings};
 use warp::chunk::{Chunk, ChunkKind, CompressionType};
 use warp::r#type::chunk::TypeChunk;
 use warp::signature::chunk::SignatureChunk;
@@ -645,7 +646,11 @@ impl WarpFileProcessor {
             if !view.file().is_database_backed() {
                 // Update the cache.
                 tracing::debug!("Saving analysis database to {:?}", file_cache_path);
-                if !view.file().create_database(&file_cache_path) {
+                let save_settings = SaveSettings::new().with_option(SaveOption::RemoveUndoData);
+                if !view
+                    .file()
+                    .create_database(&file_cache_path, &save_settings)
+                {
                     // TODO: We might want to error here...
                     tracing::warn!("Failed to save analysis database to {:?}", file_cache_path);
                 }

plugins/warp/tests/matcher.rs

@@ -33,8 +33,7 @@ const MOCK_FUNCTION_BYTES: &[u8] = &[
 
 fn create_mock_bn_function(_session: &Session) -> Ref<BNFunction> {
     let file = FileMetadata::new();
-    let view =
-        BinaryView::from_data(&file, MOCK_FUNCTION_BYTES).expect("Failed to create mock view");
+    let view = BinaryView::from_data(&file, MOCK_FUNCTION_BYTES);
     let platform = Platform::by_name("x86").unwrap();
     // Add the constraint symbol so that the matcher picks it up, so we can test constraint matching.
     let constraint_symbol =
@@ -160,7 +159,7 @@ fn test_add_type_to_view() {
 
     let _session = Session::new().expect("Failed to create session");
     let file = FileMetadata::new();
-    let view = BinaryView::from_data(&file, &[]).expect("Failed to create view");
+    let view = BinaryView::from_data(&file, &[]);
     let arch = CoreArchitecture::by_name("x86").expect("Failed to get architecture");
 
     // Try and add a NTR to the view, this should also add the referenced struct type.

rust/examples/dump_type_library.rs

@@ -36,8 +36,7 @@ fn main() {
         type_lib_header_path
     );
     let type_printer = CoreTypePrinter::default();
-    let empty_bv =
-        BinaryView::from_data(&FileMetadata::new(), &[]).expect("Failed to create empty view");
+    let empty_bv = BinaryView::from_data(&FileMetadata::new(), &[]);
     let printed_types = type_printer
         .print_all_types(
             all_types,

rust/src/binary_view.rs

@@ -12,14 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! A view on binary data and queryable interface of a binary file.
-//!
-//! One key job of BinaryView is file format parsing which allows Binary Ninja to read, write,
-//! insert, remove portions of the file given a virtual address.
-//!
-//! For the purposes of this documentation we define a virtual address as the memory address that
-//! the various pieces of the physical file will be loaded at.
-//! TODO : Mirror the Python docs for this
+//! A view on binary data and queryable interface of a binary files analysis.
 
 use binaryninjacore_sys::*;
 
@@ -104,6 +97,7 @@ pub trait BinaryViewBase: AsRef<BinaryView> {
         0
     }
 
+    /// Check if the offset is valid for the current view.
     fn offset_valid(&self, offset: u64) -> bool {
         let mut buf = [0u8; 1];
 
@@ -112,22 +106,28 @@ pub trait BinaryViewBase: AsRef<BinaryView> {
         self.as_ref().read(&mut buf[..], offset) == buf.len()
     }
 
+    /// Check if the offset is readable for the current view.
     fn offset_readable(&self, offset: u64) -> bool {
         self.offset_valid(offset)
     }
 
+    /// Check if the offset is writable for the current view.
     fn offset_writable(&self, offset: u64) -> bool {
         self.offset_valid(offset)
     }
 
+    /// Check if the offset is executable for the current view.
     fn offset_executable(&self, offset: u64) -> bool {
         self.offset_valid(offset)
     }
 
+    /// Check if the offset is backed by the original file and not added after the fact.
     fn offset_backed_by_file(&self, offset: u64) -> bool {
         self.offset_valid(offset)
     }
 
+    /// Get the next valid offset after the provided `offset`, useful if you need to iterate over all
+    /// readable offsets in the view.
     fn next_valid_offset_after(&self, offset: u64) -> u64 {
         let start = self.as_ref().start();
 
@@ -138,15 +138,17 @@ pub trait BinaryViewBase: AsRef<BinaryView> {
         }
     }
 
-    #[allow(unused)]
-    fn modification_status(&self, offset: u64) -> ModificationStatus {
+    /// Whether the data at the given `offset` been modified (patched).
+    fn modification_status(&self, _offset: u64) -> ModificationStatus {
         ModificationStatus::Original
     }
 
+    /// The lowest address in the view.
     fn start(&self) -> u64 {
         0
     }
 
+    /// The length of the view.
     fn len(&self) -> u64 {
         0
     }
@@ -546,8 +548,6 @@ pub trait BinaryViewExt: BinaryViewBase {
     }
 
     /// The highest address in the view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::end`].
     fn end(&self) -> u64 {
         unsafe { BNGetEndOffset(self.as_ref().handle) }
     }
@@ -2454,6 +2454,26 @@ pub trait BinaryViewExt: BinaryViewBase {
 
 impl<T: BinaryViewBase> BinaryViewExt for T {}
 
+/// Represents the "whole view" of the binary and its analysis.
+///
+/// Analysis information:
+///
+/// - [`BinaryViewExt::functions`]
+/// - [`BinaryViewExt::data_variables`]
+/// - [`BinaryViewExt::strings`]
+///
+/// Annotation information:
+///
+/// - [`BinaryViewExt::symbols`]
+/// - [`BinaryViewExt::tags_all_scopes`]
+/// - [`BinaryViewExt::comments`]
+///
+/// Data representation and binary information:
+///
+/// - [`BinaryViewExt::types`]
+/// - [`BinaryViewExt::segments`]
+/// - [`BinaryViewExt::sections`]
+///
 /// # Cleaning up
 ///
 /// [`BinaryView`] has a cyclic relationship with the associated [`FileMetadata`], each holds a strong
@@ -2476,21 +2496,20 @@ impl BinaryView {
         Ref::new(Self { handle })
     }
 
-    /// Construct the raw binary view from the given metadata. Before calling this make sure you have
-    /// a valid file path set for the [`FileMetadata`]. It is required that the [`FileMetadata::file_path`]
-    /// exist on the local filesystem.
+    /// Construct the raw binary view from the given metadata.
+    ///
+    /// Before calling this, make sure you have a valid file path set for the [`FileMetadata`]. It is
+    /// required that the [`FileMetadata::file_path`] exist in the local filesystem.
     pub fn from_metadata(meta: &FileMetadata) -> Result<Ref<Self>> {
         if !meta.file_path().exists() {
             return Err(());
         }
         let file = meta.file_path().to_cstr();
         let handle =
             unsafe { BNCreateBinaryDataViewFromFilename(meta.handle, file.as_ptr() as *mut _) };
-
         if handle.is_null() {
             return Err(());
         }
-
         unsafe { Ok(Ref::new(Self { handle })) }
     }
 
@@ -2503,29 +2522,34 @@ impl BinaryView {
         Self::from_metadata(meta)
     }
 
-    pub fn from_accessor<A: Accessor>(
+    // TODO: Provide an API that manages the lifetime of the accessor and the view.
+    /// Construct the raw binary view from the given `accessor` and metadata.
+    ///
+    /// It is the responsibility of the caller to keep the accessor alive for the lifetime of the view;
+    /// because of this, we mark the function as unsafe.
+    pub unsafe fn from_accessor<A: Accessor>(
         meta: &FileMetadata,
-        file: &mut FileAccessor<A>,
+        accessor: &mut FileAccessor<A>,
     ) -> Result<Ref<Self>> {
-        let handle = unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut file.raw) };
-
+        let handle = unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut accessor.raw) };
         if handle.is_null() {
             return Err(());
         }
-
         unsafe { Ok(Ref::new(Self { handle })) }
     }
 
-    pub fn from_data(meta: &FileMetadata, data: &[u8]) -> Result<Ref<Self>> {
+    /// Construct the raw binary view from the given `data` and metadata.
+    ///
+    /// The data will be copied into the view, so the caller does not need to keep the data alive.
+    pub fn from_data(meta: &FileMetadata, data: &[u8]) -> Ref<Self> {
         let handle = unsafe {
             BNCreateBinaryDataViewFromData(meta.handle, data.as_ptr() as *mut _, data.len())
         };
-
-        if handle.is_null() {
-            return Err(());
-        }
-
-        unsafe { Ok(Ref::new(Self { handle })) }
+        assert!(
+            !handle.is_null(),
+            "BNCreateBinaryDataViewFromData should always succeed"
+        );
+        unsafe { Ref::new(Self { handle }) }
     }
 
     /// Save the original binary file to the provided `file_path` along with any modifications.
@@ -2571,45 +2595,26 @@ impl BinaryViewBase for BinaryView {
         unsafe { BNRemoveViewData(self.handle, offset, len as u64) }
     }
 
-    /// Check if the offset is valid for the current view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::is_offset_valid`].
     fn offset_valid(&self, offset: u64) -> bool {
         unsafe { BNIsValidOffset(self.handle, offset) }
     }
 
-    /// Check if the offset is readable for the current view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::is_offset_valid`].
     fn offset_readable(&self, offset: u64) -> bool {
         unsafe { BNIsOffsetReadable(self.handle, offset) }
     }
 
-    /// Check if the offset is writable for the current view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::is_offset_writable`].
     fn offset_writable(&self, offset: u64) -> bool {
         unsafe { BNIsOffsetWritable(self.handle, offset) }
     }
 
-    /// Check if the offset is executable for the current view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::is_offset_executable`].
     fn offset_executable(&self, offset: u64) -> bool {
         unsafe { BNIsOffsetExecutable(self.handle, offset) }
     }
 
-    /// Check if the offset is backed by the original file and not added after the fact.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::is_offset_backed_by_file`].
     fn offset_backed_by_file(&self, offset: u64) -> bool {
         unsafe { BNIsOffsetBackedByFile(self.handle, offset) }
     }
 
-    /// Get the next valid offset after the provided `offset`, useful if you need to iterate over all
-    /// readable offsets in the view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::next_valid_offset`].
     fn next_valid_offset_after(&self, offset: u64) -> u64 {
         unsafe { BNGetNextValidOffset(self.handle, offset) }
     }
@@ -2618,16 +2623,10 @@ impl BinaryViewBase for BinaryView {
         unsafe { BNGetModification(self.handle, offset) }
     }
 
-    /// The lowest address in the view.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::start`].
     fn start(&self) -> u64 {
         unsafe { BNGetStartOffset(self.handle) }
     }
 
-    /// The length of the view, lowest to highest address.
-    ///
-    /// NOTE: If operating within a [`Workflow`], consider using [`AnalysisContext::length`].
     fn len(&self) -> u64 {
         unsafe { BNGetViewLength(self.handle) }
     }

rust/src/custom_binary_view.rs

@@ -178,12 +178,23 @@ where
 }
 
 pub trait BinaryViewTypeBase: AsRef<BinaryViewType> {
+    /// Is this [`BinaryViewType`] valid for the given the raw [`BinaryView`]?
+    ///
+    /// Typical implementations will read the magic bytes (e.g. 'MZ'), this is a performance-sensitive
+    /// path so prefer inexpensive checks rather than comprehensive ones.
     fn is_valid_for(&self, data: &BinaryView) -> bool;
 
+    /// Is this [`BinaryViewType`] deprecated and should not be used?
+    ///
+    /// We specify this such that the view type may still be used by existing databases, but not
+    /// newly created views.
     fn is_deprecated(&self) -> bool {
         false
     }
 
+    /// Is this [`BinaryViewType`] able to be loaded forcefully?
+    ///
+    /// If so, it will be shown in the drop-down when a user opens a file with options.
     fn is_force_loadable(&self) -> bool {
         false
     }
@@ -319,6 +330,9 @@ pub trait BinaryViewTypeExt: BinaryViewTypeBase {
 
 impl<T: BinaryViewTypeBase> BinaryViewTypeExt for T {}
 
+/// A [`BinaryViewType`] acts as a factory for [`BinaryView`] objects.
+///
+/// Each file format will have its own type, such as PE, ELF, or Mach-O.
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
 pub struct BinaryViewType {
     pub handle: *mut BNBinaryViewType,
@@ -338,7 +352,9 @@ impl BinaryViewType {
         }
     }
 
-    pub fn list_valid_types_for(data: &BinaryView) -> Array<BinaryViewType> {
+    /// Enumerates all view types and checks to see if the given raw [`BinaryView`] is valid,
+    /// returning only those that are.
+    pub fn valid_types_for_data(data: &BinaryView) -> Array<BinaryViewType> {
         unsafe {
             let mut count: usize = 0;
             let types = BNGetBinaryViewTypesForData(data.handle, &mut count as *mut _);