Skip to content

Commit

Permalink
Merge pull request #111 from steffahn/headerslice-always-slice
Browse files Browse the repository at this point in the history 
Make `HeaderSliceWithLen…` always contain slices
  • Loading branch information
@Manishearth
Manishearth authored Jan 2, 2025
2 parents 6ccaace + 5a20cd2 commit 24d2d56
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 71 deletions.
21 changes: 10 additions & 11 deletions src/header.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,19 +251,18 @@ impl<T> From<Vec<T>> for Arc<[T]> {
///
/// Safety-usable invariants:
///
/// - This is guaranteed to have the same representation as `HeaderSlice<HeaderWithLength<H>, 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<HeaderWithLength<H>, [T]>`
/// - The header length (`.length()`) is checked to be the slice length
#[derive(Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
#[repr(transparent)]
pub struct HeaderSliceWithLengthProtected<H, T: ?Sized> {
// 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<H, T> {
// Invariant: the header's length field must be the slice length
inner: HeaderSliceWithLengthUnchecked<H, T>,
}

pub(crate) type HeaderSliceWithLengthUnchecked<H, T> = HeaderSlice<HeaderWithLength<H>, T>;
pub(crate) type HeaderSliceWithLengthUnchecked<H, T> = HeaderSlice<HeaderWithLength<H>, [T]>;

impl<H, T: ?Sized> HeaderSliceWithLengthProtected<H, T> {
impl<H, T> HeaderSliceWithLengthProtected<H, T> {
pub fn header(&self) -> &H {
&self.inner.header.header
}
Expand All @@ -275,10 +274,10 @@ impl<H, T: ?Sized> HeaderSliceWithLengthProtected<H, T> {
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
}
Expand All @@ -288,13 +287,13 @@ impl<H, T: ?Sized> HeaderSliceWithLengthProtected<H, T> {
}
}

impl<H: PartialOrd, T: ?Sized + PartialOrd> PartialOrd for HeaderSliceWithLengthUnchecked<H, T> {
impl<H: PartialOrd, T: ?Sized + PartialOrd> PartialOrd for HeaderSlice<HeaderWithLength<H>, T> {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
(&self.header.header, &self.slice).partial_cmp(&(&other.header.header, &other.slice))
}
}

impl<H: Ord, T: ?Sized + Ord> Ord for HeaderSliceWithLengthUnchecked<H, T> {
impl<H: Ord, T: ?Sized + Ord> Ord for HeaderSlice<HeaderWithLength<H>, T> {
fn cmp(&self, other: &Self) -> Ordering {
(&self.header.header, &self.slice).cmp(&(&other.header.header, &other.slice))
}
Expand Down
126 changes: 67 additions & 59 deletions src/thin_arc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,7 +28,19 @@ use crate::header::HeaderSliceWithLengthUnchecked;
/// via `HeaderSliceWithLengthProtected`.
#[repr(transparent)]
pub struct ThinArc<H, T> {
ptr: ptr::NonNull<ArcInner<HeaderSliceWithLengthProtected<H, [T; 0]>>>,
// We can pointer-cast between this target type
// of `ArcInner<HeaderSlice<HeaderWithLength<H>, [T; 0]>`
// and the types
// `ArcInner<HeaderSliceWithLengthProtected<H, T>>` and
// `ArcInner<HeaderSliceWithLengthUnchecked<H, T>>` (= `ArcInner<HeaderSlice<HeaderWithLength<H>, [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<ArcInner<HeaderSlice<HeaderWithLength<H>, [T; 0]>>>,
phantom: PhantomData<(H, T)>,
}

Expand All @@ -40,13 +51,12 @@ unsafe impl<H: Sync + Send, T: Sync + Send> Sync for ThinArc<H, T> {}
//
// See the comment around the analogous operation in from_header_and_iter.
#[inline]
fn thin_to_thick<H, T>(
thin: *mut ArcInner<HeaderSliceWithLengthProtected<H, [T; 0]>>,
) -> *mut ArcInner<HeaderSliceWithLengthProtected<H, [T]>> {
let len = unsafe { (*thin).data.length() };
fn thin_to_thick<H, T>(arc: &ThinArc<H, T>) -> *mut ArcInner<HeaderSliceWithLengthProtected<H, T>> {
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<HeaderSliceWithLengthProtected<H, [T]>>
fake_slice as *mut ArcInner<HeaderSliceWithLengthProtected<H, T>>
}

impl<H, T> ThinArc<H, T> {
Expand All @@ -55,15 +65,12 @@ impl<H, T> ThinArc<H, T> {
#[inline]
pub fn with_arc<F, U>(&self, f: F) -> U
where
F: FnOnce(&Arc<HeaderSliceWithLengthUnchecked<H, [T]>>) -> U,
F: FnOnce(&Arc<HeaderSliceWithLengthUnchecked<H, T>>) -> 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
Expand All @@ -75,15 +82,10 @@ impl<H, T> ThinArc<H, T> {
#[inline]
fn with_protected_arc<F, U>(&self, f: F) -> U
where
F: FnOnce(&Arc<HeaderSliceWithLengthProtected<H, [T]>>) -> U,
F: FnOnce(&Arc<HeaderSliceWithLengthProtected<H, T>>) -> 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
Expand All @@ -95,29 +97,34 @@ impl<H, T> ThinArc<H, T> {
#[inline]
pub fn with_arc_mut<F, U>(&mut self, f: F) -> U
where
F: FnOnce(&mut Arc<HeaderSliceWithLengthProtected<H, [T]>>) -> U,
F: FnOnce(&mut Arc<HeaderSliceWithLengthProtected<H, T>>) -> 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<Arc<HeaderSliceWithLengthProtected<H, [T]>>>,
transient: ManuallyDrop<Arc<HeaderSliceWithLengthProtected<H, T>>>,
this: &'a mut ThinArc<H, T>,
}

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,
Expand All @@ -128,6 +135,13 @@ impl<H, T> ThinArc<H, T> {
// 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
}

Expand Down Expand Up @@ -155,7 +169,7 @@ impl<H, T> ThinArc<H, T> {
/// within it -- for memory reporting.
#[inline]
pub fn ptr(&self) -> *const c_void {
self.ptr.as_ptr() as *const ArcInner<T> 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
Expand Down Expand Up @@ -188,7 +202,7 @@ impl<H, T> ThinArc<H, T> {
#[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.
Expand All @@ -214,11 +228,11 @@ impl<H, T> ThinArc<H, T> {
}

impl<H, T> Deref for ThinArc<H, T> {
type Target = HeaderSliceWithLengthUnchecked<H, [T]>;
type Target = HeaderSliceWithLengthUnchecked<H, T>;

#[inline]
fn deref(&self) -> &Self::Target {
unsafe { &(*thin_to_thick(self.ptr.as_ptr())).data.inner() }
unsafe { (*thin_to_thick(self)).data.inner() }
}
}

Expand All @@ -232,14 +246,14 @@ impl<H, T> Clone for ThinArc<H, T> {
impl<H, T> Drop for ThinArc<H, T> {
#[inline]
fn drop(&mut self) {
let _ = Arc::from_thin(ThinArc {
let _ = Arc::protected_from_thin(ThinArc {
ptr: self.ptr,
phantom: PhantomData,
});
}
}

impl<H, T> Arc<HeaderSliceWithLengthUnchecked<H, [T]>> {
impl<H, T> Arc<HeaderSliceWithLengthUnchecked<H, T>> {
/// Converts an `Arc` into a `ThinArc`. This consumes the `Arc`, so the refcount
/// is not modified.
///
Expand All @@ -248,7 +262,7 @@ impl<H, T> Arc<HeaderSliceWithLengthUnchecked<H, [T]>> {
#[inline]
unsafe fn into_thin_unchecked(a: Self) -> ThinArc<H, T> {
// Safety: invariant bubbled up
let this_protected: Arc<HeaderSliceWithLengthProtected<H, [T]>> =
let this_protected: Arc<HeaderSliceWithLengthProtected<H, T>> =
unsafe { Arc::from_unprotected_unchecked(a) };

Arc::protected_into_thin(this_protected)
Expand All @@ -271,19 +285,21 @@ impl<H, T> Arc<HeaderSliceWithLengthUnchecked<H, [T]>> {
/// is not modified.
#[inline]
pub fn from_thin(a: ThinArc<H, T>) -> Self {
Self::from_protected(Arc::<HeaderSliceWithLengthProtected<H, [T]>>::protected_from_thin(a))
Self::from_protected(Arc::<HeaderSliceWithLengthProtected<H, T>>::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<HeaderSliceWithLengthProtected<H, [T]>>) -> Self {
fn from_protected(a: Arc<HeaderSliceWithLengthProtected<H, T>>) -> 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<H, T> Arc<HeaderSliceWithLengthProtected<H, [T]>> {
impl<H, T> Arc<HeaderSliceWithLengthProtected<H, T>> {
/// Converts an `Arc` into a `ThinArc`. This consumes the `Arc`, so the refcount
/// is not modified.
#[inline]
Expand All @@ -294,16 +310,11 @@ impl<H, T> Arc<HeaderSliceWithLengthProtected<H, [T]>> {
"Length needs to be correct for ThinArc to work"
);

let a = ManuallyDrop::new(a);
let fat_ptr: *mut ArcInner<HeaderSliceWithLengthProtected<H, [T]>> = a.ptr();
let thin_ptr = fat_ptr as *mut [usize] as *mut usize;
let fat_ptr: *mut ArcInner<HeaderSliceWithLengthProtected<H, T>> = 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<HeaderSlice<HeaderWithLength<H>, [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<HeaderSliceWithLengthProtected<H, [T; 0]>>,
)
},
ptr: unsafe { ptr::NonNull::new_unchecked(thin_ptr) },
phantom: PhantomData,
}
}
Expand All @@ -313,24 +324,21 @@ impl<H, T> Arc<HeaderSliceWithLengthProtected<H, [T]>> {
#[inline]
pub fn protected_from_thin(a: ThinArc<H, T>) -> 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
///
/// # Safety
/// Assumes that the header length matches the slice length.
#[inline]
unsafe fn from_unprotected_unchecked(a: Arc<HeaderSliceWithLengthUnchecked<H, [T]>>) -> Self {
unsafe fn from_unprotected_unchecked(a: Arc<HeaderSliceWithLengthUnchecked<H, T>>) -> 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 _) }
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/unique_arc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ mod tests {

#[test]
fn from_header_and_uninit_slice() {
let mut uarc: UniqueArc<HeaderSliceWithLengthUnchecked<u8, [MaybeUninit<u16>]>> =
let mut uarc: UniqueArc<HeaderSliceWithLengthUnchecked<u8, MaybeUninit<u16>>> =
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();
Expand Down

0 comments on commit 24d2d56

Please sign in to comment.