From e5db6e60eece25ea94ceff51824be34889e0443f Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Tue, 30 Dec 2025 19:25:33 +0000 Subject: [PATCH] [WIP] gherrit-pr-id: G60971ace7acef9cd1c74f74f397cb7c48e5e3f73 --- src/impls.rs | 59 ++++++++++++++++--------------- src/lib.rs | 4 +-- src/pointer/ptr.rs | 8 ++++- src/util/macros.rs | 32 ++++++++++------- src/wrappers.rs | 12 +++++-- zerocopy-derive/src/lib.rs | 72 +++++++++++++++++++++++++------------- 6 files changed, 115 insertions(+), 72 deletions(-) diff --git a/src/impls.rs b/src/impls.rs index c7d77326a8..7692e6f220 100644 --- a/src/impls.rs +++ b/src/impls.rs @@ -863,29 +863,16 @@ unsafe impl TryFromBytes for UnsafeCell { } #[inline] - fn is_bit_valid(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 - // guarantee how our users are accessing or modifying the `UnsafeCell`). - // - // `is_bit_valid` is documented as panicking or failing to monomorphize - // if called with a shared-aliased pointer on a type containing an - // `UnsafeCell`. In practice, it will always be a monomorphization error. - // Since `is_bit_valid` is `#[doc(hidden)]` and only called directly - // from this crate, we only need to worry about our own code incorrectly - // calling `UnsafeCell::is_bit_valid`. The post-monomorphization error - // makes it easier to test that this is truly the case, and also means - // that if we make a mistake, it will cause downstream code to fail to - // compile, which will immediately surface the mistake and give us a - // chance to fix it quickly. - let c = candidate.into_exclusive_or_pme(); - - // SAFETY: Since `UnsafeCell` and `T` have the same layout and bit - // validity, `UnsafeCell` is bit-valid exactly when its wrapped `T` - // is. Thus, this is a sound implementation of - // `UnsafeCell::is_bit_valid`. - T::is_bit_valid(c.get_mut()) + #[inline] + fn is_bit_valid(candidate: Maybe<'_, Self>) -> bool { + let candidate = unsafe { + candidate.transmute_unchecked::< + crate::wrappers::ReadOnly, + _, + crate::pointer::cast::CastUnsized + >() + }; + T::is_bit_valid(candidate) } } @@ -1339,17 +1326,17 @@ mod tests { pub(super) trait TestIsBitValidShared { #[allow(clippy::needless_lifetimes)] - fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>( + fn test_is_bit_valid_shared<'ptr>( &self, - candidate: Maybe<'ptr, T, A>, + candidate: Maybe<'ptr, T>, ) -> Option; } impl TestIsBitValidShared for AutorefWrapper { #[allow(clippy::needless_lifetimes)] - fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>( + fn test_is_bit_valid_shared<'ptr>( &self, - candidate: Maybe<'ptr, T, A>, + candidate: Maybe<'ptr, T>, ) -> Option { Some(T::is_bit_valid(candidate)) } @@ -1455,9 +1442,9 @@ mod tests { #[allow(unused, non_local_definitions)] impl AutorefWrapper<$ty> { #[allow(clippy::needless_lifetimes)] - fn test_is_bit_valid_shared<'ptr, A: invariant::Reference>( + fn test_is_bit_valid_shared<'ptr>( &mut self, - candidate: Maybe<'ptr, $ty, A>, + candidate: Maybe<'ptr, $ty>, ) -> Option { assert_on_allowlist!( test_is_bit_valid_shared($ty): @@ -1564,6 +1551,13 @@ mod tests { // necessarily `IntoBytes`, but that's the corner we've // backed ourselves into by using `Ptr::from_ref`. let c = unsafe { c.assume_initialized() }; + let c = unsafe { + c.transmute_unchecked::< + crate::wrappers::ReadOnly<$ty>, + _, + crate::pointer::cast::CastUnsized + >() + }; let res = w.test_is_bit_valid_shared(c); if let Some(res) = res { assert!(res, "{}::is_bit_valid({:?}) (shared `Ptr`): got false, expected true", stringify!($ty), val); @@ -1575,6 +1569,13 @@ mod tests { // necessarily `IntoBytes`, but that's the corner we've // backed ourselves into by using `Ptr::from_ref`. let c = unsafe { c.assume_initialized() }; + let c = unsafe { + c.transmute_unchecked::< + crate::wrappers::ReadOnly<$ty>, + _, + crate::pointer::cast::CastUnsized + >().assume_aliasing::() + }; let res = <$ty as TryFromBytes>::is_bit_valid(c); assert!(res, "{}::is_bit_valid({:?}) (exclusive `Ptr`): got false, expected true", stringify!($ty), val); diff --git a/src/lib.rs b/src/lib.rs index 8a90b1d751..2e969c59a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1532,7 +1532,7 @@ pub use zerocopy_derive::TryFromBytes; not(no_zerocopy_diagnostic_on_unimplemented_1_78_0), diagnostic::on_unimplemented(note = "Consider adding `#[derive(TryFromBytes)]` to `{Self}`") )] -pub unsafe trait TryFromBytes { +pub unsafe trait TryFromBytes: KnownLayout { // The `Self: Sized` bound makes it so that `TryFromBytes` is still object // safe. #[doc(hidden)] @@ -1562,7 +1562,7 @@ pub unsafe trait TryFromBytes { /// [`UnsafeCell`]: core::cell::UnsafeCell /// [`Shared`]: invariant::Shared #[doc(hidden)] - fn is_bit_valid(candidate: Maybe<'_, Self, A>) -> bool; + fn is_bit_valid(candidate: Maybe<'_, Self>) -> bool; /// Attempts to interpret the given `source` as a `&Self`. /// diff --git a/src/pointer/ptr.rs b/src/pointer/ptr.rs index 21b733fc87..fc2cc7987b 100644 --- a/src/pointer/ptr.rs +++ b/src/pointer/ptr.rs @@ -833,7 +833,13 @@ mod _transitions { // 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 fix before returning. - if T::is_bit_valid(self.reborrow().forget_aligned()) { + let candidate = unsafe { + self.reborrow() + .forget_aligned() + .transmute_unchecked::, _, crate::pointer::cast::CastUnsized>() + .assume_aliasing::() + }; + if T::is_bit_valid(candidate) { // SAFETY: If `T::is_bit_valid`, code may assume that `self` // contains a bit-valid instance of `T`. By `T: // TryTransmuteFromPtr`, so diff --git a/src/util/macros.rs b/src/util/macros.rs index b2ff64bd17..c91e43f760 100644 --- a/src/util/macros.rs +++ b/src/util/macros.rs @@ -135,7 +135,7 @@ macro_rules! unsafe_impl { fn only_derive_is_allowed_to_implement_this_trait() {} #[inline] - fn is_bit_valid($candidate: Maybe<'_, Self, AA>) -> bool { + fn is_bit_valid($candidate: Maybe<'_, Self>) -> bool { $is_bit_valid } }; @@ -143,7 +143,7 @@ macro_rules! unsafe_impl { #[allow(clippy::missing_inline_in_public_items)] #[cfg_attr(all(coverage_nightly, __ZEROCOPY_INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS), coverage(off))] fn only_derive_is_allowed_to_implement_this_trait() {} - #[inline(always)] fn is_bit_valid(_: Maybe<'_, Self, AA>) -> bool { true } + #[inline(always)] fn is_bit_valid(_: Maybe<'_, Self>) -> bool { true } }; (@method $trait:ident) => { #[allow(clippy::missing_inline_in_public_items, dead_code)] @@ -218,12 +218,15 @@ macro_rules! impl_for_transmute_from { TryFromBytes for $ty:ty [UnsafeCell<$repr:ty>] ) => { #[inline] - fn is_bit_valid(candidate: Maybe<'_, Self, A>) -> bool { - let c: Maybe<'_, Self, crate::pointer::invariant::Exclusive> = candidate.into_exclusive_or_pme(); - let c: Maybe<'_, $repr, _> = c.transmute::<_, _, (_, (_, (BecauseExclusive, BecauseExclusive)))>(); - // SAFETY: This macro ensures that `$repr` and `Self` have the same - // size and bit validity. Thus, a bit-valid instance of `$repr` is - // also a bit-valid instance of `Self`. + #[inline] + fn is_bit_valid(candidate: Maybe<'_, Self>) -> bool { + let c = unsafe { + candidate.transmute_unchecked::< + crate::wrappers::ReadOnly<$repr>, + _, + crate::pointer::cast::CastUnsized + >() + }; <$repr as TryFromBytes>::is_bit_valid(c) } }; @@ -233,11 +236,14 @@ macro_rules! impl_for_transmute_from { TryFromBytes for $ty:ty [<$repr:ty>] ) => { #[inline] - fn is_bit_valid(candidate: $crate::Maybe<'_, Self, A>) -> bool { - // SAFETY: This macro ensures that `$repr` and `Self` have the same - // size and bit validity. Thus, a bit-valid instance of `$repr` is - // also a bit-valid instance of `Self`. - <$repr as TryFromBytes>::is_bit_valid(candidate.transmute()) + fn is_bit_valid(candidate: $crate::Maybe<'_, Self>) -> bool { + <$repr as TryFromBytes>::is_bit_valid(unsafe { + candidate.transmute_unchecked::< + crate::wrappers::ReadOnly<$repr>, + _, + crate::pointer::cast::CastUnsized + >() + }) } }; ( diff --git a/src/wrappers.rs b/src/wrappers.rs index b286b29991..abcd88a3af 100644 --- a/src/wrappers.rs +++ b/src/wrappers.rs @@ -148,8 +148,14 @@ const _: () = unsafe { impl_or_verify!(T => Unaligned for Unalign); impl_or_verify!(T: Immutable => Immutable for Unalign); impl_or_verify!( - T: TryFromBytes => TryFromBytes for Unalign; - |c| T::is_bit_valid(c.transmute()) + T: TryFromBytes + KnownLayout => TryFromBytes for Unalign; + |c| T::is_bit_valid(unsafe { + c.transmute_unchecked::< + crate::wrappers::ReadOnly, + _, + crate::pointer::cast::CastUnsized + >() + }) ); impl_or_verify!(T: FromZeros => FromZeros for Unalign); impl_or_verify!(T: FromBytes => FromBytes for Unalign); @@ -615,7 +621,7 @@ const _: () = unsafe { const _: () = unsafe { impl_or_verify!(T: ?Sized + Unaligned => Unaligned for ReadOnly); impl_or_verify!( - T: ?Sized + TryFromBytes => TryFromBytes for ReadOnly; + T: ?Sized + TryFromBytes + KnownLayout => TryFromBytes for ReadOnly; |c| T::is_bit_valid(c.transmute::<_, _, BecauseImmutable>()) ); impl_or_verify!(T: ?Sized + FromZeros => FromZeros for ReadOnly); diff --git a/zerocopy-derive/src/lib.rs b/zerocopy-derive/src/lib.rs index 83717340c8..76ade7cf65 100644 --- a/zerocopy-derive/src/lib.rs +++ b/zerocopy-derive/src/lib.rs @@ -879,21 +879,36 @@ fn derive_try_from_bytes_struct( // validity of a struct is just the composition of the bit // validities of its fields, so this is a sound implementation // of `is_bit_valid`. - fn is_bit_valid<___ZerocopyAliasing>( - mut candidate: #zerocopy_crate::Maybe, - ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool - where - ___ZerocopyAliasing: #zerocopy_crate::pointer::invariant::Reference, - { + fn is_bit_valid( + mut candidate: #zerocopy_crate::Maybe, + ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool { use #zerocopy_crate::util::macro_util::core_reexport; use #zerocopy_crate::pointer::PtrInner; + // SAFETY: `ReadOnly` has the same memory layout as `Self`. + let mut candidate = unsafe { + candidate.transmute_unchecked::< + Self, + _, + #zerocopy_crate::pointer::cast::CastUnsized + >() + }; + true #(&& { let field_candidate = candidate.reborrow().project::< _, { #zerocopy_crate::ident_id!(#field_names) } >(); + // SAFETY: `ReadOnly` has the same memory layout as `T`. + let field_candidate = unsafe { + field_candidate.transmute_unchecked::< + #zerocopy_crate::wrappers::ReadOnly<#field_tys>, + _, + #zerocopy_crate::pointer::cast::CastUnsized + >() + }; + <#field_tys as #zerocopy_crate::TryFromBytes>::is_bit_valid(field_candidate) })* } @@ -933,15 +948,21 @@ fn derive_try_from_bytes_union( // The bit validity of a union is not yet well defined in Rust, // but it is guaranteed to be no more strict than this // definition. See #696 for a more in-depth discussion. - fn is_bit_valid<___ZerocopyAliasing>( - mut candidate: #zerocopy_crate::Maybe<'_, Self,___ZerocopyAliasing> - ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool - where - ___ZerocopyAliasing: #zerocopy_crate::pointer::invariant::Reference, - { + fn is_bit_valid( + mut candidate: #zerocopy_crate::Maybe<'_, Self> + ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool { use #zerocopy_crate::util::macro_util::core_reexport; use #zerocopy_crate::pointer::PtrInner; + // SAFETY: `ReadOnly` has the same memory layout as `Self`. + let mut candidate = unsafe { + candidate.transmute_unchecked::< + Self, + _, + #zerocopy_crate::pointer::cast::CastUnsized + >() + }; + false #(|| { // SAFETY: // - Since `Self: Immutable` is enforced by @@ -959,6 +980,15 @@ fn derive_try_from_bytes_union( >() }; + // SAFETY: `ReadOnly` has the same memory layout as `T`. + let field_candidate = unsafe { + field_candidate.transmute_unchecked::< + #zerocopy_crate::wrappers::ReadOnly<#field_tys>, + _, + #zerocopy_crate::pointer::cast::CastUnsized + >() + }; + <#field_tys as #zerocopy_crate::TryFromBytes>::is_bit_valid(field_candidate) })* } @@ -1040,12 +1070,9 @@ fn try_gen_trivial_is_bit_valid( if matches!(top_level, Trait::FromBytes) && ast.generics.params.is_empty() { Some(quote!( // SAFETY: See inline. - fn is_bit_valid<___ZerocopyAliasing>( - _candidate: #zerocopy_crate::Maybe, - ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool - where - ___ZerocopyAliasing: #zerocopy_crate::pointer::invariant::Reference, - { + fn is_bit_valid( + _candidate: #zerocopy_crate::Maybe, + ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool { if false { fn assert_is_from_bytes() where @@ -1083,12 +1110,9 @@ unsafe fn gen_trivial_is_bit_valid_unchecked(zerocopy_crate: &Path) -> proc_macr quote!( // SAFETY: The caller of `gen_trivial_is_bit_valid_unchecked` has // promised that all initialized bit patterns are valid for `Self`. - fn is_bit_valid<___ZerocopyAliasing>( - _candidate: #zerocopy_crate::Maybe, - ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool - where - ___ZerocopyAliasing: #zerocopy_crate::pointer::invariant::Reference, - { + fn is_bit_valid( + _candidate: #zerocopy_crate::Maybe, + ) -> #zerocopy_crate::util::macro_util::core_reexport::primitive::bool { true } )