From dd70b7f9fd6478310cb10279b7e1e4729cba43e3 Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Tue, 7 May 2024 13:50:08 -0700 Subject: [PATCH] [pointer] Relax UnsafeCell requirements Previously, we required there to always be "`UnsafeCell` agreement" between any `Ptr`s or references referencing a given region of memory. This was based on an overly-strict interpretation of the semantics of `UnsafeCell`. In this commit, we relax `Ptr` to only require "`UnsafeCell` agreement" when the aliasing model is `Shared`. All of the places that our internal invariants are "consumed" - ie, where we use them as safety preconditions for calling unsafe functions defined outside our crate - this relaxation is sufficient. This is based on what we (@jswrenn and I) believe to be a more accurate model of the semantics of `UnsafeCell`s. In particular, `UnsafeCell`s do not affect the semantics of loads or stores in Rust. All they do is affect the semantics of shared references. In particular, Rust assumes that the referent of a shared reference will not be stored to during the lifetime of any shared reference, but this assumption is not made for bytes which are covered by an `UnsafeCell`. This is entirely a runtime property. If two references refer to the same memory, but disagree on whether that memory is covered by an `UnsafeCell`, this results in UB if the `UnsafeCell` is used to store to the memory, violating the expectations of the non-`UnsafeCell` shared reference. This commit is consistent with the runtime nature of this property, but is inconsistent with Stacked Borrows (rust-lang/unsafe-code-guidelines#455). However, this is considered to be a bug in Stacked Borrows. --- src/lib.rs | 17 +- src/macros.rs | 18 +- src/pointer/ptr.rs | 290 ++++++------------ src/util.rs | 45 ++- src/wrappers.rs | 10 +- zerocopy-derive/src/lib.rs | 32 +- .../tests/struct_try_from_bytes.rs | 19 +- zerocopy-derive/tests/union_try_from_bytes.rs | 18 +- 8 files changed, 172 insertions(+), 277 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b5a154fcbc..8bbf03856a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3935,12 +3935,17 @@ unsafe impl TryFromBytes for UnsafeCell { // We wrap in `Unalign` here so that we can get a vanilla Rust reference // below, which in turn allows us to call `UnsafeCell::get_mut`. // - // SAFETY: `Unalign` and `MaybeUninit` both have the same size as the - // types they wrap [1]. Thus, this cast will preserve the size of the - // pointer. Since both the source and destination types are wrapped in - // `UnsafeCell`, all bytes of both types are inside of `UnsafeCell`s, - // and so the byte ranges covered by `UnsafeCell`s are identical in both - // types. + // SAFETY: + // - `.cast` preserves address. `Unalign` and `MaybeUninit` both have + // the same size as the types they wrap [1]. Thus, this cast will + // preserve the size of the pointer. As a result, the cast will + // address the same bytes as `c`. + // - `.cast` preserves provenance. + // - Since both the source and destination types are wrapped in + // `UnsafeCell`, all bytes of both types are inside of `UnsafeCell`s, + // and so the byte ranges covered by `UnsafeCell`s are identical in + // both types. Since the pointers refer to the same byte ranges, + // the same is true of the pointers' referents as well. // // [1] Per https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#layout-1: // diff --git a/src/macros.rs b/src/macros.rs index 062cb250d5..baf20aa330 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -142,10 +142,11 @@ macro_rules! unsafe_impl { #[inline] fn is_bit_valid>(candidate: Maybe<'_, Self, AA>) -> bool { // SAFETY: - // - The argument to `cast_unsized` is `|p| p as *mut _` as required - // by that method's safety precondition. - // - The caller has promised that the cast results in an object of - // equal or lesser size. + // - The cast preserves address. The caller has promised that the + // cast results in an object of equal or lesser size, and so the + // cast returns a pointer which references a subset of the bytes + // of `p`. + // - The cast preserves provenance. // - The caller has promised that the destination type has // `UnsafeCell`s at the same byte ranges as the source type. #[allow(clippy::as_conversions)] @@ -164,10 +165,11 @@ macro_rules! unsafe_impl { #[inline] fn is_bit_valid>(candidate: Maybe<'_, Self, AA>) -> bool { // SAFETY: - // - The argument to `cast_unsized` is `|p| p as *mut _` as required - // by that method's safety precondition. - // - The caller has promised that the cast results in an object of - // equal or lesser size. + // - The cast preserves address. The caller has promised that the + // cast results in an object of equal or lesser size, and so the + // cast returns a pointer which references a subset of the bytes + // of `p`. + // - The cast preserves provenance. // - The caller has promised that the destination type has // `UnsafeCell`s at the same byte ranges as the source type. #[allow(clippy::as_conversions)] diff --git a/src/pointer/ptr.rs b/src/pointer/ptr.rs index 25ebcbf713..7a34a14cde 100644 --- a/src/pointer/ptr.rs +++ b/src/pointer/ptr.rs @@ -60,12 +60,6 @@ mod def { /// [`I::Alignment`](invariant::Alignment). /// 9. `ptr` conforms to the validity invariant of /// [`I::Validity`](invariant::Validity). - /// 10. During the lifetime 'a, no code will load or store this memory - /// region treating `UnsafeCell`s as existing at different ranges - /// than they exist in `T`. - /// 11. During the lifetime 'a, no reference will exist to this memory - /// which treats `UnsafeCell`s as existing at different ranges than - /// they exist in `T`. // SAFETY: `NonNull` is covariant over `T` [1]. // // [1]: https://doc.rust-lang.org/std/ptr/struct.NonNull.html @@ -100,12 +94,6 @@ mod def { /// [`I::Alignment`](invariant::Alignment). /// 8. `ptr` conforms to the validity invariant of /// [`I::Validity`](invariant::Validity). - /// 9. During the lifetime 'a, no code will load or store this memory - /// region treating `UnsafeCell`s as existing at different ranges - /// than they exist in `T`. - /// 10. During the lifetime 'a, no reference will exist to this memory - /// which treats `UnsafeCell`s as existing at different ranges than - /// they exist in `T`. pub(super) unsafe fn new(ptr: NonNull) -> Ptr<'a, T, I> { // SAFETY: The caller has promised to satisfy all safety invariants // of `Ptr`. @@ -386,10 +374,6 @@ mod _conversions { // invariant of `Aligned`. // 9. `ptr`, by invariant on `&'a T`, conforms to the validity // invariant of `Valid`. - // 10. The referents of `Ptr` and `&T` have `UnsafeCell`s at - // identical ranges. Both `Ptr` and `&T` prevent loads and - // stores which treat other byte ranges as having `UnsafeCell`s. - // 11. See 10. unsafe { Self::new(ptr) } } } @@ -462,8 +446,8 @@ mod _conversions { // // 4. You must enforce Rust’s aliasing rules. This is ensured by // contract on `Ptr`, because the `I::Aliasing` is - // `AtLeast`. Either it is `Shared` or `Exclusive`. In - // both cases, other references may not mutate the referent + // `AtLeast`. Either it is `Shared` or `Exclusive`. If it + // is `Shared`, other references may not mutate the referent // outside of `UnsafeCell`s. // // [1]: https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.as_ref @@ -505,12 +489,6 @@ mod _conversions { // [`I::Alignment`](invariant::Alignment). // 8. `ptr` conforms to the validity invariant of // [`I::Validity`](invariant::Validity). - // 9. During the lifetime 'a, no code will load or store this memory - // region treating `UnsafeCell`s as existing at different ranges - // than they exist in `T`. - // 10. During the lifetime 'a, no reference will exist to this - // memory which treats `UnsafeCell`s as existing at different - // ranges than they exist in `T`. // // For aliasing (6 above), since `I::Aliasing: AtLeast`, // there are two cases for `I::Aliasing`: @@ -592,11 +570,14 @@ mod _conversions { > { // SAFETY: // - By invariant on `TransparentWrapper::cast_into_inner`: - // - This cast preserves the referent's size. - // - This cast preserves provenance. + // - This cast preserves address and referent size, and thus the + // returned pointer addresses the same bytes as `p` + // - This cast preserves provenance // - By invariant on `TransparentWrapper`, `T` and `T::Inner` have `UnsafeCell`s at the same - // byte ranges. + // 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)) }; // SAFETY: By invariant on `TransparentWrapper`, since `self` // satisfies the alignment invariant `I::Alignment`, `c` (of type @@ -624,21 +605,21 @@ mod _conversions { pub(crate) fn into_unalign( self, ) -> Ptr<'a, crate::Unalign, (I::Aliasing, Aligned, I::Validity)> { - // SAFETY: We define `Unalign` to be a `#[repr(C, packed)]` type - // wrapping a single `T` field. Thus, `Unalign` has the same size - // as `T` and contains `UnsafeCell`s at the same locations as `T`. - // The cast is implemented in the form `|p: *mut T| p as *mut U`, - // where `U` is `Unalign`. + // SAFETY: + // - This cast preserves provenance. + // - This cast preserves address. `Unalign` promises to have the + // same size as `T`, and so the cast returns a pointer addressing + // the same byte range as `p`. + // - By the same argument, the returned pointer refers to + // `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) }; - // SAFETY: We define `Unalign` to be a `#[repr(C, packed)]` type - // wrapping a single `T` field, thus `Unalign` has exactly the - // same validity as `T`. + // SAFETY: `Unalign` promises to have the same bit validity as + // `T`. let ptr = unsafe { ptr.assume_validity::() }; - // SAFETY: We define `Unalign` to be a `#[repr(C, packed)]` type - // wrapping a single `T` field, thus `Unalign` is always + // SAFETY: `Unalign` promises to have alignment 1, and so it is // trivially aligned. let ptr = unsafe { ptr.assume_alignment::() }; ptr @@ -940,37 +921,13 @@ mod _casts { /// /// # Safety /// - /// The caller promises that `cast(p)` is a pointer cast that does not - /// increase the referent's size. A 'cast', here, means a - /// provenance-preserving transformation of a pointer-like value into - /// another pointer-like value to the same allocation. - /// - /// For all kinds of casts, the caller must promise that: - /// - the the size of the object referenced by the resulting pointer is - /// less than or equal to the size of the object referenced by `self`. - /// - `UnsafeCell`s in `U` exist at ranges identical to those at which - /// `UnsafeCell`s exist in `T`. - /// - /// For pointer-to-pointer casts, it suffices for the caller to - /// additionally promise that `cast(p)` is implemented as: - /// ```ignore - /// |p: *mut T| p as *mut U - /// ``` - /// - /// ...or as: - /// ```ignore - /// |p: *mut T| p.cast::() - /// ``` - /// - /// For pointer-to-slice casts, it sufficies for the caller to - /// additionally promise that `cast` is implemented as: - /// ```ignore - /// |p: *mut T| - /// core::ptr::slice_from_raw_parts_mut( - /// p.cast::(), - /// len, - /// ) - /// ``` + /// 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` + /// - If `I::Aliasing` is [`Any`] or [`Shared`], `UnsafeCell`s in `*u` + /// must exist at ranges identical to those at which `UnsafeCell`s + /// exist in `*p` #[doc(hidden)] #[inline] pub unsafe fn cast_unsized *mut U>( @@ -979,42 +936,58 @@ mod _casts { ) -> Ptr<'a, U, (I::Aliasing, Any, Any)> { let ptr = cast(self.as_non_null().as_ptr()); - // SAFETY: Caller promises that `cast` is just a cast. We call - // `cast` on `self.ptr.as_non_null().as_ptr()`, which is non-null by - // construction. + // SAFETY: Caller promises that `cast` returns a pointer whose + // address is in the range of `self.as_non_null()`'s referent. By + // invariant, none of these addresses are null. let ptr = unsafe { NonNull::new_unchecked(ptr) }; - // SAFETY: Lemma: In the following safety arguments, note that the - // caller promises that `cast` produces a pointer which addresses no - // more bytes than those addressed by its input pointer. As a - // result, the bytes addressed by `ptr` are a subset of the bytes - // addressed by `self`. + // SAFETY: // - // 0. By invariant on `self` and contract on `cast`, `ptr` is - // derived from some valid Rust allocation, `A`. - // 1. By invariant on `self` and contract on `cast` (its - // implementation is provenance-preserving), `ptr` has valid - // provenance for `A`. - // 2. By the above lemma, `ptr` addresses a byte range which is - // entirely contained in `A`. - // 3. By the above lemma, `ptr` addresses a byte range whose length - // fits in an `isize`. - // 4. By the above lemma, `ptr` addresses a byte range which does - // not wrap around the address space. - // 5. By invariant on `self` and contract on `cast`, `A` is - // guaranteed to live for at least `'a`. - // 6. `ptr` conforms to the aliasing invariant of `I::Aliasing` - // because casting does not impact the aliasing invariant. + // Lemma 1: `ptr` has the same provenance as `self`. The caller + // promises that `cast` preserves provenance, and we call it with + // `self.as_non_null()`. + // + // 0. By invariant, `self` is derived from some valid Rust + // allocation, `A`. By Lemma 1, `ptr` has the same provenance as + // `self`. Thus, `ptr` is derived from `A`. + // 1. By invariant, `self` has valid provenance for `A`. By Lemma 1, + // so does `ptr`. + // 2. By invariant on `self` and caller precondition, `ptr` + // addresses a byte range which is entirely contained in `A`. + // 3. By invariant on `self` and caller precondition, `ptr` + // addresses a byte range whose length fits in an `isize`. + // 4. By invariant on `self` and caller precondition, `ptr` + // addresses a byte range which does not wrap around the address + // space. + // 5. By invariant on `self`, `A` is guaranteed to live for at least + // `'a`. + // 6. `ptr` conforms to the aliasing invariant of `I::Aliasing`: + // - `Exclusive`: `self` is the only `Ptr` or reference which is + // permitted to read or modify the referent for the lifetime + // `'a`. Since we consume `self` by value, the returned pointer + // remains the only `Ptr` or reference which is permitted to + // read or modify the referent for the lifetime `'a`. + // - `Shared`: Since `self` has aliasing `Shared`, we know that + // no other code may mutate the referent during the lifetime + // `'a`, except via `UnsafeCell`s. The caller promises that + // `UnsafeCell`s cover the same byte ranges in `*self` and + // `*ptr`. For each byte in the referent, there are two cases: + // - If the byte is not covered by an `UnsafeCell` in `*ptr`, + // then it is not covered in `*self`. By invariant on `self`, + // it will not be mutated during `'a`, as required by the + // constructed pointer. Similarly, the returned pointer will + // not permit any mutations to these locations, as required + // by the invariant on `self`. + // - If the byte is covered by an `UnsafeCell` in `*ptr`, then + // the returned pointer's invariants do not assume that the + // byte will not be mutated during `'a`. While the returned + // pointer will permit mutation of this byte during `'a`, by + // invariant on `self`, no other code assumes that this will + // not happen. // 7. `ptr`, trivially, conforms to the alignment invariant of // `Any`. // 8. `ptr`, trivially, conforms to the validity invariant of // `Any`. - // 9. During the lifetime 'a, no code will reference or load or - // store this memory region treating `UnsafeCell`s as existing at - // different ranges than they exist in `U`. This is true by - // invariant on Ptr<'a, T, I>, and preserved through the cast to - // `U` by contract on the caller. - // 10. See 9. unsafe { Ptr::new(ptr) } } } @@ -1038,12 +1011,11 @@ mod _casts { None => unsafe { core::hint::unreachable_unchecked() }, }; - // SAFETY: We ensure that: - // - `cast(p)` is implemented as an invocation to - // `slice_from_raw_parts_mut`. - // - The size of the object referenced by the resulting pointer is - // exactly equal to the size of the object referenced by `self` as - // guaranteed by `KnownLayout::size_of_val_raw` + // SAFETY: + // - `slice_from_raw_parts_mut` and `.cast` both preserve the + // 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 // - `T` and `[u8]` trivially contain `UnsafeCell`s at identical // ranges [u8]`, because both are `Immutable`. let ptr: Ptr<'a, [u8], _> = unsafe { @@ -1112,13 +1084,6 @@ mod _casts { // 8. By the above lemma, `slice` conforms to the validity invariant // of `I::Validity`, because the operations that produced `slice` // from `self` do not impact validity. - // 9. During the lifetime 'a, no code will reference or load or - // store this memory region treating `UnsafeCell`s as existing at - // different ranges than they exist in `[T]`. This is true by - // invariant on Ptr<'a, [T; N], I>, and preserved through the - // cast to `[T]` because `[T]` has `UnsafeCell`s at exactly the - // same ranges as `[T; N]`. - // 10. See 9. unsafe { Ptr::new(slice) } } } @@ -1218,13 +1183,6 @@ mod _casts { // `[u8]`. All bit-valid `[u8]`s have all of their bytes // initialized, so `ptr` conforms to the validity invariant of // `Initialized`. - // 9. During the lifetime 'a, no code will reference or load from or - // store to `target`'s referent memory region treating - // `UnsafeCell`s as existing at different ranges than they exist - // in `U`. This is true because neither the source type (`[u8]`) - // nor the destination type (`U: Immutable`) contain any - // `UnsafeCell`s. - // 10. See 9. Ok((unsafe { Ptr::new(ptr) }, remainder)) } @@ -1289,90 +1247,25 @@ mod _project { /// /// # Safety /// - /// ## Preconditions - /// - /// The caller promises that: - /// - `projector` projects a field of its argument. Its argument will be - /// `self` casted to a raw pointer. The pointer it returns must - /// reference only a subset of `self`'s bytes. - /// - `T` is a struct or union type. - /// - The projected pointer contains `UnsafeCell`s at exactly the same - /// ranges at which `UnsafeCell`s appear in the projected-from type. - /// This is necessarily true for projections of struct fields, but not - /// for all projections of union fields. - /// - /// ## Postconditions - /// - /// If the preconditions of this function are met, this function will - /// return a `Ptr` to the field projected from `self` by `projector`. + /// `project` has the same safety preconditions as `cast_unsized`. #[doc(hidden)] #[inline] pub unsafe fn project( self, projector: impl FnOnce(*mut T) -> *mut U, ) -> Ptr<'a, U, (I::Aliasing, Any, Initialized)> { - // SAFETY: `projector` is provided with `self` casted to a raw - // pointer. - let field = projector(self.as_non_null().as_ptr()); - - // SAFETY: We promise that `projector` is provided with `self` - // casted to a raw pointer, and the caller promises that `projector` - // is a field projection of its argument. By invariant on `Ptr`, - // `self` is non-null, and by contract on `projector`, so too will - // its return value. - // - // Note that field projection cannot wrap around the address space - // to null. - // - // TODO(https://github.com/rust-lang/rust/pull/116675): Cite - // documentation that allocated objects do not wrap around the - // address space. - let field = unsafe { NonNull::new_unchecked(field) }; - - // SAFETY: The safety invariants of `Ptr::new` (see definition) are - // satisfied: - // 0. `field` is derived from a valid Rust allocation, because - // `self` is derived from a valid Rust allocation, by invariant - // on `Ptr`, and `projector` (by contract) is a field projection - // through `self`. - // 1. `field` has valid provenance for `self`, because it derived - // from `self` using a series of provenance-preserving - // operations. - // 2. `field` is entirely contained in the allocation of `self`, - // because it is derived by `projector`, which is (by contract) a - // field projection through `self`. - // 3. `field` addresses a byte range whose length fits in an - // `isize`, because it is a field projection through `self` and - // thus is entirely contained by `self`, which satisfies this - // invariant. - // 4. `field` addresses a byte range which does not wrap around the - // address space (see above). - // 5. The allocation of `field` is guaranteed to live for at least - // `'a`, because `field` is entirely contained in `self`, which - // lives for at least `'a` by invariant on `Ptr`. - // 6. `field` conforms to the aliasing invariant of `I::Aliasing` - // because projection does not impact the aliasing invariant. - // 7. `field`, trivially, conforms to the alignment invariant of - // `Any`. - // 8. By type bound on `I::Validity`, `self` satisfies the - // "as-initialized" property relative to `T`. The returned `Ptr` - // has the validity `AsInitialized`. The caller promises that `T` - // is either a struct type or a union type. Returning a `Ptr` - // with the validity `AsInitialized` is valid in both cases. The - // struct case is self-explanatory, but the union case bears - // explanation. The "as-initialized" property says that a byte - // must be initialized if it is initialized in *any* instance of - // the type. Thus, if `self`'s referent is as-initialized as `T`, - // then it is at least as-initialized as each of its fields. - // 9. During the lifetime 'a, no code will reference or load or - // store this memory region treating `UnsafeCell`s as existing at - // different ranges than they exist in `U`. This is true by - // invariant on Ptr<'a, T, I>, preserved by precondition on - // `cast`. The field type cannot contain `UnsafeCell`s at ranges - // that are not `UnsafeCell` in its parent struct/enum type, - // because the field type is contained in the struct/enum type. - // 10. See 9. - unsafe { Ptr::new(field) } + // TODO(#1122): If `cast_unsized` were able to reason that, when + // casting from an `Initialized` pointer, the result is another + // `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) }; + + // SAFETY: If all of the bytes of `self` are initialized (as + // promised by `I: Invariants`), then any + // subset of those bytes are also all initialized. + return unsafe { ptr.assume_validity::() }; } } @@ -1553,13 +1446,6 @@ mod _project { // 8. `elem`, conditionally, conforms to the validity invariant // of `I::Validity`. If `elem` is projected from data valid // for `[T]`, `elem` will be valid for `T`. - // 9. During the lifetime 'a, no code will reference or load or - // store this memory region treating `UnsafeCell`s as - // existing at different ranges than they exist in `T`. This - // property holds by invariant on `Ptr<'a, [T], I>`, and is - // preserved because `Ptr<'a, T, I>` is an element within - // `Ptr<'a, [T], I>`. - // 10. See 9. unsafe { Ptr::new(elem) } }) } diff --git a/src/util.rs b/src/util.rs index 6a6d88adae..083cd6a1d8 100644 --- a/src/util.rs +++ b/src/util.rs @@ -28,10 +28,8 @@ use crate::{ /// /// `T: TransparentWrapper` implies that `T` has the same size as [`T::Inner`]. /// Further, `T: TransparentWrapper` implies that: -/// - If `T::UnsafeCellVariance = Covariant`, then: -/// - `T` has `UnsafeCell`s covering the same byte ranges as `T::Inner` -/// - `T` has zero-sized `UnsafeCell`s (e.g., `UnsafeCell<()>`, -/// `[UnsafeCell; 0]`, etc) at the same byte offsets as `T::Inner` +/// - If `T::UnsafeCellVariance = Covariant`, then `T` has `UnsafeCell`s +/// covering the same byte ranges as `T::Inner`. /// - If a `T` pointer satisfies the alignment invariant `I::Alignment`, then /// that same pointer, cast to `T::Inner`, satisfies the alignment invariant /// `>::Applied`. @@ -112,8 +110,8 @@ impl ValidityVariance for Invariant { unsafe impl TransparentWrapper for MaybeUninit { type Inner = T; - // SAFETY: Per [1], `MaybeUninit` has `UnsafeCell`s at the same byte - // ranges as `Inner = T`, and `UnsafeCell`s at the same byte offsets as `T`. + // SAFETY: Per [1], `MaybeUninit` has `UnsafeCell`s covering the same + // byte ranges as `Inner = T`. // // [1] TODO(#896): Write a safety proof before the next stable release. type UnsafeCellVariance = Covariant; @@ -154,8 +152,8 @@ unsafe impl TransparentWrapper for MaybeUninit { unsafe impl TransparentWrapper for ManuallyDrop { type Inner = T; - // SAFETY: Per [1], `ManuallyDrop` has `UnsafeCell`s at the same byte - // ranges as `Inner = T`, and `UnsafeCell`s at the same byte offsets as `T`. + // SAFETY: Per [1], `ManuallyDrop` has `UnsafeCell`s covering the same + // byte ranges as `Inner = T`. // // [1] TODO(#896): Write a safety proof before the next stable release. type UnsafeCellVariance = Covariant; @@ -199,8 +197,8 @@ unsafe impl TransparentWrapper for ManuallyDrop unsafe impl TransparentWrapper for Wrapping { type Inner = T; - // SAFETY: Per [1], `Wrapping` has `UnsafeCell`s at the same byte ranges - // as `Inner = T`, and `UnsafeCell`s at the same byte offsets as `T`. + // SAFETY: Per [1], `Wrapping` has `UnsafeCell`s covering the same byte + // ranges as `Inner = T`. // // [1] TODO(#896): Write a safety proof before the next stable release. type UnsafeCellVariance = Covariant; @@ -249,8 +247,8 @@ unsafe impl TransparentWrapper for Wrapping { // // [1] Per https://doc.rust-lang.org/core/cell/struct.UnsafeCell.html#memory-layout: // -// `UnsafeCell`` has the same in-memory representation as its inner -// type `T`. +// `UnsafeCell` has the same in-memory representation as its inner type +// `T`. unsafe impl TransparentWrapper for UnsafeCell { type Inner = T; @@ -268,9 +266,9 @@ unsafe impl TransparentWrapper for UnsafeCell { // // [1] Per https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html#memory-layout: // - // `UnsafeCell`` has the same in-memory representation as its inner - // type `T`. A consequence of this guarantee is that it is possible to - // convert between `T` and `UnsafeCell`. + // `UnsafeCell` has the same in-memory representation as its inner type + // `T`. A consequence of this guarantee is that it is possible to convert + // between `T` and `UnsafeCell`. type ValidityVariance = Covariant; fn cast_into_inner(ptr: *mut UnsafeCell) -> *mut T { @@ -292,25 +290,22 @@ unsafe impl TransparentWrapper for UnsafeCell { } } -// SAFETY: We define `Unalign` to be a `#[repr(C, packed)]` type wrapping a -// single `T` field. Thus, `Unalign` has the same size as `T`. +// SAFETY: `Unalign` promises to have the same size as `T`. // // See inline comments for other safety justifications. unsafe impl TransparentWrapper for Unalign { type Inner = T; - // SAFETY: `Unalign` is a `#[repr(C, packed)]` type wrapping a single `T` - // field, and so has `UnsafeCell`s at the same byte ranges as `Inner = T`, - // and `UnsafeCell`s at the same byte offsets as `T`. + // SAFETY: `Unalign` promises to have `UnsafeCell`s covering the same + // byte ranges as `Inner = T`. type UnsafeCellVariance = Covariant; - // SAFETY: Since `Unalign` is `repr(packed)`, it has the alignment 1 - // regardless of `T`'s alignment. Thus, an aligned pointer to `Unalign` - // is not necessarily an aligned pointer to `T`. + // SAFETY: Since `Unalign` promises to have alignment 1 regardless of + // `T`'s alignment. Thus, an aligned pointer to `Unalign` is not + // necessarily an aligned pointer to `T`. type AlignmentVariance = Invariant; - // SAFETY: `Unalign` is a `#[repr(C, packed)]` type wrapping a single `T` - // field, and so has the same validity as `T`. + // SAFETY: `Unalign` promises to have the same validity as `T`. type ValidityVariance = Covariant; fn cast_into_inner(ptr: *mut Unalign) -> *mut T { diff --git a/src/wrappers.rs b/src/wrappers.rs index 763a3987c6..d670604bb9 100644 --- a/src/wrappers.rs +++ b/src/wrappers.rs @@ -39,6 +39,12 @@ use super::*; /// [`try_deref_mut`]: Unalign::try_deref_mut /// [`deref_unchecked`]: Unalign::deref_unchecked /// [`deref_mut_unchecked`]: Unalign::deref_mut_unchecked +/// +/// # Safety +/// +/// `Unalign` is guaranteed to have the same size and bit validity as `T`, +/// and to have [`UnsafeCell`]s covering the same byte ranges as `T`. +/// `Unalign` is guaranteed to have alignment 1. // NOTE: This type is sound to use with types that need to be dropped. The // reason is that the compiler-generated drop code automatically moves all // values to aligned memory slots before dropping them in-place. This is not @@ -63,8 +69,8 @@ impl_known_layout!(T => Unalign); safety_comment! { /// SAFETY: - /// - `Unalign` is `repr(packed)`, so it is unaligned regardless of the - /// alignment of `T`, and so we don't require that `T: Unaligned` + /// - `Unalign` promises to have alignment 1, and so we don't require + /// that `T: Unaligned`. /// - `Unalign` has the same bit validity as `T`, and so it is /// `FromZeros`, `FromBytes`, or `IntoBytes` exactly when `T` is as well. /// - `Immutable`: `Unalign` has the same fields as `T`, so it contains diff --git a/zerocopy-derive/src/lib.rs b/zerocopy-derive/src/lib.rs index 0155aa3bb4..c08ef8042f 100644 --- a/zerocopy-derive/src/lib.rs +++ b/zerocopy-derive/src/lib.rs @@ -395,11 +395,13 @@ fn derive_try_from_bytes_struct(ast: &DeriveInput, strct: &DataStruct) -> proc_m mut candidate: ::zerocopy::Maybe ) -> bool { true #(&& { - // SAFETY: `project` is a field projection of `candidate`, - // and `Self` is a struct type. The candidate will have - // `UnsafeCell`s at exactly the same ranges as its - // projection, because the projection is a field of the - // candidate struct. + // SAFETY: + // - `project` is a field projection, and so it addresses a + // subset of the bytes addressed by `slf` + // - ..., and so it preserves provenance + // - ..., and `*slf` is a struct, so `UnsafeCell`s exist at + // the same byte ranges in the returned pointer's referent + // as they do in `*slf` let field_candidate = unsafe { let project = |slf: *mut Self| ::zerocopy::macro_util::core_reexport::ptr::addr_of_mut!((*slf).#field_names); @@ -444,11 +446,13 @@ fn derive_try_from_bytes_union(ast: &DeriveInput, unn: &DataUnion) -> proc_macro mut candidate: ::zerocopy::Maybe ) -> bool { false #(|| { - // SAFETY: `project` is a field projection of `candidate`, - // and `Self` is a union type. The candidate and projection - // agree exactly on where their `UnsafeCell` ranges are, - // because `Self: Immutable` is enforced by - // `self_type_trait_bounds`. + // SAFETY: + // - `project` is a field projection, and so it addresses a + // subset of the bytes addressed by `slf` + // - ..., and so it preserves provenance + // - Since `Self: Immutable` is enforced by + // `self_type_trait_bounds`, neither `*slf` nor the + // returned pointer's referent contain any `UnsafeCell`s let field_candidate = unsafe { let project = |slf: *mut Self| ::zerocopy::macro_util::core_reexport::ptr::addr_of_mut!((*slf).#field_names); @@ -514,9 +518,11 @@ fn derive_try_from_bytes_enum(ast: &DeriveInput, enm: &DataEnum) -> proc_macro2: quote!( use ::zerocopy::macro_util::core_reexport; // SAFETY: - // - `cast` is implemented as required. - // - By definition, `*mut Self` and `*mut [u8; size_of::()]` - // are types of the same size. + // - The closure is a pointer cast, and `Self` and `[u8; + // size_of::()]` have the same size, so the returned pointer + // addresses the same bytes as `p` subset of the bytes addressed + // by `slf` + // - ..., and so it preserves provenance // - Since we validate that this type is a field-less enum, it // cannot contain any `UnsafeCell`s. Neither does `[u8; N]`. let discriminant = unsafe { candidate.cast_unsized(|p: *mut Self| p as *mut [core_reexport::primitive::u8; core_reexport::mem::size_of::()]) }; diff --git a/zerocopy-derive/tests/struct_try_from_bytes.rs b/zerocopy-derive/tests/struct_try_from_bytes.rs index 9423ef7a6f..0986fddc4c 100644 --- a/zerocopy-derive/tests/struct_try_from_bytes.rs +++ b/zerocopy-derive/tests/struct_try_from_bytes.rs @@ -74,11 +74,10 @@ fn two_bad() { let candidate = unsafe { candidate.assume_initialized() }; // SAFETY: - // - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as - // *mut U`. - // - The size of the object referenced by the resulting pointer is equal to - // the size of the object referenced by `self`. - // - `Two` does not contain any `UnsafeCell`s. + // - The cast preserves address and size. As a result, the cast will address + // the same bytes as `c`. + // - The cast preserves provenance. + // - Neither the input nor output types contain any `UnsafeCell`s. let candidate = unsafe { candidate.cast_unsized(|p| p as *mut Two) }; // SAFETY: `candidate`'s referent is as-initialized as `Two`. @@ -105,12 +104,10 @@ fn un_sized() { let candidate = unsafe { candidate.assume_initialized() }; // SAFETY: - // - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as - // *mut U`. - // - The size of the object referenced by the resulting pointer is equal to - // the size of the object referenced by `self`. - // - The alignment of `Unsized` is equal to the alignment of `[u8]`. - // - `Unsized` does not contain any `UnsafeCell`s. + // - The cast preserves address and size. As a result, the cast will address + // the same bytes as `c`. + // - The cast preserves provenance. + // - Neither the input nor output types contain any `UnsafeCell`s. let candidate = unsafe { candidate.cast_unsized(|p| p as *mut Unsized) }; // SAFETY: `candidate`'s referent is as-initialized as `Two`. diff --git a/zerocopy-derive/tests/union_try_from_bytes.rs b/zerocopy-derive/tests/union_try_from_bytes.rs index fd81f636ca..9355cbc7f5 100644 --- a/zerocopy-derive/tests/union_try_from_bytes.rs +++ b/zerocopy-derive/tests/union_try_from_bytes.rs @@ -69,11 +69,10 @@ fn two_bad() { let candidate = unsafe { candidate.assume_initialized() }; // SAFETY: - // - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as - // *mut U`. - // - The size of the object referenced by the resulting pointer is equal to - // the size of the object referenced by `self`. - // - `Two` does not contain any `UnsafeCell`s. + // - The cast preserves address and size. As a result, the cast will address + // the same bytes as `c`. + // - The cast preserves provenance. + // - Neither the input nor output types contain any `UnsafeCell`s. let candidate = unsafe { candidate.cast_unsized(|p| p as *mut Two) }; // SAFETY: `candidate`'s referent is as-initialized as `Two`. @@ -99,11 +98,10 @@ fn bool_and_zst() { let candidate = unsafe { candidate.assume_initialized() }; // SAFETY: - // - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as - // *mut U`. - // - The size of the object referenced by the resulting pointer is equal to - // the size of the object referenced by `self`. - // - `BoolAndZst` does not contain any `UnsafeCell`s. + // - The cast preserves address and size. As a result, the cast will address + // the same bytes as `c`. + // - The cast preserves provenance. + // - Neither the input nor output types contain any `UnsafeCell`s. let candidate = unsafe { candidate.cast_unsized(|p| p as *mut BoolAndZst) }; // SAFETY: `candidate`'s referent is fully initialized.