Skip to content

Commit

Permalink
[pointer] Clarify semantics of aliasing invariants
Browse files Browse the repository at this point in the history
Previously, we supported the `AtLeast` bound, which was used to describe
a subset relationship in which `I: AtLeast<J>` implied that `I` as at
least as restrictive as `J`. However, as described in #1866, this
incorrectly models invariants as monotonic. In reality, invariants both
provide guarantees but also *require* guarantees.

This commit takes a step in the direction of resolving #1866 by removing
`AtLeast`. Uses of `AtLeast<Shared>` are replaced by a new `Reference`
trait, which is implemented for `Shared` and `Exclusive`. This serves
two purposes: First, it makes it explicit what this bound means.
Previously, `AtLeast<Shared>` had an ambiguous meaning, while
`Reference` means precisely that an invariant is either `Shared` or
`Exclusive` and nothing else. Second, it paves the way for #1183, in
which we may add new aliasing invariants which convey ownership. In that
case, it will be important for existing methods to add `Reference`
bounds when those methods would not be sound in the face of ownership
semantics.

We also inline the items in the `invariant` module, which were
previously generated by macro. The addition of the `Reference` trait did
not play nicely with that macro, and we will likely need to go further
from the macro in order to fix #1839 – this fix will likely require
making aliasing invariants meaningfully different than other invariants,
for example by adding an associated type.

Makes progress on #1866
  • Loading branch information
joshlf committed Oct 12, 2024
1 parent 5452c3d commit f421eaa
Show file tree
Hide file tree
Showing 9 changed files with 199 additions and 315 deletions.
16 changes: 4 additions & 12 deletions src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,7 @@ unsafe impl<T: TryFromBytes + ?Sized> TryFromBytes for UnsafeCell<T> {
}

#[inline]
fn is_bit_valid<A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>>(
candidate: Maybe<'_, Self, A>,
) -> bool {
fn is_bit_valid<A: invariant::Reference>(candidate: Maybe<'_, Self, A>) -> bool {
// The only way to implement this function is using an exclusive-aliased
// pointer. `UnsafeCell`s cannot be read via shared-aliased pointers
// (other than by using `unsafe` code, which we can't use since we can't
Expand Down Expand Up @@ -1134,21 +1132,15 @@ mod tests {

pub(super) trait TestIsBitValidShared<T: ?Sized> {
#[allow(clippy::needless_lifetimes)]
fn test_is_bit_valid_shared<
'ptr,
A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>,
>(
fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>(
&self,
candidate: Maybe<'ptr, T, A>,
) -> Option<bool>;
}

impl<T: TryFromBytes + Immutable + ?Sized> TestIsBitValidShared<T> for AutorefWrapper<T> {
#[allow(clippy::needless_lifetimes)]
fn test_is_bit_valid_shared<
'ptr,
A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>,
>(
fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>(
&self,
candidate: Maybe<'ptr, T, A>,
) -> Option<bool> {
Expand Down Expand Up @@ -1238,7 +1230,7 @@ mod tests {
#[allow(unused, non_local_definitions)]
impl AutorefWrapper<$ty> {
#[allow(clippy::needless_lifetimes)]
fn test_is_bit_valid_shared<'ptr, A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>>(
fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>(
&mut self,
candidate: Maybe<'ptr, $ty, A>,
) -> Option<bool> {
Expand Down
4 changes: 1 addition & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1321,9 +1321,7 @@ pub unsafe trait TryFromBytes {
/// [`UnsafeCell`]: core::cell::UnsafeCell
/// [`Shared`]: invariant::Shared
#[doc(hidden)]
fn is_bit_valid<A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>>(
candidate: Maybe<'_, Self, A>,
) -> bool;
fn is_bit_valid<A: invariant::Reference>(candidate: Maybe<'_, Self, A>) -> bool;

/// Attempts to interpret the given `source` as a `&Self`.
///
Expand Down
17 changes: 13 additions & 4 deletions src/pointer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub type MaybeAligned<'a, T, Aliasing = invariant::Shared, Alignment = invariant
impl<'a, T, Aliasing, Alignment> MaybeAligned<'a, T, Aliasing, Alignment>
where
T: 'a + ?Sized,
Aliasing: invariant::Aliasing + invariant::AtLeast<invariant::Shared>,
Aliasing: invariant::Reference,
Alignment: invariant::Alignment,
{
/// Reads the value from `MaybeAligned`.
Expand All @@ -47,11 +47,20 @@ where
{
let raw = self.as_non_null().as_ptr();
// SAFETY: By invariant on `MaybeAligned`, `raw` contains
// validly-initialized data for `T`. The value is safe to read and
// return, because `T` is copy.
// validly-initialized data for `T`. By `Aliasing: Reference`,
// `Aliasing` is either `Shared` or `Exclusive`, both of which ensure
// that it is sound to perform this read. By `T: Copy`, the value is
// safe to return.
unsafe { core::ptr::read_unaligned(raw) }
}
}

impl<'a, T, Aliasing, Alignment> MaybeAligned<'a, T, Aliasing, Alignment>
where
T: 'a + ?Sized,
Aliasing: invariant::Reference,
Alignment: invariant::Alignment,
{
/// Views the value as an aligned reference.
///
/// This is only available if `T` is [`Unaligned`].
Expand All @@ -70,7 +79,7 @@ pub(crate) fn is_zeroed<T, I>(ptr: Ptr<'_, T, I>) -> bool
where
T: crate::Immutable + crate::KnownLayout,
I: invariant::Invariants<Validity = invariant::Initialized>,
I::Aliasing: invariant::AtLeast<invariant::Shared>,
I::Aliasing: invariant::Reference,
{
ptr.as_bytes::<BecauseImmutable>().as_ref().iter().all(|&byte| byte == 0)
}
Loading

0 comments on commit f421eaa

Please sign in to comment.