From 4a09a005a4c2169a55529b9850c991a8ae7838fe Mon Sep 17 00:00:00 2001 From: Lukas Dresel Date: Thu, 17 Feb 2022 20:46:48 -0500 Subject: [PATCH 1/4] initial attempt at a better error interface --- rust/src/backgroundtask.rs | 10 ++++- rust/src/binaryview.rs | 79 ++++++++++++++++++++++++------------ rust/src/custombinaryview.rs | 48 +++++++++++++--------- rust/src/errors.rs | 69 +++++++++++++++++++++++++++++++ rust/src/filemetadata.rs | 29 +++++++++---- rust/src/lib.rs | 23 +++++++---- rust/src/platform.rs | 6 +++ 7 files changed, 198 insertions(+), 66 deletions(-) create mode 100644 rust/src/errors.rs diff --git a/rust/src/backgroundtask.rs b/rust/src/backgroundtask.rs index 451ecd072..abb8b8d39 100644 --- a/rust/src/backgroundtask.rs +++ b/rust/src/backgroundtask.rs @@ -18,6 +18,7 @@ use std::result; use crate::rc::*; use crate::string::*; +use crate::errors::*; pub type Result = result::Result; @@ -33,13 +34,18 @@ impl BackgroundTask { Self { handle } } - pub fn new(initial_text: S, can_cancel: bool) -> Result> { + pub fn new(initial_text: S, can_cancel: bool) -> BNResult> { let text = initial_text.as_bytes_with_nul(); let handle = unsafe { BNBeginBackgroundTask(text.as_ref().as_ptr() as *mut _, can_cancel) }; if handle.is_null() { - return Err(()); + return Err( + bn_api_error!( + BNBeginBackgroundTask, + &format!("initial_text={:?}, can_cancel={:?}", bytes_error_repr(text.as_ref()), can_cancel) + ) + ); } unsafe { Ok(Ref::new(Self { handle })) } diff --git a/rust/src/binaryview.rs b/rust/src/binaryview.rs index 5209f7449..a8c4c756e 100644 --- a/rust/src/binaryview.rs +++ b/rust/src/binaryview.rs @@ -18,13 +18,13 @@ pub use binaryninjacore_sys::BNModificationStatus as ModificationStatus; use std::ops; use std::ptr; -use std::result; use crate::architecture::Architecture; use crate::architecture::CoreArchitecture; use crate::basicblock::BasicBlock; use crate::databuffer::DataBuffer; use crate::debuginfo::DebugInfo; +use crate::errors::*; use crate::fileaccessor::FileAccessor; use crate::filemetadata::FileMetadata; use crate::flowgraph::FlowGraph; @@ -45,8 +45,6 @@ use crate::string::*; // TODO : general reorg of modules related to bv -pub type Result = result::Result; - pub trait BinaryViewBase: AsRef { fn read(&self, _buf: &mut [u8], _offset: u64) -> usize { 0 @@ -136,11 +134,11 @@ pub trait BinaryViewExt: BinaryViewBase { } } - fn parent_view(&self) -> Result> { + fn parent_view(&self) -> BNResult> { let handle = unsafe { BNGetParentView(self.as_ref().handle) }; if handle.is_null() { - return Err(()); + return Err(BNError::api_error("BNGetParentView", None)); } unsafe { Ok(BinaryView::from_raw(handle)) } @@ -273,19 +271,19 @@ pub trait BinaryViewExt: BinaryViewBase { } } - fn symbol_by_address(&self, addr: u64) -> Result> { + fn symbol_by_address(&self, addr: u64) -> BNResult> { unsafe { let raw_sym = BNGetSymbolByAddress(self.as_ref().handle, addr, ptr::null_mut()); if raw_sym.is_null() { - return Err(()); + return Err(bn_api_error!(BNGetSymbolByAddress, &format!("addr=0x{addr:x}"))); } Ok(Ref::new(Symbol::from_raw(raw_sym))) } } - fn symbol_by_raw_name(&self, raw_name: S) -> Result> { + fn symbol_by_raw_name(&self, raw_name: S) -> BNResult> { let raw_name = raw_name.as_bytes_with_nul(); unsafe { @@ -296,7 +294,7 @@ pub trait BinaryViewExt: BinaryViewBase { ); if raw_sym.is_null() { - return Err(()); + return Err(bn_api_error!(BNGetSymbolByRawName, &format!("raw_name={:?}", bytes_error_repr(raw_name.as_ref())))); } Ok(Ref::new(Symbol::from_raw(raw_sym))) @@ -382,7 +380,7 @@ pub trait BinaryViewExt: BinaryViewBase { sym: &Symbol, plat: &Platform, ty: T, - ) -> Result> { + ) -> BNResult> { let raw_type = if let Some(t) = ty.into() { t.handle } else { @@ -398,7 +396,10 @@ pub trait BinaryViewExt: BinaryViewBase { ); if raw_sym.is_null() { - return Err(()); + return Err(bn_api_error!( + BNDefineAutoSymbolAndVariableOrFunction, + &format!("platform={:?}, sym={:?}, raw_type={:?}", plat, sym, raw_type) + )); } Ok(Ref::new(Symbol::from_raw(raw_sym))) @@ -485,14 +486,17 @@ pub trait BinaryViewExt: BinaryViewBase { } } - fn section_by_name(&self, name: S) -> Result
{ + fn section_by_name(&self, name: S) -> BNResult
{ unsafe { let raw_name = name.as_bytes_with_nul(); let name_ptr = raw_name.as_ref().as_ptr() as *mut _; let raw_section = BNGetSectionByName(self.as_ref().handle, name_ptr); if raw_section.is_null() { - return Err(()); + return Err(bn_api_error!( + BNGetSectionByName, + &format!("name={:?}", bytes_error_repr(raw_name.as_ref())) + )); } Ok(Section::from_raw(raw_section)) @@ -539,12 +543,12 @@ pub trait BinaryViewExt: BinaryViewBase { unsafe { BNHasFunctions(self.as_ref().handle) } } - fn entry_point_function(&self) -> Result> { + fn entry_point_function(&self) -> BNResult> { unsafe { let func = BNGetAnalysisEntryPoint(self.as_ref().handle); if func.is_null() { - return Err(()); + return Err(bn_api_error!(BNGetAnalysisEntryPoint)); } Ok(Function::from_raw(func)) @@ -571,12 +575,15 @@ pub trait BinaryViewExt: BinaryViewBase { } } - fn function_at(&self, platform: &Platform, addr: u64) -> Result> { + fn function_at(&self, platform: &Platform, addr: u64) -> BNResult> { unsafe { let handle = BNGetAnalysisFunction(self.as_ref().handle, platform.handle, addr); if handle.is_null() { - return Err(()); + return Err(bn_api_error!( + BNGetAnalysisFunction, + &format!("platform={:?}, addr={:x}", platform, addr) + )); } Ok(Function::from_raw(handle)) @@ -611,10 +618,13 @@ pub trait BinaryViewExt: BinaryViewBase { } } - fn read_buffer(&self, offset: u64, len: usize) -> Result { + fn read_buffer(&self, offset: u64, len: usize) -> BNResult { let read_buffer = unsafe { BNReadViewBuffer(self.as_ref().handle, offset, len) }; if read_buffer.is_null() { - Err(()) + Err(bn_api_error!( + BNReadViewBuffer, + &format!("offset=0x{offset:x}, len=0x{len:x}") + )) } else { Ok(DataBuffer::from_raw(read_buffer)) } @@ -643,7 +653,7 @@ pub trait BinaryViewExt: BinaryViewBase { } } - fn load_settings(&self, view_type_name: S) -> Result> { + fn load_settings(&self, view_type_name: S) -> BNResult> { let view_type_name = view_type_name.as_bytes_with_nul(); let settings_handle = unsafe { BNBinaryViewGetLoadSettings( @@ -653,7 +663,10 @@ pub trait BinaryViewExt: BinaryViewBase { }; if settings_handle.is_null() { - Err(()) + Err(bn_api_error!( + BNBinaryViewGetLoadSettings, + &format!("name={:?}", bytes_error_repr(view_type_name.as_ref())) + )) } else { Ok(unsafe { Settings::from_raw(settings_handle) }) } @@ -843,7 +856,7 @@ impl BinaryView { pub fn from_filename( meta: &mut FileMetadata, filename: S, - ) -> Result> { + ) -> BNResult> { let file = filename.as_bytes_with_nul(); let handle = unsafe { @@ -851,30 +864,42 @@ impl BinaryView { }; if handle.is_null() { - return Err(()); + return Err(bn_api_error!( + BNCreateBinaryDataViewFromFilename, + &format!("meta.filename={:?}, filename={:?}", + bytes_error_repr(meta.filename().as_ref()), + bytes_error_repr(file.as_ref()) + ) + )); } unsafe { Ok(Ref::new(Self { handle })) } } - pub fn from_accessor(meta: &FileMetadata, file: &mut FileAccessor) -> Result> { + pub fn from_accessor(meta: &FileMetadata, file: &mut FileAccessor) -> BNResult> { let handle = unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut file.api_object as *mut _) }; if handle.is_null() { - return Err(()); + return Err(bn_api_error!( + BNCreateBinaryDataViewFromFile, + &format!("meta.filename={:?}", bytes_error_repr(meta.filename().as_ref())) + )); } unsafe { Ok(Ref::new(Self { handle })) } } - pub fn from_data(meta: &FileMetadata, data: &[u8]) -> Result> { + pub fn from_data(meta: &FileMetadata, data: &[u8]) -> BNResult> { let handle = unsafe { BNCreateBinaryDataViewFromData(meta.handle, data.as_ptr() as *mut _, data.len()) }; if handle.is_null() { - return Err(()); + return Err(bn_api_error!( + BNCreateBinaryDataViewFromData, + &format!("meta.filename={:?}", bytes_error_repr(meta.filename().as_ref())) + )); } unsafe { Ok(Ref::new(Self { handle })) } diff --git a/rust/src/custombinaryview.rs b/rust/src/custombinaryview.rs index 48274fe4c..605b4c935 100644 --- a/rust/src/custombinaryview.rs +++ b/rust/src/custombinaryview.rs @@ -23,7 +23,8 @@ use std::ptr; use std::slice; use crate::architecture::Architecture; -use crate::binaryview::{BinaryView, BinaryViewBase, BinaryViewExt, Result}; +use crate::binaryview::{BinaryView, BinaryViewBase, BinaryViewExt}; +use crate::errors::*; use crate::platform::Platform; use crate::settings::Settings; use crate::Endianness; @@ -141,12 +142,14 @@ where pub trait BinaryViewTypeBase: AsRef { fn is_valid_for(&self, data: &BinaryView) -> bool; - fn load_settings_for_data(&self, data: &BinaryView) -> Result> { + fn load_settings_for_data(&self, data: &BinaryView) -> BNResult> { let settings_handle = unsafe { BNGetBinaryViewDefaultLoadSettingsForData(self.as_ref().0, data.handle) }; if settings_handle.is_null() { - Err(()) + return Err(bn_api_error!( + BNGetBinaryViewDefaultLoadSettingsForData + )); } else { unsafe { Ok(Settings::from_raw(settings_handle)) } } @@ -176,15 +179,14 @@ pub trait BinaryViewTypeExt: BinaryViewTypeBase { } } - fn open(&self, data: &BinaryView) -> Result> { + fn open(&self, data: &BinaryView) -> BNResult> { let handle = unsafe { BNCreateBinaryViewOfType(self.as_ref().0, data.handle) }; if handle.is_null() { - error!( - "failed to create BinaryView of BinaryViewType '{}'", - self.name() - ); - return Err(()); + return Err(bn_api_error!( + BNCreateBinaryViewOfType, + &format!("failed to create BinaryView of BinaryViewType '{}'", self.name()) + )); } unsafe { Ok(BinaryView::from_raw(handle)) } @@ -216,14 +218,17 @@ impl BinaryViewType { } /// Looks up a BinaryViewType by its short name - pub fn by_name(name: N) -> Result { + pub fn by_name(name: N) -> BNResult { let bytes = name.as_bytes_with_nul(); let res = unsafe { BNGetBinaryViewTypeByName(bytes.as_ref().as_ptr() as *const _) }; match res.is_null() { false => Ok(BinaryViewType(res)), - true => Err(()), + true => Err(bn_api_error!( + BNGetBinaryViewTypeByName, + &format!("name={:?}", bytes_error_repr(bytes.as_ref())) + )), } } } @@ -233,12 +238,14 @@ impl BinaryViewTypeBase for BinaryViewType { unsafe { BNIsBinaryViewTypeValidForData(self.0, data.handle) } } - fn load_settings_for_data(&self, data: &BinaryView) -> Result> { + fn load_settings_for_data(&self, data: &BinaryView) -> BNResult> { let settings_handle = unsafe { BNGetBinaryViewDefaultLoadSettingsForData(self.as_ref().0, data.handle) }; if settings_handle.is_null() { - Err(()) + Err(bn_api_error!( + BNGetBinaryViewDefaultLoadSettingsForData + )) } else { unsafe { Ok(Settings::from_raw(settings_handle)) } } @@ -278,7 +285,7 @@ pub trait CustomBinaryViewType: 'static + BinaryViewTypeBase + Sync { &self, data: &BinaryView, builder: CustomViewBuilder<'builder, Self>, - ) -> Result>; + ) -> BNResult>; } /// Represents a request from the core to instantiate a custom BinaryView @@ -290,8 +297,8 @@ pub struct CustomViewBuilder<'a, T: CustomBinaryViewType + ?Sized> { pub unsafe trait CustomBinaryView: 'static + BinaryViewBase + Sync + Sized { type Args: Send; - fn new(handle: &BinaryView, args: &Self::Args) -> Result; - fn init(&self, args: Self::Args) -> Result<()>; + fn new(handle: &BinaryView, args: &Self::Args) -> BNResult; + fn init(&self, args: Self::Args) -> BNResult<()>; } /// Represents a partially initialized custom `BinaryView` that should be returned to the core @@ -338,7 +345,7 @@ impl<'a, T: CustomBinaryViewType> CustomViewBuilder<'a, T> { /// The `BinaryView` argument passed to the constructor function is the object that is expected /// to be returned by the `AsRef` implementation required by the `BinaryViewBase` trait. /// TODO FIXME welp this is broke going to need 2 init callbacks - pub fn create(self, parent: &BinaryView, view_args: V::Args) -> Result> + pub fn create(self, parent: &BinaryView, view_args: V::Args) -> BNResult> where V: CustomBinaryView, { @@ -357,13 +364,14 @@ impl<'a, T: CustomBinaryViewType> CustomViewBuilder<'a, T> { // even if we deal with it gracefully in cb_free_object, // BNCreateBinaryViewOfType is still going to crash, so we're just // going to try and stop this from happening in the first place. - error!( + let msg = format!( "attempt to create duplicate view of type '{}' (existing: {:?})", view_name.as_str(), bv.handle ); + error!("{}", msg); - return Err(()); + return Err(BNError::generic(&msg)); } // wildly unsafe struct representing the context of a BNCustomBinaryView @@ -746,7 +754,7 @@ impl<'a, T: CustomBinaryViewType> CustomViewBuilder<'a, T> { } } - pub fn wrap_existing(self, wrapped_view: Ref) -> Result> { + pub fn wrap_existing(self, wrapped_view: Ref) -> BNResult> { Ok(CustomView { handle: wrapped_view, _builder: PhantomData, diff --git a/rust/src/errors.rs b/rust/src/errors.rs new file mode 100644 index 000000000..9785421d9 --- /dev/null +++ b/rust/src/errors.rs @@ -0,0 +1,69 @@ +use std::{fmt, backtrace::Backtrace, error::Error}; + +#[allow(dead_code)] // it's used in `Debug` which is used in `Display` which is used to show the error +#[derive(Debug)] +pub struct BNError { + repr: BNErrorRepr, + backtrace: Option, + cause: Option>, +} + +#[derive(Debug, Eq, PartialEq, Hash)] +pub enum BNErrorRepr { + Generic(String), + APIError { api_function: String, other_info: Option}, + IOError, +} + +macro_rules! bn_api_error { + ($func:tt) => { + BNError::api_error(stringify!($func), None) + }; + ($func:tt, $extra:expr) => { + BNError::api_error(stringify!($func), Some($extra)) + }; +} +pub(crate) use bn_api_error; + +pub type BNResult = Result; + +pub fn bytes_error_repr(r: &[u8]) -> String { + String::from_utf8_lossy(r).to_string() +} + +impl fmt::Display for BNError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + ::fmt(self, f) + } +} + +impl BNError { + pub fn generic(msg: &str) -> BNError { + BNError { + repr: BNErrorRepr::Generic(String::from(msg)), + backtrace: Some(Backtrace::capture()), + cause: None + } + } + pub fn api_error(func: &str, other_info: Option<&str>) -> BNError { + BNError { + repr: BNErrorRepr::APIError{ + api_function: String::from(func), + other_info: other_info.map(String::from), + }, + backtrace: Some(Backtrace::capture()), + cause: None + } + } +} +impl Error for BNError { + fn backtrace(&self) -> Option<&Backtrace> { + self.backtrace.as_ref() + } + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.cause { + Some(ex) => Some(ex.as_ref()), + None => None, + } + } +} \ No newline at end of file diff --git a/rust/src/filemetadata.rs b/rust/src/filemetadata.rs index f694655dc..231410e45 100644 --- a/rust/src/filemetadata.rs +++ b/rust/src/filemetadata.rs @@ -47,6 +47,7 @@ use crate::binaryview::BinaryView; use crate::rc::*; use crate::string::*; +use crate::errors::*; use std::ptr; @@ -156,26 +157,32 @@ impl FileMetadata { unsafe { BNGetCurrentOffset(self.handle) } } - pub fn navigate_to(&self, view: S, offset: u64) -> Result<(), ()> { + pub fn navigate_to(&self, view: S, offset: u64) -> BNResult<()> { let view = view.as_bytes_with_nul(); unsafe { if BNNavigate(self.handle, view.as_ref().as_ptr() as *const _, offset) { Ok(()) } else { - Err(()) + Err(bn_api_error!( + BNNavigate, + &format!("view={:?}, offset=0x{:x}", bytes_error_repr(view.as_ref()), offset) + )) } } } - pub fn get_view_of_type(&self, view: S) -> Result, ()> { + pub fn get_view_of_type(&self, view: S) -> BNResult> { let view = view.as_bytes_with_nul(); unsafe { let res = BNGetFileViewOfType(self.handle, view.as_ref().as_ptr() as *const _); if res.is_null() { - Err(()) + Err(bn_api_error!( + BNGetFileViewOfType, + &format!("view={:?}", bytes_error_repr(view.as_ref())) + )) } else { Ok(BinaryView::from_raw(res)) } @@ -208,21 +215,24 @@ impl FileMetadata { pub fn open_database_for_configuration( &self, filename: S, - ) -> Result, ()> { + ) -> BNResult> { let filename = filename.as_bytes_with_nul(); unsafe { let bv = BNOpenDatabaseForConfiguration(self.handle, filename.as_ref().as_ptr() as *const _); if bv.is_null() { - Err(()) + Err(bn_api_error!( + BNOpenDatabaseForConfiguration, + &format!("filename={:?}", bytes_error_repr(filename.as_ref())) + )) } else { Ok(BinaryView::from_raw(bv)) } } } - pub fn open_database(&self, filename: S) -> Result, ()> { + pub fn open_database(&self, filename: S) -> BNResult> { let filename = filename.as_bytes_with_nul(); let filename_ptr = filename.as_ref().as_ptr() as *mut _; @@ -235,7 +245,10 @@ impl FileMetadata { // }; if view.is_null() { - Err(()) + Err(bn_api_error!( + BNOpenExistingDatabase, + &format!("filename={:?}", bytes_error_repr(filename.as_ref())) + )) } else { Ok(unsafe { BinaryView::from_raw(view) }) } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 966f3ff26..accf91631 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -15,6 +15,8 @@ //! # Warning //! > ⚠️ **These bindings are in a very early beta, only have partial support for the core APIs and are still actively under development. Compatibility _will_ break and conventions _will_ change! They are being used for core Binary Ninja features however, so we expect much of what is already there to be reliable enough to build on, just don't be surprised if your plugins/scripts need to hit a moving target.** +#![feature(backtrace)] + #[macro_use] extern crate log; pub extern crate binaryninjacore_sys; @@ -67,6 +69,7 @@ pub mod string; pub mod symbol; pub mod tags; pub mod types; +pub mod errors; use std::collections::HashMap; use std::fs::File; @@ -76,6 +79,8 @@ use std::path::Path; pub use binaryninjacore_sys::BNBranchType as BranchType; pub use binaryninjacore_sys::BNEndianness as Endianness; +use errors::*; + // Commented out to suppress unused warnings // const BN_MAX_INSTRUCTION_LENGTH: u64 = 256; // const BN_DEFAULT_INSTRUCTION_LENGTH: u64 = 16; @@ -297,7 +302,7 @@ pub fn open_view_with_options>( filename: F, update_analysis_and_wait: bool, options: Option>, -) -> Result, String> { +) -> Result, BNError> { //! This is incomplete, but should work in most cases: //! ``` //! let settings = [("analysis.linearSweep.autorun", "false")] @@ -326,13 +331,13 @@ pub fn open_view_with_options>( Ok(_) => { let sqlite_string = "SQLite format 3"; if buf != sqlite_string.as_bytes() { - return Err("Not a valid BNDB (invalid magic)".to_string()); + return Err(BNError::generic("Not a valid BNDB (invalid magic)")); } } - _ => return Err("Not a valid BNDB (too small)".to_string()), + _ => return Err(BNError::generic("Not a valid BNDB (too small)")), } } - _ => return Err("Could not open file".to_string()), + _ => return Err(BNError::generic("Could not open file")), } is_bndb = true; metadata.open_database_for_configuration(filename.to_str().unwrap()) @@ -340,7 +345,7 @@ pub fn open_view_with_options>( false => binaryview::BinaryView::from_filename(&mut metadata, filename.to_str().unwrap()), } { Ok(view) => view, - _ => return Err("Unable to open file".to_string()), + _ => return Err(BNError::generic("Unable to open file")), }; let mut universal_view_type = None; @@ -382,7 +387,7 @@ pub fn open_view_with_options>( .load_settings_for_data(view.as_ref()) { Ok(settings) => Some(settings), - _ => return Err("Could not load settings for universal view data".to_string()), + _ => return Err(BNError::generic("Could not load settings for universal view data")), }; // let arch_list = load_settings.as_ref().unwrap().get_string( @@ -422,14 +427,14 @@ pub fn open_view_with_options>( for (setting, value) in options { if load_settings.contains(setting) { if !load_settings.set_json(setting, value, Some(view.as_ref()), None) { - return Err(format!("Setting: {} set operation failed!", setting)); + return Err(BNError::generic(&format!("Setting: {} set operation failed!", setting))); } } else if default_settings.contains(setting) { if !default_settings.set_json(setting, value, Some(view.as_ref()), None) { - return Err(format!("Setting: {} set operation failed!", setting)); + return Err(BNError::generic(&format!("Setting: {} set operation failed!", setting))); } } else { - return Err(format!("Setting: {} not available!", setting)); + return Err(BNError::generic(&format!("Setting: {} not available!", setting))); } } } diff --git a/rust/src/platform.rs b/rust/src/platform.rs index 9d7987c4e..86f1579af 100644 --- a/rust/src/platform.rs +++ b/rust/src/platform.rs @@ -232,6 +232,12 @@ impl Platform { } } +impl std::fmt::Debug for Platform { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Platform[name={:?}, arch={:?}]", self.name().as_str(), self.arch().name().as_str()) + } +} + pub trait TypeParser { fn parse_types_from_source>( &self, From 2adc9e477042a750bb4a8950b0418ec9cb89ae8a Mon Sep 17 00:00:00 2001 From: Lukas Dresel Date: Thu, 17 Feb 2022 20:58:57 -0500 Subject: [PATCH 2/4] inlining of helpers --- rust/src/errors.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/src/errors.rs b/rust/src/errors.rs index 9785421d9..1bfb73637 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -38,6 +38,7 @@ impl fmt::Display for BNError { } impl BNError { + #[inline(always)] pub fn generic(msg: &str) -> BNError { BNError { repr: BNErrorRepr::Generic(String::from(msg)), @@ -45,6 +46,7 @@ impl BNError { cause: None } } + #[inline(always)] pub fn api_error(func: &str, other_info: Option<&str>) -> BNError { BNError { repr: BNErrorRepr::APIError{ From 575eae3e0b3a3e1a60423d05625f7c57f5ade6f1 Mon Sep 17 00:00:00 2001 From: Lukas Dresel Date: Fri, 18 Feb 2022 11:42:13 -0500 Subject: [PATCH 3/4] finished better error refactor for most cases --- rust/src/backgroundtask.rs | 2 +- rust/src/binaryview.rs | 14 ++++---- rust/src/custombinaryview.rs | 2 +- rust/src/debuginfo.rs | 8 +++-- rust/src/demangle.rs | 31 +++++++++++------ rust/src/errors.rs | 51 +++++++++++++++++++++------- rust/src/filemetadata.rs | 8 ++--- rust/src/function.rs | 15 ++++++--- rust/src/platform.rs | 17 +++++++--- rust/src/types.rs | 65 ++++++++++++++++++------------------ 10 files changed, 134 insertions(+), 79 deletions(-) diff --git a/rust/src/backgroundtask.rs b/rust/src/backgroundtask.rs index abb8b8d39..931b76708 100644 --- a/rust/src/backgroundtask.rs +++ b/rust/src/backgroundtask.rs @@ -43,7 +43,7 @@ impl BackgroundTask { return Err( bn_api_error!( BNBeginBackgroundTask, - &format!("initial_text={:?}, can_cancel={:?}", bytes_error_repr(text.as_ref()), can_cancel) + &format!("initial_text={:?}, can_cancel={:?}", Utf8Display(&text), can_cancel) ) ); } diff --git a/rust/src/binaryview.rs b/rust/src/binaryview.rs index a8c4c756e..0433a8955 100644 --- a/rust/src/binaryview.rs +++ b/rust/src/binaryview.rs @@ -294,7 +294,7 @@ pub trait BinaryViewExt: BinaryViewBase { ); if raw_sym.is_null() { - return Err(bn_api_error!(BNGetSymbolByRawName, &format!("raw_name={:?}", bytes_error_repr(raw_name.as_ref())))); + return Err(bn_api_error!(BNGetSymbolByRawName, &format!("raw_name={:?}", Utf8Display(&raw_name)))); } Ok(Ref::new(Symbol::from_raw(raw_sym))) @@ -495,7 +495,7 @@ pub trait BinaryViewExt: BinaryViewBase { if raw_section.is_null() { return Err(bn_api_error!( BNGetSectionByName, - &format!("name={:?}", bytes_error_repr(raw_name.as_ref())) + &format!("name={:?}", Utf8Display(&raw_name)) )); } @@ -665,7 +665,7 @@ pub trait BinaryViewExt: BinaryViewBase { if settings_handle.is_null() { Err(bn_api_error!( BNBinaryViewGetLoadSettings, - &format!("name={:?}", bytes_error_repr(view_type_name.as_ref())) + &format!("name={:?}", Utf8Display(&view_type_name)) )) } else { Ok(unsafe { Settings::from_raw(settings_handle) }) @@ -867,8 +867,8 @@ impl BinaryView { return Err(bn_api_error!( BNCreateBinaryDataViewFromFilename, &format!("meta.filename={:?}, filename={:?}", - bytes_error_repr(meta.filename().as_ref()), - bytes_error_repr(file.as_ref()) + Utf8Display(&meta.filename()), + Utf8Display(&file) ) )); } @@ -883,7 +883,7 @@ impl BinaryView { if handle.is_null() { return Err(bn_api_error!( BNCreateBinaryDataViewFromFile, - &format!("meta.filename={:?}", bytes_error_repr(meta.filename().as_ref())) + &format!("meta.filename={:?}", Utf8Display(&meta.filename())) )); } @@ -898,7 +898,7 @@ impl BinaryView { if handle.is_null() { return Err(bn_api_error!( BNCreateBinaryDataViewFromData, - &format!("meta.filename={:?}", bytes_error_repr(meta.filename().as_ref())) + &format!("meta.filename={:?}", Utf8Display(&meta.filename())) )); } diff --git a/rust/src/custombinaryview.rs b/rust/src/custombinaryview.rs index 605b4c935..5037470c7 100644 --- a/rust/src/custombinaryview.rs +++ b/rust/src/custombinaryview.rs @@ -227,7 +227,7 @@ impl BinaryViewType { false => Ok(BinaryViewType(res)), true => Err(bn_api_error!( BNGetBinaryViewTypeByName, - &format!("name={:?}", bytes_error_repr(bytes.as_ref())) + &format!("name={:?}", Utf8Display(&bytes)) )), } } diff --git a/rust/src/debuginfo.rs b/rust/src/debuginfo.rs index 56786d191..73382a28e 100644 --- a/rust/src/debuginfo.rs +++ b/rust/src/debuginfo.rs @@ -71,6 +71,7 @@ use crate::{ binaryview::BinaryView, callingconvention::CallingConvention, platform::Platform, + errors::*, rc::*, string::{raw_to_string, BnStrCompatible, BnString}, types::{DataVariableAndName, NameAndType, Type}, @@ -96,12 +97,15 @@ impl DebugInfoParser { } /// Returns debug info parser of the given name, if it exists - pub fn from_name(name: S) -> Result, ()> { + pub fn from_name(name: S) -> BNResult> { let name = name.as_bytes_with_nul(); let parser = unsafe { BNGetDebugInfoParserByName(name.as_ref().as_ptr() as *mut _) }; if parser.is_null() { - Err(()) + Err(bn_api_error!( + BNGetDebugInfoParserByName, + &format!("name={:?}", Utf8Display(&name)) + )) } else { unsafe { Ok(Self::from_raw(parser)) } } diff --git a/rust/src/demangle.rs b/rust/src/demangle.rs index ccc3d2dea..d9b743322 100644 --- a/rust/src/demangle.rs +++ b/rust/src/demangle.rs @@ -1,19 +1,18 @@ use binaryninjacore_sys::*; -use std::{ffi::CStr, result}; +use std::{ffi::CStr}; use crate::architecture::CoreArchitecture; use crate::string::{BnStrCompatible, BnString}; use crate::types::Type; use crate::rc::*; - -pub type Result = result::Result; +use crate::errors::*; pub fn demangle_gnu3( arch: &CoreArchitecture, mangled_name: S, simplify: bool, -) -> Result<(Option>, Vec)> { +) -> BNResult<(Option>, Vec)> { let mangled_name_bwn = mangled_name.as_bytes_with_nul(); let mangled_name_ptr = mangled_name_bwn.as_ref(); let mut out_type: *mut BNType = unsafe { std::mem::zeroed() }; @@ -33,9 +32,9 @@ pub fn demangle_gnu3( if !res || out_size == 0 { let cstr = match CStr::from_bytes_with_nul(mangled_name_ptr) { Ok(cstr) => cstr, - Err(_) => { + Err(e) => { log::error!("demangle_gnu3: failed to parse mangled name"); - return Err(()); + return Err(e.into()); } }; return Ok((None, vec![cstr.to_string_lossy().into_owned()])); @@ -51,7 +50,12 @@ pub fn demangle_gnu3( if out_name.is_null() { log::error!("demangle_gnu3: out_name is NULL"); - return Err(()); + return Err(bn_api_error!( + BNDemangleGNU3, + &format!("arch={:?}, mangled_name={:?}, simplify={:?}", + arch.0, Utf8Display(&mangled_name_bwn), simplify + ) + )); } let names = unsafe { ArrayGuard::::new(out_name, out_size, ()) } @@ -68,7 +72,7 @@ pub fn demangle_ms( arch: &CoreArchitecture, mangled_name: S, simplify: bool, -) -> Result<(Option>, Vec)> { +) -> BNResult<(Option>, Vec)> { let mangled_name_bwn = mangled_name.as_bytes_with_nul(); let mangled_name_ptr = mangled_name_bwn.as_ref(); @@ -89,9 +93,9 @@ pub fn demangle_ms( if !res || out_size == 0 { let cstr = match CStr::from_bytes_with_nul(mangled_name_ptr) { Ok(cstr) => cstr, - Err(_) => { + Err(e) => { log::error!("demangle_ms: failed to parse mangled name"); - return Err(()); + return Err(e.into()); } }; return Ok((None, vec![cstr.to_string_lossy().into_owned()])); @@ -107,7 +111,12 @@ pub fn demangle_ms( if out_name.is_null() { log::error!("demangle_ms: out_name is NULL"); - return Err(()); + return Err(bn_api_error!( + BNDemangleMS, + &format!("arch={:?}, mangled_name={:?}, simplify={:?}", + arch.0, Utf8Display(&mangled_name_bwn), simplify + ) + )); } let names = unsafe { ArrayGuard::::new(out_name, out_size, ()) } diff --git a/rust/src/errors.rs b/rust/src/errors.rs index 1bfb73637..8e70d321b 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -1,18 +1,18 @@ -use std::{fmt, backtrace::Backtrace, error::Error}; +use std::{fmt, backtrace::Backtrace, error::Error, ffi::FromBytesWithNulError}; #[allow(dead_code)] // it's used in `Debug` which is used in `Display` which is used to show the error #[derive(Debug)] pub struct BNError { repr: BNErrorRepr, backtrace: Option, - cause: Option>, } -#[derive(Debug, Eq, PartialEq, Hash)] +#[derive(Debug)] pub enum BNErrorRepr { Generic(String), APIError { api_function: String, other_info: Option}, - IOError, + IOError { cause: std::io::Error }, + FFIStrDecodeError { cause: FromBytesWithNulError } } macro_rules! bn_api_error { @@ -27,8 +27,16 @@ pub(crate) use bn_api_error; pub type BNResult = Result; -pub fn bytes_error_repr(r: &[u8]) -> String { - String::from_utf8_lossy(r).to_string() +pub struct Utf8Display<'a>(pub &'a dyn AsRef<[u8]>); +impl<'a> std::fmt::Debug for Utf8Display<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", String::from_utf8_lossy(self.0.as_ref()).to_string()) + } +} +impl<'a> std::fmt::Display for Utf8Display<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ::fmt(self, f) + } } impl fmt::Display for BNError { @@ -43,7 +51,6 @@ impl BNError { BNError { repr: BNErrorRepr::Generic(String::from(msg)), backtrace: Some(Backtrace::capture()), - cause: None } } #[inline(always)] @@ -54,18 +61,38 @@ impl BNError { other_info: other_info.map(String::from), }, backtrace: Some(Backtrace::capture()), - cause: None } } } + impl Error for BNError { fn backtrace(&self) -> Option<&Backtrace> { - self.backtrace.as_ref() + // if the cause has a backtrace use that, otherwise use ours if possible + self.source() + .and_then(Error::backtrace) + .or(self.backtrace.as_ref()) } fn source(&self) -> Option<&(dyn Error + 'static)> { - match &self.cause { - Some(ex) => Some(ex.as_ref()), - None => None, + match &self.repr { + BNErrorRepr::IOError { cause } => Some(cause), + _ => None, + } + } +} + +impl From for BNError { + fn from(e: FromBytesWithNulError) -> Self { + BNError { + repr: BNErrorRepr::FFIStrDecodeError { cause: e }, + backtrace: None // we rely on the `cause` Backtrace + } + } +} +impl From for BNError { + fn from(e: std::io::Error) -> Self { + BNError { + repr: BNErrorRepr::IOError { cause: e }, + backtrace: None // we rely on the `cause` Backtrace } } } \ No newline at end of file diff --git a/rust/src/filemetadata.rs b/rust/src/filemetadata.rs index 231410e45..e7827a8f0 100644 --- a/rust/src/filemetadata.rs +++ b/rust/src/filemetadata.rs @@ -166,7 +166,7 @@ impl FileMetadata { } else { Err(bn_api_error!( BNNavigate, - &format!("view={:?}, offset=0x{:x}", bytes_error_repr(view.as_ref()), offset) + &format!("view={:?}, offset=0x{:x}", Utf8Display(&view), offset) )) } } @@ -181,7 +181,7 @@ impl FileMetadata { if res.is_null() { Err(bn_api_error!( BNGetFileViewOfType, - &format!("view={:?}", bytes_error_repr(view.as_ref())) + &format!("view={:?}", Utf8Display(&view)) )) } else { Ok(BinaryView::from_raw(res)) @@ -224,7 +224,7 @@ impl FileMetadata { if bv.is_null() { Err(bn_api_error!( BNOpenDatabaseForConfiguration, - &format!("filename={:?}", bytes_error_repr(filename.as_ref())) + &format!("filename={:?}", Utf8Display(&filename)) )) } else { Ok(BinaryView::from_raw(bv)) @@ -247,7 +247,7 @@ impl FileMetadata { if view.is_null() { Err(bn_api_error!( BNOpenExistingDatabase, - &format!("filename={:?}", bytes_error_repr(filename.as_ref())) + &format!("filename={:?}", Utf8Display(&filename)) )) } else { Ok(unsafe { BinaryView::from_raw(view) }) diff --git a/rust/src/function.rs b/rust/src/function.rs index 4ce90457a..8cbe4ef56 100644 --- a/rust/src/function.rs +++ b/rust/src/function.rs @@ -25,6 +25,7 @@ use crate::types::Type; use crate::llil; +use crate::errors::*; use crate::rc::*; use crate::string::*; @@ -210,24 +211,30 @@ impl Function { } } - pub fn low_level_il(&self) -> Result>, ()> { + pub fn low_level_il(&self) -> BNResult>> { unsafe { let llil = BNGetFunctionLowLevelIL(self.handle); if llil.is_null() { - return Err(()); + return Err(bn_api_error!( + BNGetFunctionLowLevelIL, + &format!("self={:?}", self) + )); } Ok(Ref::new(llil::RegularFunction::from_raw(self.arch(), llil))) } } - pub fn lifted_il(&self) -> Result>, ()> { + pub fn lifted_il(&self) -> BNResult>> { unsafe { let llil = BNGetFunctionLiftedIL(self.handle); if llil.is_null() { - return Err(()); + return Err(bn_api_error!( + BNGetFunctionLiftedIL, + &format!("self={:?}", self) + )); } Ok(Ref::new(llil::LiftedFunction::from_raw(self.arch(), llil))) diff --git a/rust/src/platform.rs b/rust/src/platform.rs index 86f1579af..4049dc47f 100644 --- a/rust/src/platform.rs +++ b/rust/src/platform.rs @@ -20,6 +20,7 @@ use crate::{ architecture::{Architecture, CoreArchitecture}, callingconvention::CallingConvention, rc::*, + errors::*, string::*, types::{QualifiedName, QualifiedNameAndType, Type}, }; @@ -245,8 +246,8 @@ pub trait TypeParser { _filename: S, _include_directories: &[P], _auto_type_source: S, - ) -> Result { - Err(String::new()) + ) -> BNResult { + Err(BNError::generic("unimplemented parse_types_from_sources")) } } @@ -264,7 +265,7 @@ impl TypeParser for Platform { filename: S, include_directories: &[P], auto_type_source: S, - ) -> Result { + ) -> BNResult { let mut result = BNTypeParserResult { functionCount: 0, typeCount: 0, @@ -304,7 +305,15 @@ impl TypeParser for Platform { let error_msg = BnString::from_raw(error_string); if !success { - return Err(error_msg.to_string()); + return Err(bn_api_error!( + BNParseTypesFromSource, + &format!("self={:?}, src={:?}, filename={:?}, error_msg={:?}", + self, + Utf8Display(&src), + Utf8Display(&filename), + Utf8Display(&error_msg) + ) + )); } for i in slice::from_raw_parts(result.types, result.typeCount) { diff --git a/rust/src/types.rs b/rust/src/types.rs index 3afc9c9d1..c005e3f44 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -17,15 +17,14 @@ // TODO : Test the get_enumeration and get_structure methods use binaryninjacore_sys::*; -use std::{fmt, mem, ptr, result, slice}; +use std::{fmt, mem, ptr, slice}; use crate::architecture::{Architecture, CoreArchitecture}; use crate::callingconvention::CallingConvention; use crate::string::{raw_to_string, BnStr, BnStrCompatible, BnString}; use crate::rc::*; - -pub type Result = result::Result; +use crate::errors::*; pub type ReferenceType = BNReferenceType; pub type TypeClass = BNTypeClass; @@ -229,37 +228,37 @@ impl TypeBuilder { unsafe { BNIsTypeBuilderFloatingPoint(self.handle) } } - pub fn target(&self) -> Result>> { + pub fn target(&self) -> BNResult>> { let raw_target = unsafe { BNGetTypeBuilderChildType(self.handle) }; if raw_target.type_.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderChildType)) } else { Ok(raw_target.into()) } } - pub fn element_type(&self) -> Result>> { + pub fn element_type(&self) -> BNResult>> { let raw_target = unsafe { BNGetTypeBuilderChildType(self.handle) }; if raw_target.type_.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderChildType)) } else { Ok(raw_target.into()) } } - pub fn return_value(&self) -> Result>> { + pub fn return_value(&self) -> BNResult>> { let raw_target = unsafe { BNGetTypeBuilderChildType(self.handle) }; if raw_target.type_.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderChildType)) } else { Ok(raw_target.into()) } } - pub fn calling_convention(&self) -> Result>>> { + pub fn calling_convention(&self) -> BNResult>>> { let convention_confidence = unsafe { BNGetTypeBuilderCallingConvention(self.handle) }; if convention_confidence.convention.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderCallingConvention)) } else { Ok(convention_confidence.into()) } @@ -271,28 +270,28 @@ impl TypeBuilder { unsafe { BNTypeBuilderHasVariableArguments(self.handle).into() } } - pub fn get_structure(&self) -> Result> { + pub fn get_structure(&self) -> BNResult> { let result = unsafe { BNGetTypeBuilderStructure(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderStructure)) } else { Ok(unsafe { Structure::ref_from_raw(result) }) } } - pub fn get_enumeration(&self) -> Result> { + pub fn get_enumeration(&self) -> BNResult> { let result = unsafe { BNGetTypeBuilderEnumeration(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderEnumeration)) } else { Ok(Enumeration::ref_from_raw(result)) } } - pub fn get_named_type_reference(&self) -> Result { + pub fn get_named_type_reference(&self) -> BNResult { let result = unsafe { BNGetTypeBuilderNamedTypeReference(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeBuilderNamedTypeReference)) } else { Ok(unsafe { NamedTypeReference::from_raw(result) }) } @@ -572,37 +571,37 @@ impl Type { unsafe { BNIsTypeFloatingPoint(self.handle) } } - pub fn target(&self) -> Result>> { + pub fn target(&self) -> BNResult>> { let raw_target = unsafe { BNGetChildType(self.handle) }; if raw_target.type_.is_null() { - Err(()) + Err(bn_api_error!(BNGetChildType)) } else { Ok(raw_target.into()) } } - pub fn element_type(&self) -> Result>> { + pub fn element_type(&self) -> BNResult>> { let raw_target = unsafe { BNGetChildType(self.handle) }; if raw_target.type_.is_null() { - Err(()) + Err(bn_api_error!(BNGetChildType)) } else { Ok(raw_target.into()) } } - pub fn return_value(&self) -> Result>> { + pub fn return_value(&self) -> BNResult>> { let raw_target = unsafe { BNGetChildType(self.handle) }; if raw_target.type_.is_null() { - Err(()) + Err(bn_api_error!(BNGetChildType)) } else { Ok(raw_target.into()) } } - pub fn calling_convention(&self) -> Result>>> { + pub fn calling_convention(&self) -> BNResult>>> { let convention_confidence = unsafe { BNGetTypeCallingConvention(self.handle) }; if convention_confidence.convention.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeCallingConvention)) } else { Ok(convention_confidence.into()) } @@ -619,28 +618,28 @@ impl Type { unsafe { BNFunctionTypeCanReturn(self.handle).into() } } - pub fn get_structure(&self) -> Result> { + pub fn get_structure(&self) -> BNResult> { let result = unsafe { BNGetTypeStructure(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeStructure)) } else { Ok(unsafe { Structure::ref_from_raw(result) }) } } - pub fn get_enumeration(&self) -> Result> { + pub fn get_enumeration(&self) -> BNResult> { let result = unsafe { BNGetTypeEnumeration(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeEnumeration)) } else { Ok(Enumeration::ref_from_raw(result)) } } - pub fn get_named_type_reference(&self) -> Result { + pub fn get_named_type_reference(&self) -> BNResult { let result = unsafe { BNGetTypeNamedTypeReference(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetTypeNamedTypeReference)) } else { Ok(unsafe { NamedTypeReference::from_raw(result) }) } @@ -658,10 +657,10 @@ impl Type { unsafe { BNGetTypeStackAdjustment(self.handle).into() } } - pub fn registered_name(&self) -> Result { + pub fn registered_name(&self) -> BNResult { let result = unsafe { BNGetRegisteredTypeName(self.handle) }; if result.is_null() { - Err(()) + Err(bn_api_error!(BNGetRegisteredTypeName)) } else { Ok(unsafe { NamedTypeReference::from_raw(result) }) } From 552b6c2a6e837570fbea300a6def433b0c5ff64f Mon Sep 17 00:00:00 2001 From: Lukas Dresel Date: Fri, 18 Feb 2022 16:11:25 -0500 Subject: [PATCH 4/4] added support for context in errors, fixed open_view logic, added printing of full dependent backtraces --- rust/src/binaryview.rs | 2 +- rust/src/demangle.rs | 12 ++++--- rust/src/errors.rs | 74 ++++++++++++++++++++++++++++++++---------- rust/src/lib.rs | 22 +++++++------ 4 files changed, 77 insertions(+), 33 deletions(-) diff --git a/rust/src/binaryview.rs b/rust/src/binaryview.rs index 0433a8955..8ce8408a7 100644 --- a/rust/src/binaryview.rs +++ b/rust/src/binaryview.rs @@ -138,7 +138,7 @@ pub trait BinaryViewExt: BinaryViewBase { let handle = unsafe { BNGetParentView(self.as_ref().handle) }; if handle.is_null() { - return Err(BNError::api_error("BNGetParentView", None)); + return Err(bn_api_error!(BNGetParentView)); } unsafe { Ok(BinaryView::from_raw(handle)) } diff --git a/rust/src/demangle.rs b/rust/src/demangle.rs index d9b743322..8a8e912da 100644 --- a/rust/src/demangle.rs +++ b/rust/src/demangle.rs @@ -33,8 +33,10 @@ pub fn demangle_gnu3( let cstr = match CStr::from_bytes_with_nul(mangled_name_ptr) { Ok(cstr) => cstr, Err(e) => { - log::error!("demangle_gnu3: failed to parse mangled name"); - return Err(e.into()); + let msg = "demangle_gnu3: failed to parse mangled name"; + log::error!("{}", msg); + let e: BNError = e.into(); + return Err(e.contextualize(msg)); } }; return Ok((None, vec![cstr.to_string_lossy().into_owned()])); @@ -94,8 +96,10 @@ pub fn demangle_ms( let cstr = match CStr::from_bytes_with_nul(mangled_name_ptr) { Ok(cstr) => cstr, Err(e) => { - log::error!("demangle_ms: failed to parse mangled name"); - return Err(e.into()); + let msg = "demangle_ms: failed to parse mangled name"; + log::error!("{}", msg); + let e: BNError = e.into(); + return Err(e.contextualize(msg)); } }; return Ok((None, vec![cstr.to_string_lossy().into_owned()])); diff --git a/rust/src/errors.rs b/rust/src/errors.rs index 8e70d321b..4ebb107c5 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -1,18 +1,45 @@ use std::{fmt, backtrace::Backtrace, error::Error, ffi::FromBytesWithNulError}; #[allow(dead_code)] // it's used in `Debug` which is used in `Display` which is used to show the error -#[derive(Debug)] pub struct BNError { repr: BNErrorRepr, backtrace: Option, + cause: Option>, + context: Vec, +} +impl std::fmt::Display for BNError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "\nEncountered error: {:?}\n", self.repr)?; + if self.context.len() > 0 { + writeln!(f, "Contextual information: ")?; + for m in &self.context { + writeln!(f, " - {}", m)?; + } + } + + if let Some(bt) = &self.backtrace { + writeln!(f, "Backtrace: ")?; + writeln!(f, "{:#}", bt)?; + } + if let Some(cause) = self.source() { + writeln!(f, "Caused by: {:?}", cause)?; + } + + Ok(()) + } +} + +impl fmt::Debug for BNError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + ::fmt(self, f) + } } #[derive(Debug)] pub enum BNErrorRepr { - Generic(String), + Generic, APIError { api_function: String, other_info: Option}, - IOError { cause: std::io::Error }, - FFIStrDecodeError { cause: FromBytesWithNulError } + Chained, } macro_rules! bn_api_error { @@ -39,18 +66,14 @@ impl<'a> std::fmt::Display for Utf8Display<'a> { } } -impl fmt::Display for BNError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - ::fmt(self, f) - } -} - impl BNError { #[inline(always)] - pub fn generic(msg: &str) -> BNError { + pub fn generic(context: &str) -> BNError { BNError { - repr: BNErrorRepr::Generic(String::from(msg)), + repr: BNErrorRepr::Generic, backtrace: Some(Backtrace::capture()), + cause: None, + context: vec![String::from(context)], } } #[inline(always)] @@ -61,8 +84,19 @@ impl BNError { other_info: other_info.map(String::from), }, backtrace: Some(Backtrace::capture()), + cause: None, + context: Default::default(), } } + pub fn contextualize(mut self, msg: &str) -> Self{ + self.context.push(String::from(msg)); + self + } + pub fn caused_by(mut self, e: T) -> Self { + assert!(self.cause.take().is_none()); + self.cause = Some(Box::new(e)); + self + } } impl Error for BNError { @@ -73,8 +107,8 @@ impl Error for BNError { .or(self.backtrace.as_ref()) } fn source(&self) -> Option<&(dyn Error + 'static)> { - match &self.repr { - BNErrorRepr::IOError { cause } => Some(cause), + match &self.cause { + Some(x) => Some(x.as_ref()), _ => None, } } @@ -83,16 +117,20 @@ impl Error for BNError { impl From for BNError { fn from(e: FromBytesWithNulError) -> Self { BNError { - repr: BNErrorRepr::FFIStrDecodeError { cause: e }, - backtrace: None // we rely on the `cause` Backtrace + repr: BNErrorRepr::Chained, + backtrace: Some(Backtrace::capture()), + cause: Some(Box::new(e)), + context: Default::default() } } } impl From for BNError { fn from(e: std::io::Error) -> Self { BNError { - repr: BNErrorRepr::IOError { cause: e }, - backtrace: None // we rely on the `cause` Backtrace + repr: BNErrorRepr::Chained, + backtrace: Some(Backtrace::capture()), + cause: Some(Box::new(e)), + context: Default::default() } } } \ No newline at end of file diff --git a/rust/src/lib.rs b/rust/src/lib.rs index accf91631..3136eff24 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -229,7 +229,7 @@ pub mod logger { } } -pub fn open_view>(filename: F) -> Result, String> { +pub fn open_view>(filename: F) -> BNResult> { use crate::binaryview::BinaryViewExt; use crate::custombinaryview::BinaryViewTypeExt; @@ -238,14 +238,17 @@ pub fn open_view>(filename: F) -> Result>(filename: F) -> Result>(filename: F) -> Result>(filename: F) -> Result