diff --git a/src/header.rs b/src/header.rs index e24fd89..c606fb2 100644 --- a/src/header.rs +++ b/src/header.rs @@ -251,19 +251,18 @@ impl From> for Arc<[T]> { /// /// Safety-usable invariants: /// -/// - This is guaranteed to have the same representation as `HeaderSlice, T>` (i.e. `HeaderSliceWithLengthUnchecked`) -/// - For `T` that is `[U]` or `str`, the header length (`.length()` is checked to be the slice length) +/// - This is guaranteed to have the same representation as `HeaderSlice, [T]>` +/// - The header length (`.length()`) is checked to be the slice length #[derive(Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] #[repr(transparent)] -pub struct HeaderSliceWithLengthProtected { - // Invariant: if T is [U] or str, then the header's length field must be the slice length - // Currently no other DSTs are used with this type amd it has no invariants, but these may be added in the future +pub struct HeaderSliceWithLengthProtected { + // Invariant: the header's length field must be the slice length inner: HeaderSliceWithLengthUnchecked, } -pub(crate) type HeaderSliceWithLengthUnchecked = HeaderSlice, T>; +pub(crate) type HeaderSliceWithLengthUnchecked = HeaderSlice, [T]>; -impl HeaderSliceWithLengthProtected { +impl HeaderSliceWithLengthProtected { pub fn header(&self) -> &H { &self.inner.header.header } @@ -275,10 +274,10 @@ impl HeaderSliceWithLengthProtected { self.inner.header.length } - pub fn slice(&self) -> &T { + pub fn slice(&self) -> &[T] { &self.inner.slice } - pub fn slice_mut(&mut self) -> &mut T { + pub fn slice_mut(&mut self) -> &mut [T] { // Safety: only the length is unsafe to mutate &mut self.inner.slice } @@ -288,13 +287,13 @@ impl HeaderSliceWithLengthProtected { } } -impl PartialOrd for HeaderSliceWithLengthUnchecked { +impl PartialOrd for HeaderSlice, T> { fn partial_cmp(&self, other: &Self) -> Option { (&self.header.header, &self.slice).partial_cmp(&(&other.header.header, &other.slice)) } } -impl Ord for HeaderSliceWithLengthUnchecked { +impl Ord for HeaderSlice, T> { fn cmp(&self, other: &Self) -> Ordering { (&self.header.header, &self.slice).cmp(&(&other.header.header, &other.slice)) } diff --git a/src/thin_arc.rs b/src/thin_arc.rs index 6d51c2c..7158924 100644 --- a/src/thin_arc.rs +++ b/src/thin_arc.rs @@ -4,12 +4,11 @@ use core::fmt; use core::hash::{Hash, Hasher}; use core::iter::{ExactSizeIterator, Iterator}; use core::marker::PhantomData; -use core::mem; use core::mem::ManuallyDrop; use core::ops::Deref; use core::ptr; -use super::{Arc, ArcInner, HeaderSliceWithLengthProtected, HeaderWithLength}; +use super::{Arc, ArcInner, HeaderSlice, HeaderSliceWithLengthProtected, HeaderWithLength}; use crate::header::HeaderSliceWithLengthUnchecked; /// A "thin" `Arc` containing dynamically sized data @@ -29,7 +28,19 @@ use crate::header::HeaderSliceWithLengthUnchecked; /// via `HeaderSliceWithLengthProtected`. #[repr(transparent)] pub struct ThinArc { - ptr: ptr::NonNull>>, + // We can pointer-cast between this target type + // of `ArcInner, [T; 0]>` + // and the types + // `ArcInner>` and + // `ArcInner>` (= `ArcInner, [T]>>`). + // [By adding appropriate length metadata to the pointer.] + // All types involved are #[repr(C)] or #[repr(transparent)], to ensure the safety of such casts + // (in particular `HeaderSlice`, `HeaderWithLength`, `HeaderSliceWithLengthProtected`). + // + // The safe API of `ThinArc` ensures that the length in the `HeaderWithLength` + // corretcly set - or verified - upon creation of a `ThinArc` and can't be modified + // to fall out of sync with the true slice length for this value & allocation. + ptr: ptr::NonNull, [T; 0]>>>, phantom: PhantomData<(H, T)>, } @@ -40,13 +51,12 @@ unsafe impl Sync for ThinArc {} // // See the comment around the analogous operation in from_header_and_iter. #[inline] -fn thin_to_thick( - thin: *mut ArcInner>, -) -> *mut ArcInner> { - let len = unsafe { (*thin).data.length() }; +fn thin_to_thick(arc: &ThinArc) -> *mut ArcInner> { + let thin = arc.ptr.as_ptr(); + let len = unsafe { (*thin).data.header.length }; let fake_slice = ptr::slice_from_raw_parts_mut(thin as *mut T, len); - fake_slice as *mut ArcInner> + fake_slice as *mut ArcInner> } impl ThinArc { @@ -55,15 +65,12 @@ impl ThinArc { #[inline] pub fn with_arc(&self, f: F) -> U where - F: FnOnce(&Arc>) -> U, + F: FnOnce(&Arc>) -> U, { // Synthesize transient Arc, which never touches the refcount of the ArcInner. - let transient = unsafe { - ManuallyDrop::new(Arc::from_protected(Arc { - p: ptr::NonNull::new_unchecked(thin_to_thick(self.ptr.as_ptr())), - phantom: PhantomData, - })) - }; + let transient = ManuallyDrop::new(Arc::from_protected(unsafe { + Arc::from_raw_inner(thin_to_thick(self)) + })); // Expose the transient Arc to the callback, which may clone it if it wants // and forward the result to the user @@ -75,15 +82,10 @@ impl ThinArc { #[inline] fn with_protected_arc(&self, f: F) -> U where - F: FnOnce(&Arc>) -> U, + F: FnOnce(&Arc>) -> U, { // Synthesize transient Arc, which never touches the refcount of the ArcInner. - let transient = unsafe { - ManuallyDrop::new(Arc { - p: ptr::NonNull::new_unchecked(thin_to_thick(self.ptr.as_ptr())), - phantom: PhantomData, - }) - }; + let transient = ManuallyDrop::new(unsafe { Arc::from_raw_inner(thin_to_thick(self)) }); // Expose the transient Arc to the callback, which may clone it if it wants // and forward the result to the user @@ -95,29 +97,34 @@ impl ThinArc { #[inline] pub fn with_arc_mut(&mut self, f: F) -> U where - F: FnOnce(&mut Arc>) -> U, + F: FnOnce(&mut Arc>) -> U, { // It is possible for the user to replace the Arc entirely here. If so, we need to update the ThinArc as well // whenever this method exits. We do this with a drop guard to handle the panicking case struct DropGuard<'a, H, T> { - transient: ManuallyDrop>>, + transient: ManuallyDrop>>, this: &'a mut ThinArc, } impl<'a, H, T> Drop for DropGuard<'a, H, T> { fn drop(&mut self) { + // This guard is only dropped when the same debug_assert already succeeded + // or while panicking. This has the effect that, if the debug_assert fails, we abort! + // This should never fail, unless a user used `transmute` to violate the invariants of + // `HeaderSliceWithLengthProtected`. + // In this case, there is no sound fallback other than aborting. + debug_assert_eq!( + self.transient.length(), + self.transient.slice().len(), + "Length needs to be correct for ThinArc to work" + ); // Safety: We're still in the realm of Protected types so this cast is safe self.this.ptr = self.transient.p.cast(); } } // Synthesize transient Arc, which never touches the refcount of the ArcInner. - let transient = unsafe { - ManuallyDrop::new(Arc { - p: ptr::NonNull::new_unchecked(thin_to_thick(self.ptr.as_ptr())), - phantom: PhantomData, - }) - }; + let transient = ManuallyDrop::new(unsafe { Arc::from_raw_inner(thin_to_thick(self)) }); let mut guard = DropGuard { transient, @@ -128,6 +135,13 @@ impl ThinArc { // and forward the result to the user let ret = f(&mut guard.transient); + // deliberately checked both here AND in the `DropGuard` + debug_assert_eq!( + guard.transient.length(), + guard.transient.slice().len(), + "Length needs to be correct for ThinArc to work" + ); + ret } @@ -155,7 +169,7 @@ impl ThinArc { /// within it -- for memory reporting. #[inline] pub fn ptr(&self) -> *const c_void { - self.ptr.as_ptr() as *const ArcInner as *const c_void + self.ptr.cast().as_ptr() } /// Returns the address on the heap of the Arc itself -- not the T within it -- for memory @@ -188,7 +202,7 @@ impl ThinArc { #[inline] pub fn into_raw(self) -> *const c_void { let this = ManuallyDrop::new(self); - this.ptr.cast().as_ptr() + this.ptr() } /// Provides a raw pointer to the data. @@ -214,11 +228,11 @@ impl ThinArc { } impl Deref for ThinArc { - type Target = HeaderSliceWithLengthUnchecked; + type Target = HeaderSliceWithLengthUnchecked; #[inline] fn deref(&self) -> &Self::Target { - unsafe { &(*thin_to_thick(self.ptr.as_ptr())).data.inner() } + unsafe { (*thin_to_thick(self)).data.inner() } } } @@ -232,14 +246,14 @@ impl Clone for ThinArc { impl Drop for ThinArc { #[inline] fn drop(&mut self) { - let _ = Arc::from_thin(ThinArc { + let _ = Arc::protected_from_thin(ThinArc { ptr: self.ptr, phantom: PhantomData, }); } } -impl Arc> { +impl Arc> { /// Converts an `Arc` into a `ThinArc`. This consumes the `Arc`, so the refcount /// is not modified. /// @@ -248,7 +262,7 @@ impl Arc> { #[inline] unsafe fn into_thin_unchecked(a: Self) -> ThinArc { // Safety: invariant bubbled up - let this_protected: Arc> = + let this_protected: Arc> = unsafe { Arc::from_unprotected_unchecked(a) }; Arc::protected_into_thin(this_protected) @@ -271,19 +285,21 @@ impl Arc> { /// is not modified. #[inline] pub fn from_thin(a: ThinArc) -> Self { - Self::from_protected(Arc::>::protected_from_thin(a)) + Self::from_protected(Arc::>::protected_from_thin(a)) } /// Converts an `Arc` into a `ThinArc`. This consumes the `Arc`, so the refcount /// is not modified. #[inline] - fn from_protected(a: Arc>) -> Self { + fn from_protected(a: Arc>) -> Self { // Safety: HeaderSliceWithLengthProtected and HeaderSliceWithLengthUnchecked have the same layout - unsafe { mem::transmute(a) } + // The whole `Arc` should also be layout compatible (as a transparent wrapper around `NonNull` pointers with the same + // metadata type) but we still conservatively avoid a direct transmute here and use a pointer-cast instead. + unsafe { Arc::from_raw_inner(Arc::into_raw_inner(a) as _) } } } -impl Arc> { +impl Arc> { /// Converts an `Arc` into a `ThinArc`. This consumes the `Arc`, so the refcount /// is not modified. #[inline] @@ -294,16 +310,11 @@ impl Arc> { "Length needs to be correct for ThinArc to work" ); - let a = ManuallyDrop::new(a); - let fat_ptr: *mut ArcInner> = a.ptr(); - let thin_ptr = fat_ptr as *mut [usize] as *mut usize; + let fat_ptr: *mut ArcInner> = Arc::into_raw_inner(a); + // Safety: The pointer comes from a valid Arc, and HeaderSliceWithLengthProtected has the correct length invariant + let thin_ptr: *mut ArcInner, [T; 0]>> = fat_ptr.cast(); ThinArc { - // Safety: The pointer comes from a valid Arc, and HeaderSliceWithLengthProtected has the correct length invariant - ptr: unsafe { - ptr::NonNull::new_unchecked( - thin_ptr as *mut ArcInner>, - ) - }, + ptr: unsafe { ptr::NonNull::new_unchecked(thin_ptr) }, phantom: PhantomData, } } @@ -313,13 +324,8 @@ impl Arc> { #[inline] pub fn protected_from_thin(a: ThinArc) -> Self { let a = ManuallyDrop::new(a); - let ptr = thin_to_thick(a.ptr.as_ptr()); - unsafe { - Arc { - p: ptr::NonNull::new_unchecked(ptr), - phantom: PhantomData, - } - } + let ptr = thin_to_thick(&a); + unsafe { Arc::from_raw_inner(ptr) } } /// Obtains a HeaderSliceWithLengthProtected from an unchecked HeaderSliceWithLengthUnchecked, wrapped in an Arc @@ -327,10 +333,12 @@ impl Arc> { /// # Safety /// Assumes that the header length matches the slice length. #[inline] - unsafe fn from_unprotected_unchecked(a: Arc>) -> Self { + unsafe fn from_unprotected_unchecked(a: Arc>) -> Self { // Safety: HeaderSliceWithLengthProtected and HeaderSliceWithLengthUnchecked have the same layout // and the safety invariant on HeaderSliceWithLengthProtected.inner is bubbled up - unsafe { mem::transmute(a) } + // The whole `Arc` should also be layout compatible (as a transparent wrapper around `NonNull` pointers with the same + // metadata type) but we still conservatively avoid a direct transmute here and use a pointer-cast instead. + unsafe { Arc::from_raw_inner(Arc::into_raw_inner(a) as _) } } } diff --git a/src/unique_arc.rs b/src/unique_arc.rs index ccdf67d..42dc2fb 100644 --- a/src/unique_arc.rs +++ b/src/unique_arc.rs @@ -332,7 +332,7 @@ mod tests { #[test] fn from_header_and_uninit_slice() { - let mut uarc: UniqueArc]>> = + let mut uarc: UniqueArc>> = UniqueArc::from_header_and_uninit_slice(HeaderWithLength::new(1, 3), 3); uarc.slice.fill(MaybeUninit::new(2)); let arc = unsafe { uarc.assume_init_slice_with_header() }.shareable();