Skip to content

Commit

Permalink
[TryFromBytes] Permit UnsafeCells
Browse files Browse the repository at this point in the history
Previously, `TryFromBytes` could not be implemented on types which
contain `UnsafeCell`s. This commit lifts that restriction, and replaces
it with a `NoCell` bound on the `try_from_ref` method.
  • Loading branch information
jswrenn authored and joshlf committed Feb 17, 2024
1 parent 1221b31 commit 797bac0
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 85 deletions.
44 changes: 30 additions & 14 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1210,12 +1210,6 @@ pub unsafe trait NoCell {
/// including changes that only affect soundness, which may cause code which
/// uses those items to silently become unsound.
///
/// ## `UnsafeCell`
///
/// `TryFromBytes` may not be implemented for any type which contains an
/// [`UnsafeCell`]. This is a temporary limitation and will be lifted once
/// [`try_from_ref`] has a [`NoCell`] bound.
///
/// [undefined behavior]: https://raphlinus.github.io/programming/rust/2018/08/17/undefined-behavior.html
/// [github-repo]: https://github.com/google/zerocopy
/// [`try_from_ref`]: TryFromBytes::try_from_ref
Expand Down Expand Up @@ -1264,7 +1258,7 @@ pub unsafe trait TryFromBytes {
#[doc(hidden)] // TODO(#5): Finalize name before remove this attribute.
fn try_from_ref(bytes: &[u8]) -> Option<&Self>
where
Self: KnownLayout,
Self: KnownLayout + NoCell,
{
let candidate = Ptr::from_ref(bytes).try_cast_into_no_leftover::<Self>()?;

Expand Down Expand Up @@ -1293,7 +1287,7 @@ pub unsafe trait TryFromBytes {
#[doc(hidden)] // TODO(#5): Finalize name before remove this attribute.
fn try_read_from(bytes: &[u8]) -> Option<Self>
where
Self: Sized,
Self: Sized + NoCell,
{
let candidate = MaybeUninit::<Self>::read_from(bytes)?;
let c_ptr = Ptr::from_maybe_uninit_ref(&candidate);
Expand Down Expand Up @@ -3358,6 +3352,19 @@ safety_comment! {
unsafe_impl!(Option<NonZeroIsize>: TryFromBytes, FromZeros, FromBytes, IntoBytes);
}

safety_comment! {
/// SAFETY:
/// While it's not fully documented, the consensus is that `Box<T>` does not
/// contain any `UnsafeCell`s for `T: Sized` [1].
///
/// [1] https://github.com/rust-lang/unsafe-code-guidelines/issues/492
#[cfg(feature = "alloc")]
unsafe_impl!(
#[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
T: Sized => NoCell for Box<T>
);
}

safety_comment! {
/// SAFETY:
/// The following types can be transmuted from `[0u8; size_of::<T>()]`. [1]
Expand Down Expand Up @@ -3500,15 +3507,16 @@ safety_comment! {
//
/// SAFETY:
/// - `NoCell`: `MaybeUninit<T>` has `UnsafeCell`s exactly when `T` does, so
/// `T: NoCell` guarantees that `MaybeUninit<T>` has no `UnsafeCell`s.
/// `T: NoCell` guarantees that `MaybeUninit<T>` has no `UnsafeCell`s [1].
/// - `TryFromBytes` (with no validator), `FromZeros`, `FromBytes`:
/// `MaybeUninit<T>` has no restrictions on its contents. `TryFromBytes`
/// may not be implemented for types which contain `UnsafeCell`, which we
/// satisfy thanks to the `T: NoCell` bound.
/// - `Unaligned`: "MaybeUninit<T> is guaranteed to have the same size,
/// alignment, and ABI as T" [1]
/// alignment, and ABI as T" [2].
///
/// [1] https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#layout-1
/// [1] TODO(https://github.com/rust-lang/rust/pull/121215)
/// [2] https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#layout-1
unsafe_impl!(T: NoCell => NoCell for MaybeUninit<T>);
unsafe_impl!(T: NoCell => TryFromBytes for MaybeUninit<T>);
unsafe_impl!(T => FromZeros for MaybeUninit<T>);
Expand Down Expand Up @@ -3704,6 +3712,14 @@ safety_comment! {
});
unsafe_impl!(T => FromZeros for *mut T);
}

safety_comment! {
/// SAFETY:
///
/// TODO
unsafe_impl!(T: ?Sized => NoCell for NonNull<T>);
}

safety_comment! {
/// SAFETY:
/// Reference types do not contain any `UnsafeCell`s.
Expand Down Expand Up @@ -8406,14 +8422,14 @@ mod tests {
) -> (NotZerocopy, NotZerocopy);

#[cfg(feature = "alloc")]
assert_impls!(Option<Box<UnsafeCell<NotZerocopy>>>: KnownLayout, TryFromBytes, FromZeros, !NoCell, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<Box<UnsafeCell<NotZerocopy>>>: KnownLayout, NoCell, TryFromBytes, FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<Box<[UnsafeCell<NotZerocopy>]>>: KnownLayout, !NoCell, !TryFromBytes, !FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<&'static UnsafeCell<NotZerocopy>>: KnownLayout, NoCell, TryFromBytes, FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<&'static [UnsafeCell<NotZerocopy>]>: KnownLayout, NoCell, !TryFromBytes, !FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<&'static mut UnsafeCell<NotZerocopy>>: KnownLayout, NoCell, TryFromBytes, FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<&'static mut [UnsafeCell<NotZerocopy>]>: KnownLayout, NoCell, !TryFromBytes, !FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<NonNull<UnsafeCell<NotZerocopy>>>: KnownLayout, TryFromBytes, FromZeros, !NoCell, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<NonNull<[UnsafeCell<NotZerocopy>]>>: KnownLayout, !NoCell, !TryFromBytes, !FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<NonNull<UnsafeCell<NotZerocopy>>>: KnownLayout, TryFromBytes, FromZeros, NoCell, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<NonNull<[UnsafeCell<NotZerocopy>]>>: KnownLayout, NoCell, !TryFromBytes, !FromZeros, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<fn()>: KnownLayout, NoCell, FromZeros, !TryFromBytes, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<FnManyArgs>: KnownLayout, NoCell, FromZeros, !TryFromBytes, !FromBytes, !IntoBytes, !Unaligned);
assert_impls!(Option<extern "C" fn()>: KnownLayout, NoCell, FromZeros, !TryFromBytes, !FromBytes, !IntoBytes, !Unaligned);
Expand Down
1 change: 1 addition & 0 deletions src/pointer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ where
/// Checks if the referent is zeroed.
pub(crate) fn is_zeroed<T, I>(ptr: Ptr<'_, T, I>) -> bool
where
T: crate::NoCell,
I: invariant::Invariants<Aliasing = invariant::Shared, Validity = invariant::Initialized>,
{
ptr.as_bytes().as_ref().iter().all(|&byte| byte == 0)
Expand Down
Loading

0 comments on commit 797bac0

Please sign in to comment.