diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64cb7ce..4882f77 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,3 +44,13 @@ jobs: - uses: actions/checkout@v2 - uses: dtolnay/rust-toolchain@clippy - run: cargo clippy --tests -- -Dclippy::all -Dclippy::pedantic + + miri: + name: Miri + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: dtolnay/rust-toolchain@nightly + with: + components: miri + - run: cargo miri test diff --git a/src/context.rs b/src/context.rs index 25d3411..9ca9096 100644 --- a/src/context.rs +++ b/src/context.rs @@ -143,7 +143,7 @@ where } fn source(&self) -> Option<&(dyn StdError + 'static)> { - Some(self.error.inner.error()) + Some(unsafe { crate::ErrorImpl::error(self.error.inner) }) } } diff --git a/src/error.rs b/src/error.rs index 2618814..62cdf41 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,7 +4,7 @@ use crate::chain::Chain; use crate::{Error, StdError}; use core::any::TypeId; use core::fmt::{self, Debug, Display}; -use core::mem::{self, ManuallyDrop}; +use core::mem::ManuallyDrop; use core::ptr::{self, NonNull}; #[cfg(feature = "std")] @@ -83,6 +83,7 @@ impl Error { object_mut: object_mut::, object_boxed: object_boxed::, object_downcast: object_downcast::, + object_downcast_mut: object_downcast_mut::, object_drop_rest: object_drop_front::, }; @@ -103,6 +104,7 @@ impl Error { object_mut: object_mut::>, object_boxed: object_boxed::>, object_downcast: object_downcast::, + object_downcast_mut: object_downcast_mut::, object_drop_rest: object_drop_front::, }; @@ -124,6 +126,7 @@ impl Error { object_mut: object_mut::>, object_boxed: object_boxed::>, object_downcast: object_downcast::, + object_downcast_mut: object_downcast_mut::, object_drop_rest: object_drop_front::, }; @@ -146,6 +149,7 @@ impl Error { object_mut: object_mut::>, object_boxed: object_boxed::>, object_downcast: context_downcast::, + object_downcast_mut: context_downcast_mut::, object_drop_rest: context_drop_rest::, }; @@ -166,6 +170,7 @@ impl Error { object_mut: object_mut::, object_boxed: object_boxed::, object_downcast: object_downcast::>, + object_downcast_mut: object_downcast_mut::>, object_drop_rest: object_drop_front::>, }; @@ -187,7 +192,7 @@ impl Error { where E: StdError + Send + Sync + 'static, { - let inner = Box::new(ErrorImpl { + let inner: Box> = Box::new(ErrorImpl { vtable, backtrace, _object: error, @@ -198,8 +203,7 @@ impl Error { // result is a thin pointer. The necessary behavior for manipulating the // underlying ErrorImpl is preserved in the vtable provided by the // caller rather than a builtin fat pointer vtable. - let erased = mem::transmute::>, Box>>(inner); - let inner = ManuallyDrop::new(erased); + let inner = NonNull::new_unchecked(Box::into_raw(inner) as *mut ErrorImpl<()>); Error { inner } } @@ -273,6 +277,7 @@ impl Error { object_mut: object_mut::>, object_boxed: object_boxed::>, object_downcast: context_chain_downcast::, + object_downcast_mut: context_chain_downcast_mut::, object_drop_rest: context_chain_drop_rest::, }; @@ -304,7 +309,7 @@ impl Error { /// [tracking]: https://github.com/rust-lang/rust/issues/53487 #[cfg(backtrace)] pub fn backtrace(&self) -> &Backtrace { - self.inner.backtrace() + unsafe { ErrorImpl::backtrace(self.inner) } } /// An iterator of the chain of source errors contained by this Error. @@ -331,7 +336,7 @@ impl Error { #[cfg(feature = "std")] #[cfg_attr(doc_cfg, doc(cfg(feature = "std")))] pub fn chain(&self) -> Chain { - self.inner.chain() + unsafe { ErrorImpl::chain(self.inner) } } /// The lowest level cause of this error — this error's cause's @@ -369,7 +374,7 @@ impl Error { unsafe { // Use vtable to find NonNull<()> which points to a value of type E // somewhere inside the data structure. - let addr = match (self.inner.vtable.object_downcast)(&self.inner, target) { + let addr = match (vtable(self.inner).object_downcast_mut)(self.inner, target) { Some(addr) => addr, None => return Err(self), }; @@ -381,13 +386,8 @@ impl Error { // Read E from where the vtable found it. let error = ptr::read(addr.cast::().as_ptr()); - // Read Box> from self. Can't move it out because - // Error has a Drop impl which we want to not run. - let inner = ptr::read(&outer.inner); - let erased = ManuallyDrop::into_inner(inner); - // Drop rest of the data structure outside of E. - (erased.vtable.object_drop_rest)(erased, target); + (vtable(outer.inner).object_drop_rest)(outer.inner, target); Ok(error) } @@ -437,7 +437,7 @@ impl Error { unsafe { // Use vtable to find NonNull<()> which points to a value of type E // somewhere inside the data structure. - let addr = (self.inner.vtable.object_downcast)(&self.inner, target)?; + let addr = (vtable(self.inner).object_downcast)(self.inner, target)?; Some(&*addr.cast::().as_ptr()) } } @@ -451,7 +451,7 @@ impl Error { unsafe { // Use vtable to find NonNull<()> which points to a value of type E // somewhere inside the data structure. - let addr = (self.inner.vtable.object_downcast)(&self.inner, target)?; + let addr = (vtable(self.inner).object_downcast_mut)(self.inner, target)?; Some(&mut *addr.cast::().as_ptr()) } } @@ -475,7 +475,7 @@ impl Deref for Error { type Target = dyn StdError + Send + Sync + 'static; fn deref(&self) -> &Self::Target { - self.inner.error() + unsafe { ErrorImpl::error(self.inner) } } } @@ -483,100 +483,98 @@ impl Deref for Error { #[cfg_attr(doc_cfg, doc(cfg(feature = "std")))] impl DerefMut for Error { fn deref_mut(&mut self) -> &mut Self::Target { - self.inner.error_mut() + unsafe { ErrorImpl::error_mut(self.inner) } } } impl Display for Error { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - self.inner.display(formatter) + unsafe { ErrorImpl::display(self.inner, formatter) } } } impl Debug for Error { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - self.inner.debug(formatter) + unsafe { ErrorImpl::debug(self.inner, formatter) } } } impl Drop for Error { fn drop(&mut self) { unsafe { - // Read Box> from self. - let inner = ptr::read(&self.inner); - let erased = ManuallyDrop::into_inner(inner); - // Invoke the vtable's drop behavior. - (erased.vtable.object_drop)(erased); + (vtable(self.inner).object_drop)(self.inner); } } } struct ErrorVTable { - object_drop: unsafe fn(Box>), - object_ref: unsafe fn(&ErrorImpl<()>) -> &(dyn StdError + Send + Sync + 'static), + object_drop: unsafe fn(NonNull>), + object_ref: unsafe fn(NonNull>) -> *const (dyn StdError + Send + Sync + 'static), #[cfg(feature = "std")] - object_mut: unsafe fn(&mut ErrorImpl<()>) -> &mut (dyn StdError + Send + Sync + 'static), - object_boxed: unsafe fn(Box>) -> Box, - object_downcast: unsafe fn(&ErrorImpl<()>, TypeId) -> Option>, - object_drop_rest: unsafe fn(Box>, TypeId), + object_mut: unsafe fn(NonNull>) -> *mut (dyn StdError + Send + Sync + 'static), + object_boxed: unsafe fn(NonNull>) -> Box, + object_downcast: unsafe fn(NonNull>, TypeId) -> Option>, + object_downcast_mut: unsafe fn(NonNull>, TypeId) -> Option>, + object_drop_rest: unsafe fn(NonNull>, TypeId), } // Safety: requires layout of *e to match ErrorImpl. -unsafe fn object_drop(e: Box>) { +unsafe fn object_drop(e: NonNull>) { // Cast back to ErrorImpl so that the allocator receives the correct // Layout to deallocate the Box's memory. - let unerased = mem::transmute::>, Box>>(e); + let unerased = Box::from_raw(e.as_ptr() as *mut ErrorImpl); drop(unerased); } // Safety: requires layout of *e to match ErrorImpl. -unsafe fn object_drop_front(e: Box>, target: TypeId) { +unsafe fn object_drop_front(e: NonNull>, target: TypeId) { // Drop the fields of ErrorImpl other than E as well as the Box allocation, // without dropping E itself. This is used by downcast after doing a // ptr::read to take ownership of the E. let _ = target; - let unerased = mem::transmute::>, Box>>>(e); + let unerased = Box::from_raw(e.as_ptr() as *mut ErrorImpl>); drop(unerased); } // Safety: requires layout of *e to match ErrorImpl. -unsafe fn object_ref(e: &ErrorImpl<()>) -> &(dyn StdError + Send + Sync + 'static) +unsafe fn object_ref(e: NonNull>) -> *const (dyn StdError + Send + Sync + 'static) where E: StdError + Send + Sync + 'static, { // Attach E's native StdError vtable onto a pointer to self._object. - &(*(e as *const ErrorImpl<()> as *const ErrorImpl))._object + &(*(e.as_ptr() as *const ErrorImpl))._object } -// Safety: requires layout of *e to match ErrorImpl. +// Safety: requires layout of *e to match ErrorImpl, and for `e` to be derived +// from a `&mut` #[cfg(feature = "std")] -unsafe fn object_mut(e: &mut ErrorImpl<()>) -> &mut (dyn StdError + Send + Sync + 'static) +unsafe fn object_mut(e: NonNull>) -> *mut (dyn StdError + Send + Sync + 'static) where E: StdError + Send + Sync + 'static, { // Attach E's native StdError vtable onto a pointer to self._object. - &mut (*(e as *mut ErrorImpl<()> as *mut ErrorImpl))._object + &mut (*(e.as_ptr() as *mut ErrorImpl))._object } // Safety: requires layout of *e to match ErrorImpl. -unsafe fn object_boxed(e: Box>) -> Box +unsafe fn object_boxed(e: NonNull>) -> Box where E: StdError + Send + Sync + 'static, { // Attach ErrorImpl's native StdError vtable. The StdError impl is below. - mem::transmute::>, Box>>(e) + Box::>::from_raw(e.as_ptr() as *mut ErrorImpl) } // Safety: requires layout of *e to match ErrorImpl. -unsafe fn object_downcast(e: &ErrorImpl<()>, target: TypeId) -> Option> +unsafe fn object_downcast(e: NonNull>, target: TypeId) -> Option> where E: 'static, { if TypeId::of::() == target { // Caller is looking for an E pointer and e is ErrorImpl, take a // pointer to its E field. - let unerased = e as *const ErrorImpl<()> as *const ErrorImpl; + let unerased = e.as_ptr() as *const ErrorImpl; let addr = &(*unerased)._object as *const E as *mut (); Some(NonNull::new_unchecked(addr)) } else { @@ -584,19 +582,35 @@ where } } +// Safety: requires layout of *e to match ErrorImpl. +unsafe fn object_downcast_mut(e: NonNull>, target: TypeId) -> Option> +where + E: 'static, +{ + if TypeId::of::() == target { + // Caller is looking for an E pointer and e is ErrorImpl, take a + // pointer to its E field. + let unerased = e.as_ptr() as *mut ErrorImpl; + let addr = &mut (*unerased)._object as *mut E as *mut (); + Some(NonNull::new_unchecked(addr)) + } else { + None + } +} + // Safety: requires layout of *e to match ErrorImpl>. #[cfg(feature = "std")] -unsafe fn context_downcast(e: &ErrorImpl<()>, target: TypeId) -> Option> +unsafe fn context_downcast(e: NonNull>, target: TypeId) -> Option> where C: 'static, E: 'static, { if TypeId::of::() == target { - let unerased = e as *const ErrorImpl<()> as *const ErrorImpl>; + let unerased = e.as_ptr() as *const ErrorImpl>; let addr = &(*unerased)._object.context as *const C as *mut (); Some(NonNull::new_unchecked(addr)) } else if TypeId::of::() == target { - let unerased = e as *const ErrorImpl<()> as *const ErrorImpl>; + let unerased = e.as_ptr() as *const ErrorImpl>; let addr = &(*unerased)._object.error as *const E as *mut (); Some(NonNull::new_unchecked(addr)) } else { @@ -606,7 +620,30 @@ where // Safety: requires layout of *e to match ErrorImpl>. #[cfg(feature = "std")] -unsafe fn context_drop_rest(e: Box>, target: TypeId) +unsafe fn context_downcast_mut( + e: NonNull>, + target: TypeId, +) -> Option> +where + C: 'static, + E: 'static, +{ + if TypeId::of::() == target { + let unerased = e.as_ptr() as *mut ErrorImpl>; + let addr = &mut (*unerased)._object.context as *mut C as *mut (); + Some(NonNull::new_unchecked(addr)) + } else if TypeId::of::() == target { + let unerased = e.as_ptr() as *mut ErrorImpl>; + let addr = &mut (*unerased)._object.error as *mut E as *mut (); + Some(NonNull::new_unchecked(addr)) + } else { + None + } +} + +// Safety: requires layout of *e to match ErrorImpl>. +#[cfg(feature = "std")] +unsafe fn context_drop_rest(e: NonNull>, target: TypeId) where C: 'static, E: 'static, @@ -614,65 +651,82 @@ where // Called after downcasting by value to either the C or the E and doing a // ptr::read to take ownership of that value. if TypeId::of::() == target { - let unerased = mem::transmute::< - Box>, - Box, E>>>, - >(e); + let unerased = + Box::from_raw(e.as_ptr() as *mut ErrorImpl, E>>); drop(unerased); } else { - let unerased = mem::transmute::< - Box>, - Box>>>, - >(e); + let unerased = + Box::from_raw(e.as_ptr() as *mut ErrorImpl>>); drop(unerased); } } // Safety: requires layout of *e to match ErrorImpl>. -unsafe fn context_chain_downcast(e: &ErrorImpl<()>, target: TypeId) -> Option> +unsafe fn context_chain_downcast( + e: NonNull>, + target: TypeId, +) -> Option> where C: 'static, { if TypeId::of::() == target { - let unerased = e as *const ErrorImpl<()> as *const ErrorImpl>; + let unerased = e.as_ptr() as *const ErrorImpl>; let addr = &(*unerased)._object.context as *const C as *mut (); Some(NonNull::new_unchecked(addr)) } else { // Recurse down the context chain per the inner error's vtable. - let unerased = e as *const ErrorImpl<()> as *const ErrorImpl>; + let unerased = e.as_ptr() as *const ErrorImpl>; let source = &(*unerased)._object.error; - (source.inner.vtable.object_downcast)(&source.inner, target) + (vtable(source.inner).object_downcast)(source.inner, target) + } +} + +// Safety: requires layout of *e to match ErrorImpl>. +unsafe fn context_chain_downcast_mut( + e: NonNull>, + target: TypeId, +) -> Option> +where + C: 'static, +{ + if TypeId::of::() == target { + let unerased = e.as_ptr() as *mut ErrorImpl>; + let addr = &mut (*unerased)._object.context as *mut C as *mut (); + Some(NonNull::new_unchecked(addr)) + } else { + // Recurse down the context chain per the inner error's vtable. + let unerased = e.as_ptr() as *mut ErrorImpl>; + let source = &mut (*unerased)._object.error; + (vtable(source.inner).object_downcast_mut)(source.inner, target) } } // Safety: requires layout of *e to match ErrorImpl>. -unsafe fn context_chain_drop_rest(e: Box>, target: TypeId) +unsafe fn context_chain_drop_rest(e: NonNull>, target: TypeId) where C: 'static, { // Called after downcasting by value to either the C or one of the causes // and doing a ptr::read to take ownership of that value. if TypeId::of::() == target { - let unerased = mem::transmute::< - Box>, - Box, Error>>>, - >(e); + let unerased = + Box::from_raw(e.as_ptr() as *mut ErrorImpl, Error>>); // Drop the entire rest of the data structure rooted in the next Error. drop(unerased); } else { - let unerased = mem::transmute::< - Box>, - Box>>>, - >(e); - // Read out a ManuallyDrop>> from the next error. - let inner = ptr::read(&unerased._object.error.inner); + let unerased = + Box::from_raw(e.as_ptr() as *mut ErrorImpl>>); + // Read the NonNull> from the next error. + let inner = unerased._object.error.inner; drop(unerased); - let erased = ManuallyDrop::into_inner(inner); + let vtable = inner.as_ref().vtable; // Recursively drop the next error using the same target typeid. - (erased.vtable.object_drop_rest)(erased, target); + (vtable.object_drop_rest)(inner, target); } } +// NOTE: If working with `ErrorImpl<()>`, references should be avoided in favor +// of raw pointers and `NonNull`. // repr C to ensure that E remains in the final position. #[repr(C)] pub(crate) struct ErrorImpl { @@ -683,6 +737,14 @@ pub(crate) struct ErrorImpl { _object: E, } +// Reads the vtable out of `p`. This is the same as `p.as_ref().vtable`, but +// avoids converting `p` into a reference. +#[inline] +unsafe fn vtable(p: NonNull>) -> &'static ErrorVTable { + // NOTE: This assumes that `ErrorVTable` is the first field of ErrorImpl. + (p.as_ptr() as *const &'static ErrorVTable).read() +} + // repr C to ensure that ContextError has the same layout as // ContextError, E> and ContextError>. #[repr(C)] @@ -692,41 +754,46 @@ pub(crate) struct ContextError { } impl ErrorImpl { - fn erase(&self) -> &ErrorImpl<()> { + fn erase(&self) -> NonNull> { // Erase the concrete type of E but preserve the vtable in self.vtable // for manipulating the resulting thin pointer. This is analogous to an // unsize coercion. - unsafe { &*(self as *const ErrorImpl as *const ErrorImpl<()>) } + NonNull::from(self).cast::>() } } impl ErrorImpl<()> { - pub(crate) fn error(&self) -> &(dyn StdError + Send + Sync + 'static) { + pub(crate) unsafe fn error<'a>( + this: NonNull, + ) -> &'a (dyn StdError + Send + Sync + 'static) { // Use vtable to attach E's native StdError vtable for the right // original type E. - unsafe { &*(self.vtable.object_ref)(self) } + &*(vtable(this).object_ref)(this) } #[cfg(feature = "std")] - pub(crate) fn error_mut(&mut self) -> &mut (dyn StdError + Send + Sync + 'static) { + pub(crate) unsafe fn error_mut<'a>( + this: NonNull, + ) -> &'a mut (dyn StdError + Send + Sync + 'static) { // Use vtable to attach E's native StdError vtable for the right // original type E. - unsafe { &mut *(self.vtable.object_mut)(self) } + &mut *(vtable(this).object_mut)(this) } #[cfg(backtrace)] - pub(crate) fn backtrace(&self) -> &Backtrace { + pub(crate) unsafe fn backtrace<'a>(this: NonNull) -> &'a Backtrace { // This unwrap can only panic if the underlying error's backtrace method // is nondeterministic, which would only happen in maliciously // constructed code. - self.backtrace + (*this.as_ptr()) + .backtrace .as_ref() - .or_else(|| self.error().backtrace()) + .or_else(|| Self::error(this).backtrace()) .expect("backtrace capture failed") } - pub(crate) fn chain(&self) -> Chain { - Chain::new(self.error()) + pub(crate) unsafe fn chain<'a>(this: NonNull) -> Chain<'a> { + Chain::new(Self::error(this)) } } @@ -736,11 +803,11 @@ where { #[cfg(backtrace)] fn backtrace(&self) -> Option<&Backtrace> { - Some(self.erase().backtrace()) + Some(unsafe { ErrorImpl::backtrace(self.erase()) }) } fn source(&self) -> Option<&(dyn StdError + 'static)> { - self.erase().error().source() + unsafe { ErrorImpl::error(self.erase()).source() } } } @@ -749,7 +816,7 @@ where E: Debug, { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - self.erase().debug(formatter) + unsafe { ErrorImpl::debug(self.erase(), formatter) } } } @@ -758,7 +825,7 @@ where E: Display, { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - Display::fmt(&self.erase().error(), formatter) + unsafe { Display::fmt(ErrorImpl::error(self.erase()), formatter) } } } @@ -766,14 +833,9 @@ impl From for Box { fn from(error: Error) -> Self { let outer = ManuallyDrop::new(error); unsafe { - // Read Box> from error. Can't move it out because - // Error has a Drop impl which we want to not run. - let inner = ptr::read(&outer.inner); - let erased = ManuallyDrop::into_inner(inner); - // Use vtable to attach ErrorImpl's native StdError vtable for // the right original type E. - (erased.vtable.object_boxed)(erased) + (vtable(outer.inner).object_boxed)(outer.inner) } } } diff --git a/src/fmt.rs b/src/fmt.rs index be93e1a..7b7b159 100644 --- a/src/fmt.rs +++ b/src/fmt.rs @@ -1,13 +1,14 @@ use crate::chain::Chain; use crate::error::ErrorImpl; use core::fmt::{self, Debug, Write}; +use core::ptr::NonNull; impl ErrorImpl<()> { - pub(crate) fn display(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.error())?; + pub(crate) unsafe fn display(this: NonNull, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", Self::error(this))?; if f.alternate() { - for cause in self.chain().skip(1) { + for cause in Self::chain(this).skip(1) { write!(f, ": {}", cause)?; } } @@ -15,8 +16,8 @@ impl ErrorImpl<()> { Ok(()) } - pub(crate) fn debug(&self, f: &mut fmt::Formatter) -> fmt::Result { - let error = self.error(); + pub(crate) unsafe fn debug(this: NonNull, f: &mut fmt::Formatter) -> fmt::Result { + let error = Self::error(this); if f.alternate() { return Debug::fmt(error, f); @@ -42,7 +43,7 @@ impl ErrorImpl<()> { { use std::backtrace::BacktraceStatus; - let backtrace = self.backtrace(); + let backtrace = Self::backtrace(this); if let BacktraceStatus::Captured = backtrace.status() { let mut backtrace = backtrace.to_string(); write!(f, "\n\n")?; diff --git a/src/lib.rs b/src/lib.rs index c3a863b..1f94ea5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -248,10 +248,9 @@ mod kind; mod macros; mod wrapper; -use crate::alloc::Box; use crate::error::ErrorImpl; use core::fmt::Display; -use core::mem::ManuallyDrop; +use core::ptr::NonNull; #[cfg(not(feature = "std"))] use core::fmt::Debug; @@ -371,9 +370,12 @@ pub use anyhow as format_err; /// ``` #[repr(transparent)] pub struct Error { - inner: ManuallyDrop>>, + inner: NonNull>, } +unsafe impl Send for Error {} +unsafe impl Sync for Error {} + /// Iterator of a chain of source errors. /// /// This type is the iterator returned by [`Error::chain`]. diff --git a/tests/test_downcast.rs b/tests/test_downcast.rs index 62f4e98..6b93710 100644 --- a/tests/test_downcast.rs +++ b/tests/test_downcast.rs @@ -68,6 +68,12 @@ fn test_downcast_mut() { .unwrap() .to_string(), ); + + let mut bailed = bail_fmt().unwrap_err(); + *bailed.downcast_mut::().unwrap() = "clobber".to_string(); + assert_eq!(bailed.downcast_ref::().unwrap(), "clobber"); + assert_eq!(bailed.downcast_mut::().unwrap(), "clobber"); + assert_eq!(bailed.downcast::().unwrap(), "clobber"); } #[test]