Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove Immutable where it's no longer needed #1225

Merged
merged 2 commits into from
May 9, 2024
Merged

Remove Immutable where it's no longer needed #1225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0477bfb
Add `AliasingSafe` framework
joshlf May 9, 2024
ac2ba16
Remove `Immutable` where it's no longer needed
joshlf May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 30 additions & 39 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ use core::{
},
};

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

#[cfg(any(feature = "alloc", test))]
extern crate alloc;
Expand Down Expand Up @@ -1177,7 +1177,7 @@ pub unsafe trait TryFromBytes {
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
match Ptr::from_ref(candidate).try_cast_into_no_leftover::<Self>() {
match Ptr::from_ref(candidate).try_cast_into_no_leftover::<Self, BecauseImmutable>() {
Ok(candidate) => {
// 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 All @@ -1189,7 +1189,9 @@ pub unsafe trait TryFromBytes {
// condition will not happen.
match candidate.try_into_valid() {
Ok(valid) => Ok(valid.as_ref()),
Err(e) => Err(e.map_src(|src| src.as_bytes().as_ref()).into()),
Err(e) => {
Err(e.map_src(|src| src.as_bytes::<BecauseImmutable>().as_ref()).into())
}
}
}
Err(e) => Err(e.map_src(Ptr::as_ref).into()),
Expand Down Expand Up @@ -1425,10 +1427,10 @@ pub unsafe trait TryFromBytes {
#[inline]
fn try_mut_from(bytes: &mut [u8]) -> Result<&mut Self, TryCastError<&mut [u8], Self>>
where
Self: KnownLayout + Immutable, // TODO(#251): Remove the `Immutable` bound.
Self: KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
match Ptr::from_mut(bytes).try_cast_into_no_leftover::<Self>() {
match Ptr::from_mut(bytes).try_cast_into_no_leftover::<Self, BecauseExclusive>() {
Ok(candidate) => {
// 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 All @@ -1440,7 +1442,9 @@ pub unsafe trait TryFromBytes {
// condition will not happen.
match candidate.try_into_valid() {
Ok(candidate) => Ok(candidate.as_mut()),
Err(e) => Err(e.map_src(|src| src.as_bytes().as_mut()).into()),
Err(e) => {
Err(e.map_src(|src| src.as_bytes::<BecauseExclusive>().as_mut()).into())
}
}
}
Err(e) => Err(e.map_src(Ptr::as_mut).into()),
Expand Down Expand Up @@ -1528,7 +1532,7 @@ pub unsafe trait TryFromBytes {
candidate: &mut [u8],
) -> Result<(&mut Self, &mut [u8]), TryCastError<&mut [u8], Self>>
where
Self: KnownLayout + Immutable,
Self: KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
try_mut_from_prefix_suffix(candidate, CastType::Prefix)
Expand Down Expand Up @@ -1615,7 +1619,7 @@ pub unsafe trait TryFromBytes {
candidate: &mut [u8],
) -> Result<(&mut [u8], &mut Self), TryCastError<&mut [u8], Self>>
where
Self: KnownLayout + Immutable,
Self: KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
try_mut_from_prefix_suffix(candidate, CastType::Suffix).map(swap)
Expand Down Expand Up @@ -1707,7 +1711,7 @@ fn try_ref_from_prefix_suffix<T: TryFromBytes + KnownLayout + Immutable + ?Sized
candidate: &[u8],
cast_type: CastType,
) -> Result<(&T, &[u8]), TryCastError<&[u8], T>> {
match Ptr::from_ref(candidate).try_cast_into::<T>(cast_type) {
match Ptr::from_ref(candidate).try_cast_into::<T, BecauseImmutable>(cast_type) {
Ok((candidate, prefix_suffix)) => {
// 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 All @@ -1719,19 +1723,19 @@ fn try_ref_from_prefix_suffix<T: TryFromBytes + KnownLayout + Immutable + ?Sized
// condition will not happen.
match candidate.try_into_valid() {
Ok(valid) => Ok((valid.as_ref(), prefix_suffix.as_ref())),
Err(e) => Err(e.map_src(|src| src.as_bytes().as_ref()).into()),
Err(e) => Err(e.map_src(|src| src.as_bytes::<BecauseImmutable>().as_ref()).into()),
}
}
Err(e) => Err(e.map_src(Ptr::as_ref).into()),
}
}

#[inline(always)]
fn try_mut_from_prefix_suffix<T: TryFromBytes + KnownLayout + Immutable + ?Sized>(
fn try_mut_from_prefix_suffix<T: TryFromBytes + KnownLayout + ?Sized>(
candidate: &mut [u8],
cast_type: CastType,
) -> Result<(&mut T, &mut [u8]), TryCastError<&mut [u8], T>> {
match Ptr::from_mut(candidate).try_cast_into::<T>(cast_type) {
match Ptr::from_mut(candidate).try_cast_into::<T, BecauseExclusive>(cast_type) {
Ok((candidate, prefix_suffix)) => {
// 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 All @@ -1743,7 +1747,7 @@ fn try_mut_from_prefix_suffix<T: TryFromBytes + KnownLayout + Immutable + ?Sized
// condition will not happen.
match candidate.try_into_valid() {
Ok(valid) => Ok((valid.as_mut(), prefix_suffix.as_mut())),
Err(e) => Err(e.map_src(|src| src.as_bytes().as_mut()).into()),
Err(e) => Err(e.map_src(|src| src.as_bytes::<BecauseExclusive>().as_mut()).into()),
}
}
Err(e) => Err(e.map_src(Ptr::as_mut).into()),
Expand Down Expand Up @@ -2332,7 +2336,7 @@ pub unsafe trait FromBytes: FromZeros {
Self: KnownLayout + Immutable,
{
util::assert_dst_is_not_zst::<Self>();
match Ptr::from_ref(bytes).try_cast_into_no_leftover() {
match Ptr::from_ref(bytes).try_cast_into_no_leftover::<_, BecauseImmutable>() {
Ok(ptr) => Ok(ptr.bikeshed_recall_valid().as_ref()),
Err(err) => Err(err.map_src(|src| src.as_ref())),
}
Expand Down Expand Up @@ -2407,7 +2411,7 @@ pub unsafe trait FromBytes: FromZeros {
{
util::assert_dst_is_not_zst::<Self>();
let (slf, suffix) = Ptr::from_ref(bytes)
.try_cast_into(CastType::Prefix)
.try_cast_into::<_, BecauseImmutable>(CastType::Prefix)
.map_err(|err| err.map_src(|s| s.as_ref()))?;
Ok((slf.bikeshed_recall_valid().as_ref(), suffix.as_ref()))
}
Expand Down Expand Up @@ -2467,7 +2471,7 @@ pub unsafe trait FromBytes: FromZeros {
{
util::assert_dst_is_not_zst::<Self>();
let (slf, prefix) = Ptr::from_ref(bytes)
.try_cast_into(CastType::Suffix)
.try_cast_into::<_, BecauseImmutable>(CastType::Suffix)
.map_err(|err| err.map_src(|s| s.as_ref()))?;
Ok((prefix.as_ref(), slf.bikeshed_recall_valid().as_ref()))
}
Expand Down Expand Up @@ -2531,10 +2535,10 @@ pub unsafe trait FromBytes: FromZeros {
#[inline]
fn mut_from(bytes: &mut [u8]) -> Result<&mut Self, CastError<&mut [u8], Self>>
where
Self: IntoBytes + KnownLayout + Immutable,
Self: IntoBytes + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
match Ptr::from_mut(bytes).try_cast_into_no_leftover() {
match Ptr::from_mut(bytes).try_cast_into_no_leftover::<_, BecauseExclusive>() {
Ok(ptr) => Ok(ptr.bikeshed_recall_valid().as_mut()),
Err(err) => Err(err.map_src(|src| src.as_mut())),
}
Expand Down Expand Up @@ -2606,11 +2610,11 @@ pub unsafe trait FromBytes: FromZeros {
bytes: &mut [u8],
) -> Result<(&mut Self, &mut [u8]), CastError<&mut [u8], Self>>
where
Self: IntoBytes + KnownLayout + Immutable,
Self: IntoBytes + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
let (slf, suffix) = Ptr::from_mut(bytes)
.try_cast_into(CastType::Prefix)
.try_cast_into::<_, BecauseExclusive>(CastType::Prefix)
.map_err(|err| err.map_src(|s| s.as_mut()))?;
Ok((slf.bikeshed_recall_valid().as_mut(), suffix.as_mut()))
}
Expand Down Expand Up @@ -2675,11 +2679,11 @@ pub unsafe trait FromBytes: FromZeros {
bytes: &mut [u8],
) -> Result<(&mut [u8], &mut Self), CastError<&mut [u8], Self>>
where
Self: IntoBytes + KnownLayout + Immutable,
Self: IntoBytes + KnownLayout,
{
util::assert_dst_is_not_zst::<Self>();
let (slf, prefix) = Ptr::from_mut(bytes)
.try_cast_into(CastType::Suffix)
.try_cast_into::<_, BecauseExclusive>(CastType::Suffix)
.map_err(|err| err.map_src(|s| s.as_mut()))?;
Ok((prefix.as_mut(), slf.bikeshed_recall_valid().as_mut()))
}
Expand Down Expand Up @@ -3479,7 +3483,7 @@ pub unsafe trait IntoBytes {
#[inline(always)]
fn as_mut_bytes(&mut self) -> &mut [u8]
where
Self: FromBytes + Immutable,
Self: FromBytes,
{
// Note that this method does not have a `Self: Sized` bound;
// `size_of_val` works for unsized values too.
Expand Down Expand Up @@ -3714,7 +3718,7 @@ pub unsafe trait IntoBytes {
#[inline]
fn as_bytes_mut(&mut self) -> &mut [u8]
where
Self: FromBytes + Immutable,
Self: FromBytes,
{
self.as_mut_bytes()
}
Expand Down Expand Up @@ -5072,20 +5076,16 @@ macro_rules! transmute_mut {
struct AssertSrcIsSized<'a, T: ::core::marker::Sized>(&'a T);
struct AssertSrcIsFromBytes<'a, T: ?::core::marker::Sized + $crate::FromBytes>(&'a T);
struct AssertSrcIsIntoBytes<'a, T: ?::core::marker::Sized + $crate::IntoBytes>(&'a T);
struct AssertSrcIsImmutable<'a, T: ?::core::marker::Sized + $crate::Immutable>(&'a T);
struct AssertDstIsSized<'a, T: ::core::marker::Sized>(&'a T);
struct AssertDstIsFromBytes<'a, T: ?::core::marker::Sized + $crate::FromBytes>(&'a T);
struct AssertDstIsIntoBytes<'a, T: ?::core::marker::Sized + $crate::IntoBytes>(&'a T);
struct AssertDstIsImmutable<'a, T: ?::core::marker::Sized + $crate::Immutable>(&'a T);

if true {
let _ = AssertSrcIsSized(&*e);
} else if true {
let _ = AssertSrcIsFromBytes(&*e);
} else if true {
let _ = AssertSrcIsIntoBytes(&*e);
} else {
let _ = AssertSrcIsImmutable(&*e);
let _ = AssertSrcIsIntoBytes(&*e);
}

if true {
Expand All @@ -5096,13 +5096,9 @@ macro_rules! transmute_mut {
#[allow(unused, unreachable_code)]
let u = AssertDstIsFromBytes(loop {});
&mut *u.0
} else if true {
#[allow(unused, unreachable_code)]
let u = AssertDstIsIntoBytes(loop {});
&mut *u.0
} else {
#[allow(unused, unreachable_code)]
let u = AssertDstIsImmutable(loop {});
let u = AssertDstIsIntoBytes(loop {});
&mut *u.0
}
} else if false {
Expand All @@ -5125,11 +5121,6 @@ macro_rules! transmute_mut {
&mut u
} else {
// SAFETY: For source type `Src` and destination type `Dst`:
// - We know that `Src: FromBytes + IntoBytes + Immutable` and `Dst:
// FromBytes + IntoBytes + Immutable` thanks to the uses of
// `AssertSrcIsFromBytes`, `AssertSrcIsIntoBytes`,
// `AssertSrcIsImmutable`, `AssertDstIsFromBytes`,
// `AssertDstIsIntoBytes`, and `AssertDstIsImmutable` above.
// - We know that `size_of::<Src>() == size_of::<Dst>()` thanks to
// the use of `assert_size_eq!` above.
// - We know that `align_of::<Src>() >= align_of::<Dst>()` thanks to
Expand Down
7 changes: 2 additions & 5 deletions src/macro_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,8 @@ pub const unsafe fn transmute_ref<'dst, 'src: 'dst, Src: 'src, Dst: 'dst>(
/// # Safety
///
/// The caller must guarantee that:
/// - `Src: FromBytes + IntoBytes + Immutable`
/// - `Dst: FromBytes + IntoBytes + Immutable`
/// - `Src: FromBytes + IntoBytes`
/// - `Dst: FromBytes + IntoBytes`
/// - `size_of::<Src>() == size_of::<Dst>()`
/// - `align_of::<Src>() >= align_of::<Dst>()`
// TODO(#686): Consider removing the `Immutable` requirement.
Expand All @@ -403,9 +403,6 @@ pub unsafe fn transmute_mut<'dst, 'src: 'dst, Src: 'src, Dst: 'dst>(
// vice-versa because the caller has guaranteed that `Src: FromBytes +
// IntoBytes`, `Dst: FromBytes + IntoBytes`, and `size_of::<Src>() ==
// size_of::<Dst>()`.
// - We know that there are no `UnsafeCell`s, and thus we don't have to
// worry about `UnsafeCell` overlap, because `Src: Immutable`
// and `Dst: Immutable`.
// - The caller has guaranteed that alignment is not increased.
// - We know that the returned lifetime will not outlive the input lifetime
// thanks to the lifetime bounds on this function.
Expand Down
89 changes: 89 additions & 0 deletions src/pointer/aliasing_safety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright 2024 The Fuchsia Authors
//
// Licensed under a BSD-style license <LICENSE-BSD>, Apache License, Version 2.0
// <LICENSE-APACHE or https://www.apache.org/licenses/LICENSE-2.0>, or the MIT
// license <LICENSE-MIT or https://opensource.org/licenses/MIT>, at your option.
// This file may not be copied, modified, or distributed except according to
// those terms.

//! Machinery for statically proving the "aliasing-safety" of a `Ptr`.

use crate::{invariant, 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`]
///
/// [`Exclusive`]: crate::pointer::invariant::Exclusive
#[doc(hidden)]
pub unsafe trait AliasingSafe<T: ?Sized, A: invariant::Aliasing, R: AliasingSafeReason> {}

/// Used to prevent user implementations of `AliasingSafeReason`.
mod sealed {
pub trait Sealed {}

impl Sealed for super::BecauseExclusive {}
impl Sealed for super::BecauseImmutable {}
impl<S: Sealed> Sealed for (S,) {}
}

#[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, invariant::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: invariant::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: invariant::Aliasing,
R: AliasingSafeReason,
U: AliasingSafe<T, A, R>,
{
}
4 changes: 3 additions & 1 deletion src/pointer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

//! Abstractions over raw pointers.

mod aliasing_safety;
mod ptr;

pub use aliasing_safety::{AliasingSafe, BecauseExclusive, BecauseImmutable};
pub use ptr::{invariant, Ptr};

use crate::Unaligned;
Expand Down Expand Up @@ -70,5 +72,5 @@ where
I: invariant::Invariants<Validity = invariant::Initialized>,
I::Aliasing: invariant::AtLeast<invariant::Shared>,
{
ptr.as_bytes().as_ref().iter().all(|&byte| byte == 0)
ptr.as_bytes::<BecauseImmutable>().as_ref().iter().all(|&byte| byte == 0)
}
Loading
Loading