Skip to content

Commit d9e48e3

Browse files
authored
[pointer] Clarify semantics of aliasing invariants (#1889)
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
1 parent 1c8cbc7 commit d9e48e3

File tree

9 files changed

+212
-311
lines changed

9 files changed

+212
-311
lines changed

src/impls.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -657,9 +657,7 @@ unsafe impl<T: TryFromBytes + ?Sized> TryFromBytes for UnsafeCell<T> {
657657
}
658658

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

11351133
pub(super) trait TestIsBitValidShared<T: ?Sized> {
11361134
#[allow(clippy::needless_lifetimes)]
1137-
fn test_is_bit_valid_shared<
1138-
'ptr,
1139-
A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>,
1140-
>(
1135+
fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>(
11411136
&self,
11421137
candidate: Maybe<'ptr, T, A>,
11431138
) -> Option<bool>;
11441139
}
11451140

11461141
impl<T: TryFromBytes + Immutable + ?Sized> TestIsBitValidShared<T> for AutorefWrapper<T> {
11471142
#[allow(clippy::needless_lifetimes)]
1148-
fn test_is_bit_valid_shared<
1149-
'ptr,
1150-
A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>,
1151-
>(
1143+
fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>(
11521144
&self,
11531145
candidate: Maybe<'ptr, T, A>,
11541146
) -> Option<bool> {
@@ -1238,7 +1230,7 @@ mod tests {
12381230
#[allow(unused, non_local_definitions)]
12391231
impl AutorefWrapper<$ty> {
12401232
#[allow(clippy::needless_lifetimes)]
1241-
fn test_is_bit_valid_shared<'ptr, A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>>(
1233+
fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>(
12421234
&mut self,
12431235
candidate: Maybe<'ptr, $ty, A>,
12441236
) -> Option<bool> {

src/lib.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,9 +1321,7 @@ pub unsafe trait TryFromBytes {
13211321
/// [`UnsafeCell`]: core::cell::UnsafeCell
13221322
/// [`Shared`]: invariant::Shared
13231323
#[doc(hidden)]
1324-
fn is_bit_valid<A: invariant::Aliasing + invariant::AtLeast<invariant::Shared>>(
1325-
candidate: Maybe<'_, Self, A>,
1326-
) -> bool;
1324+
fn is_bit_valid<A: invariant::Reference>(candidate: Maybe<'_, Self, A>) -> bool;
13271325

13281326
/// Attempts to interpret the given `source` as a `&Self`.
13291327
///

src/pointer/mod.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub type MaybeAligned<'a, T, Aliasing = invariant::Shared, Alignment = invariant
3535
impl<'a, T, Aliasing, Alignment> MaybeAligned<'a, T, Aliasing, Alignment>
3636
where
3737
T: 'a + ?Sized,
38-
Aliasing: invariant::Aliasing + invariant::AtLeast<invariant::Shared>,
38+
Aliasing: invariant::Reference,
3939
Alignment: invariant::Alignment,
4040
{
4141
/// Reads the value from `MaybeAligned`.
@@ -47,11 +47,20 @@ where
4747
{
4848
let raw = self.as_non_null().as_ptr();
4949
// SAFETY: By invariant on `MaybeAligned`, `raw` contains
50-
// validly-initialized data for `T`. The value is safe to read and
51-
// return, because `T` is copy.
50+
// validly-initialized data for `T`. By `Aliasing: Reference`,
51+
// `Aliasing` is either `Shared` or `Exclusive`, both of which ensure
52+
// that it is sound to perform this read. By `T: Copy`, the value is
53+
// safe to return.
5254
unsafe { core::ptr::read_unaligned(raw) }
5355
}
56+
}
5457

58+
impl<'a, T, Aliasing, Alignment> MaybeAligned<'a, T, Aliasing, Alignment>
59+
where
60+
T: 'a + ?Sized,
61+
Aliasing: invariant::Reference,
62+
Alignment: invariant::Alignment,
63+
{
5564
/// Views the value as an aligned reference.
5665
///
5766
/// This is only available if `T` is [`Unaligned`].
@@ -70,7 +79,7 @@ pub(crate) fn is_zeroed<T, I>(ptr: Ptr<'_, T, I>) -> bool
7079
where
7180
T: crate::Immutable + crate::KnownLayout,
7281
I: invariant::Invariants<Validity = invariant::Initialized>,
73-
I::Aliasing: invariant::AtLeast<invariant::Shared>,
82+
I::Aliasing: invariant::Reference,
7483
{
7584
ptr.as_bytes::<BecauseImmutable>().as_ref().iter().all(|&byte| byte == 0)
7685
}

0 commit comments

Comments
 (0)