Skip to content

Commit

Permalink
[pointer] Simplify AliasingSafe, rename to Read (#1908)
Browse files Browse the repository at this point in the history 
`AliasingSafe` is really about whether a pointer permits unsynchronized
reads - either because the referent contains no `UnsafeCell`s or because
the aliasing mode is `Exclusive`. Previously, `AliasingSafe` was not
named consistent with this meaning, and was a function of a *pair* of
types rather than of a single type. This commit fixes both oversights.

While we're here, we also add `Read` bounds in some places, allowing us
to simplify many safety comments.

gherrit-pr-id: I88b9101a614459af3b062dc3d38142ba76427f34
  • Loading branch information
@joshlf
joshlf committed Feb 24, 2025
1 parent 6ebdffb commit e09713e
Show file tree
Hide file tree
Showing 13 changed files with 138 additions and 157 deletions.
1 change: 1 addition & 0 deletions src/impls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,7 @@ mod simd {
#[cfg(test)]
mod tests {
use super::*;
use crate::pointer::invariant;

#[test]
fn test_impls() {
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ use core::{
#[cfg(feature = "std")]
use std::io;

use crate::pointer::{invariant, BecauseExclusive};
use crate::pointer::invariant::{self, BecauseExclusive};

#[cfg(any(feature = "alloc", test))]
extern crate alloc;
Expand All @@ -387,7 +387,7 @@ use core::alloc::Layout;

// Used by `TryFromBytes::is_bit_valid`.
#[doc(hidden)]
pub use crate::pointer::{BecauseImmutable, Maybe, MaybeAligned, Ptr};
pub use crate::pointer::{invariant::BecauseImmutable, Maybe, MaybeAligned, Ptr};
// Used by `KnownLayout`.
#[doc(hidden)]
pub use crate::layout::*;
Expand Down
112 changes: 37 additions & 75 deletions src/pointer/invariant.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,78 +123,41 @@ impl Validity for Initialized {}
pub enum Valid {}
impl Validity for Valid {}

pub mod aliasing_safety {
use super::*;
use crate::Immutable;

/// Pointer conversions which do not violate aliasing.
///
/// `U: AliasingSafe<T, A, R>` implies that a pointer conversion from `T` to
/// `U` does not violate the aliasing invariant, `A`. This can be because
/// `A` is [`Exclusive`] or because neither `T` nor `U` permit interior
/// mutability.
///
/// # Safety
///
/// `U: AliasingSafe<T, A, R>` if either of the following conditions holds:
/// - `A` is [`Exclusive`]
/// - `T` and `U` both implement [`Immutable`]
#[doc(hidden)]
pub unsafe trait AliasingSafe<T: ?Sized, A: Aliasing, R: AliasingSafeReason> {}

#[doc(hidden)]
pub trait AliasingSafeReason: sealed::Sealed {}
impl<R: AliasingSafeReason> AliasingSafeReason for (R,) {}

/// The conversion is safe because only one live `Ptr` or reference may exist to
/// the referent bytes at a time.
#[derive(Copy, Clone, Debug)]
#[doc(hidden)]
pub enum BecauseExclusive {}
impl AliasingSafeReason for BecauseExclusive {}

/// The conversion is safe because no live `Ptr`s or references permit mutation.
#[derive(Copy, Clone, Debug)]
#[doc(hidden)]
pub enum BecauseImmutable {}
impl AliasingSafeReason for BecauseImmutable {}

/// SAFETY: `T: AliasingSafe<Exclusive, BecauseExclusive>` because for all
/// `Ptr<'a, T, I>` such that `I::Aliasing = Exclusive`, there cannot exist
/// other live references to the memory referenced by `Ptr`.
unsafe impl<T: ?Sized, U: ?Sized> AliasingSafe<T, Exclusive, BecauseExclusive> for U {}

/// SAFETY: `U: AliasingSafe<T, A, BecauseNoCell>` because for all `Ptr<'a, T,
/// I>` and `Ptr<'a, U, I>` such that `I::Aliasing = A`, all live references and
/// live `Ptr`s agree, by invariant on `Immutable`, that the referenced bytes
/// contain no `UnsafeCell`s, and thus do not permit mutation except via
/// exclusive aliasing.
unsafe impl<A, T: ?Sized, U: ?Sized> AliasingSafe<T, A, BecauseImmutable> for U
where
A: Aliasing,
T: Immutable,
U: Immutable,
{
}

/// This ensures that `U: AliasingSafe<T, A>` implies `T: AliasingSafe<U, A>` in
/// a manner legible to rustc, which in turn means we can write simpler bounds in
/// some places.
///
/// SAFETY: Per `U: AliasingSafe<T, A, R>`, either:
/// - `A` is `Exclusive`
/// - `T` and `U` both implement `Immutable`
///
/// Neither property depends on which of `T` and `U` are in the `Self` position
/// vs the first type parameter position.
unsafe impl<A, T: ?Sized, U: ?Sized, R> AliasingSafe<U, A, (R,)> for T
where
A: Aliasing,
R: AliasingSafeReason,
U: AliasingSafe<T, A, R>,
{
}
}
/// [`Ptr`](crate::Ptr) referents that permit unsynchronized read operations.
///
/// `T: Read<A, R>` implies that a pointer to `T` with aliasing `A` permits
/// unsynchronized read oeprations. This can be because `A` is [`Exclusive`] or
/// because `T` does not permit interior mutation.
///
/// # Safety
///
/// `T: Read<A, R>` if either of the following conditions holds:
/// - `A` is [`Exclusive`]
/// - `T` implements [`Immutable`](crate::Immutable)
///
/// As a consequence, if `T: Read<A, R>`, then any `Ptr<T, (A, ...)>` is
/// permitted to perform unsynchronized reads from its referent.
pub trait Read<A: Aliasing, R: ReadReason> {}

impl<A: Reference, T: ?Sized + crate::Immutable> Read<A, BecauseImmutable> for T {}
impl<T: ?Sized> Read<Exclusive, BecauseExclusive> for T {}

/// Used to disambiguate [`Read`] impls.
pub trait ReadReason: Sealed {}

/// Unsynchronized reads are permitted because only one live [`Ptr`](crate::Ptr)
/// or reference may exist to the referent bytes at a time.
#[derive(Copy, Clone, Debug)]
#[doc(hidden)]
pub enum BecauseExclusive {}
impl ReadReason for BecauseExclusive {}

/// Unsynchronized reads are permitted because no live [`Ptr`](crate::Ptr)s or
/// references permit interior mutation.
#[derive(Copy, Clone, Debug)]
#[doc(hidden)]
pub enum BecauseImmutable {}
impl ReadReason for BecauseImmutable {}

use sealed::Sealed;
mod sealed {
Expand All @@ -215,7 +178,6 @@ mod sealed {

impl<A: Sealed, AA: Sealed, V: Sealed> Sealed for (A, AA, V) {}

impl Sealed for super::aliasing_safety::BecauseExclusive {}
impl Sealed for super::aliasing_safety::BecauseImmutable {}
impl<S: Sealed> Sealed for (S,) {}
impl Sealed for BecauseImmutable {}
impl Sealed for BecauseExclusive {}
}
10 changes: 4 additions & 6 deletions src/pointer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ pub mod invariant;
mod ptr;

#[doc(hidden)]
pub use invariant::aliasing_safety::{
AliasingSafe, AliasingSafeReason, BecauseExclusive, BecauseImmutable,
};
pub use invariant::{BecauseExclusive, BecauseImmutable, Read, ReadReason};
#[doc(hidden)]
pub use ptr::Ptr;

Expand Down Expand Up @@ -50,11 +48,11 @@ where
pub fn read_unaligned<R>(self) -> T
where
T: Copy,
R: AliasingSafeReason,
T: AliasingSafe<T, Aliasing, R>,
R: invariant::ReadReason,
T: invariant::Read<Aliasing, R>,
{
// SAFETY: By invariant on `MaybeAligned`, `raw` contains
// validly-initialized data for `T`. By `T: AliasingSafe`, we are
// validly-initialized data for `T`. By `T: Read<Aliasing>`, we are
// permitted to perform a read of `self`'s referent.
unsafe { self.as_inner().read_unaligned() }
}
Expand Down
88 changes: 59 additions & 29 deletions src/pointer/ptr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ mod _conversions {
// byte ranges. Since `p` and the returned pointer address the
// same byte range, they refer to `UnsafeCell`s at the same byte
// ranges.
let c = unsafe { self.cast_unsized(|p| T::cast_into_inner(p)) };
let c = unsafe { self.cast_unsized_unchecked(|p| T::cast_into_inner(p)) };
// SAFETY: By invariant on `TransparentWrapper`, since `self`
// satisfies the alignment invariant `I::Alignment`, `c` (of type
// `T::Inner`) satisfies the given "applied" alignment invariant.
Expand Down Expand Up @@ -413,7 +413,7 @@ mod _conversions {
// `UnsafeCell`s at the same locations as `p`.
let ptr = unsafe {
#[allow(clippy::as_conversions)]
self.cast_unsized(|p: *mut T| p as *mut crate::Unalign<T>)
self.cast_unsized_unchecked(|p: *mut T| p as *mut crate::Unalign<T>)
};
// SAFETY: `Unalign<T>` promises to have the same bit validity as
// `T`.
Expand Down Expand Up @@ -659,13 +659,14 @@ mod _transitions {
/// On error, unsafe code may rely on this method's returned
/// `ValidityError` containing `self`.
#[inline]
pub(crate) fn try_into_valid(
pub(crate) fn try_into_valid<R>(
mut self,
) -> Result<Ptr<'a, T, (I::Aliasing, I::Alignment, Valid)>, ValidityError<Self, T>>
where
T: TryFromBytes,
T: TryFromBytes + Read<I::Aliasing, R>,
I::Aliasing: Reference,
I: Invariants<Validity = Initialized>,
R: crate::pointer::ReadReason,
{
// This call may panic. If that happens, it doesn't cause any soundness
// issues, as we have not generated any invalid state which we need to
Expand Down Expand Up @@ -693,14 +694,19 @@ mod _transitions {
/// Casts of the referent type.
mod _casts {
use super::*;
use crate::{pointer::invariant::aliasing_safety::*, CastError, SizeError};
use crate::{CastError, SizeError};

impl<'a, T, I> Ptr<'a, T, I>
where
T: 'a + ?Sized,
I: Invariants,
{
/// Casts to a different (unsized) target type.
/// Casts to a different (unsized) target type without checking interior
/// mutability.
///
/// Callers should prefer [`cast_unsized`] where possible.
///
/// [`cast_unsized`]: Ptr::cast_unsized
///
/// # Safety
///
Expand All @@ -713,7 +719,7 @@ mod _casts {
/// exist in `*p`
#[doc(hidden)]
#[inline]
pub unsafe fn cast_unsized<U: 'a + ?Sized, F: FnOnce(*mut T) -> *mut U>(
pub unsafe fn cast_unsized_unchecked<U: 'a + ?Sized, F: FnOnce(*mut T) -> *mut U>(
self,
cast: F,
) -> Ptr<'a, U, (I::Aliasing, Any, Any)> {
Expand Down Expand Up @@ -775,6 +781,34 @@ mod _casts {
// 8. `ptr`, trivially, conforms to the validity invariant of `Any`.
unsafe { Ptr::new(ptr) }
}

/// Casts to a different (unsized) target type.
///
/// # Safety
///
/// The caller promises that `u = cast(p)` is a pointer cast with the
/// following properties:
/// - `u` addresses a subset of the bytes addressed by `p`
/// - `u` has the same provenance as `p`
#[doc(hidden)]
#[inline]
pub unsafe fn cast_unsized<U, F, R, S>(self, cast: F) -> Ptr<'a, U, (I::Aliasing, Any, Any)>
where
T: Read<I::Aliasing, R>,
U: 'a + ?Sized + Read<I::Aliasing, S>,
R: ReadReason,
S: ReadReason,
F: FnOnce(*mut T) -> *mut U,
{
// SAFETY: Because `T` and `U` both implement `Read<I::Aliasing, _>`,
// either:
// - `I::Aliasing` is `Exclusive`
// - `T` and `U` are both `Immutable`, in which case they trivially
// contain `UnsafeCell`s at identical locations
//
// The caller promises all other safety preconditions.
unsafe { self.cast_unsized_unchecked(cast) }
}
}

impl<'a, T, I> Ptr<'a, T, I>
Expand All @@ -786,8 +820,9 @@ mod _casts {
#[allow(clippy::wrong_self_convention)]
pub(crate) fn as_bytes<R>(self) -> Ptr<'a, [u8], (I::Aliasing, Aligned, Valid)>
where
[u8]: AliasingSafe<T, I::Aliasing, R>,
R: AliasingSafeReason,
R: ReadReason,
T: Read<I::Aliasing, R>,
I::Aliasing: Reference,
{
let bytes = match T::size_of_val_raw(self.as_inner().as_non_null()) {
Some(bytes) => bytes,
Expand All @@ -804,10 +839,6 @@ mod _casts {
// pointer's address, and `bytes` is the length of `p`, so the
// returned pointer addresses the same bytes as `p`
// - `slice_from_raw_parts_mut` and `.cast` both preserve provenance
// - Because `[u8]: AliasingSafe<T, I::Aliasing, _>`, either:
// - `I::Aliasing` is `Exclusive`
// - `T` and `[u8]` are both `Immutable`, in which case they
// trivially contain `UnsafeCell`s at identical locations
let ptr: Ptr<'a, [u8], _> = unsafe {
self.cast_unsized(|p: *mut T| {
#[allow(clippy::as_conversions)]
Expand Down Expand Up @@ -887,9 +918,9 @@ mod _casts {
CastError<Self, U>,
>
where
R: AliasingSafeReason,
R: ReadReason,
I::Aliasing: Reference,
U: 'a + ?Sized + KnownLayout + AliasingSafe<[u8], I::Aliasing, R>,
U: 'a + ?Sized + KnownLayout + Read<I::Aliasing, R>,
{
let (inner, remainder) =
self.as_inner().try_cast_into(cast_type, meta).map_err(|err| {
Expand All @@ -902,12 +933,13 @@ mod _casts {
})?;

// SAFETY:
// 0. Since `U: AliasingSafe<[u8], I::Aliasing, _>`, either:
// 0. Since `U: Read<I::Aliasing, _>`, either:
// - `I::Aliasing` is `Exclusive`, in which case both `src` and
// `ptr` conform to `Exclusive`
// - `I::Aliasing` is `Shared` or `Any` and both `U` and `[u8]`
// are `Immutable`. In this case, neither pointer permits
// mutation, and so `Shared` aliasing is satisfied.
// - `I::Aliasing` is `Shared` or `Any` and `U` is `Immutable`
// (we already know that `[u8]: Immutable`). In this case,
// neither `U` nor `[u8]` permit mutation, and so `Shared`
// aliasing is satisfied.
// 1. `ptr` conforms to the alignment invariant of `Aligned` because
// it is derived from `try_cast_into`, which promises that the
// object described by `target` is validly aligned for `U`.
Expand Down Expand Up @@ -948,8 +980,8 @@ mod _casts {
) -> Result<Ptr<'a, U, (I::Aliasing, Aligned, Initialized)>, CastError<Self, U>>
where
I::Aliasing: Reference,
U: 'a + ?Sized + KnownLayout + AliasingSafe<[u8], I::Aliasing, R>,
R: AliasingSafeReason,
U: 'a + ?Sized + KnownLayout + Read<I::Aliasing, R>,
R: ReadReason,
{
// TODO(#67): Remove this allow. See NonNulSlicelExt for more
// details.
Expand Down Expand Up @@ -996,11 +1028,8 @@ mod _casts {
#[must_use]
#[inline(always)]
pub fn get_mut(self) -> Ptr<'a, T, I> {
// SAFETY:
// - The closure uses an `as` cast, which preserves address range
// and provenance.
// - We require `I: Invariants<Aliasing = Exclusive>`, so we are not
// required to uphold `UnsafeCell` equality.
// SAFETY: The closure uses an `as` cast, which preserves address
// range and provenance.
#[allow(clippy::as_conversions)]
let ptr = unsafe { self.cast_unsized(|p| p as *mut T) };

Expand Down Expand Up @@ -1046,7 +1075,8 @@ mod _project {
///
/// # Safety
///
/// `project` has the same safety preconditions as `cast_unsized`.
/// `project` has the same safety preconditions as
/// `cast_unsized_unchecked`.
#[doc(hidden)]
#[inline]
pub unsafe fn project<U: 'a + ?Sized>(
Expand All @@ -1058,8 +1088,8 @@ mod _project {
// `Initialized` pointer, we could remove this method entirely.

// SAFETY: This method has the same safety preconditions as
// `cast_unsized`.
let ptr = unsafe { self.cast_unsized(projector) };
// `cast_unsized_unchecked`.
let ptr = unsafe { self.cast_unsized_unchecked(projector) };

// SAFETY: If all of the bytes of `self` are initialized (as
// promised by `I: Invariants<Validity = Initialized>`), then any
Expand Down
Loading

0 comments on commit e09713e

Please sign in to comment.