Skip to content

Commit

Permalink
Remove unsound Ptr::forget_valid
Browse files Browse the repository at this point in the history 
With the addition of `Initialized`, validity is no longer totally
ordered.
  • Loading branch information
@jswrenn
jswrenn committed Feb 17, 2024
1 parent f4995df commit d489c56
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 21 deletions.
7 changes: 6 additions & 1 deletion src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8059,7 +8059,12 @@ mod tests {
macro_rules! assert_impls {
($ty:ty: TryFromBytes) => {
<$ty as TryFromBytesTestable>::with_passing_test_cases(|val| {
let c = Ptr::from_ref(val).forget_valid().forget_aligned();
let c = Ptr::from_ref(val).forget_aligned();
let c = c.forget_aligned();
// SAFETY: This is unsound. `$ty` is not necessarily
// `IntoBytes`, but that's the corner we've backed ourselves
// into by using `Ptr::from_ref`.
let c = unsafe { c.assume_initialized() };
let res = <$ty as TryFromBytes>::is_bit_valid(c);
assert!(res, "{}::is_bit_valid({:?}): got false, expected true", stringify!($ty), val);

Expand Down
10 changes: 0 additions & 10 deletions src/pointer/ptr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,16 +612,6 @@ mod _transitions {
// SAFETY: `AnyAlignment` is less restrictive than `Aligned`.
unsafe { Ptr::from_ptr(self) }
}

/// Forgets that `Ptr`'s referent is bit-valid for `T`.
#[doc(hidden)]
#[inline]
pub fn forget_valid(
self,
) -> Ptr<'a, T, (I::Aliasing, I::Alignment, invariant::Initialized)> {
// SAFETY: `AnyValidity` is less restrictive than `Valid`.
unsafe { Ptr::from_ptr(self) }
}
}
}

Expand Down
20 changes: 15 additions & 5 deletions zerocopy-derive/tests/struct_try_from_bytes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ include!("include.rs");
fn zst() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&());
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&()` trivially consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };
let is_bit_valid = <() as imp::TryFromBytes>::is_bit_valid(candidate);
imp::assert!(is_bit_valid);
}
Expand All @@ -36,7 +38,9 @@ util_assert_impl_all!(One: imp::TryFromBytes);
fn one() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&One { a: 42 });
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&One` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };
let is_bit_valid = <One as imp::TryFromBytes>::is_bit_valid(candidate);
imp::assert!(is_bit_valid);
}
Expand All @@ -54,7 +58,9 @@ util_assert_impl_all!(Two: imp::TryFromBytes);
fn two() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&Two { a: false, b: () });
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&Two` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };
let is_bit_valid = <Two as imp::TryFromBytes>::is_bit_valid(candidate);
imp::assert!(is_bit_valid);
}
Expand All @@ -63,7 +69,9 @@ fn two() {
fn two_bad() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&[2u8][..]);
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&Two` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };

// SAFETY:
// - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as
Expand Down Expand Up @@ -91,7 +99,9 @@ util_assert_impl_all!(Unsized: imp::TryFromBytes);
fn un_sized() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&[16, 12, 42][..]);
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&Unsized` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };

// SAFETY:
// - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as
Expand Down
20 changes: 15 additions & 5 deletions zerocopy-derive/tests/union_try_from_bytes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ util_assert_impl_all!(One: imp::TryFromBytes);
fn one() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&One { a: 42 });
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&One` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };
let is_bit_valid = <One as imp::TryFromBytes>::is_bit_valid(candidate);
assert!(is_bit_valid);
}
Expand All @@ -44,12 +46,16 @@ util_assert_impl_all!(Two: imp::TryFromBytes);
fn two() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate_a = ::zerocopy::Ptr::from_ref(&Two { a: false });
let candidate_a = candidate_a.forget_aligned().forget_valid();
let candidate_a = candidate_a.forget_aligned();
// SAFETY: `&Two` consists entirely of initialized bytes.
let candidate_a = unsafe { candidate_a.assume_initialized() };
let is_bit_valid = <Two as imp::TryFromBytes>::is_bit_valid(candidate_a);
assert!(is_bit_valid);

let candidate_b = ::zerocopy::Ptr::from_ref(&Two { b: true });
let candidate_b = candidate_b.forget_aligned().forget_valid();
let candidate_b = candidate_b.forget_aligned();
// SAFETY: `&Two` consists entirely of initialized bytes.
let candidate_b = unsafe { candidate_b.assume_initialized() };
let is_bit_valid = <Two as imp::TryFromBytes>::is_bit_valid(candidate_b);
assert!(is_bit_valid);
}
Expand All @@ -58,7 +64,9 @@ fn two() {
fn two_bad() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&[2u8][..]);
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&[u8]` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };

// SAFETY:
// - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as
Expand All @@ -85,7 +93,9 @@ union BoolAndZst {
fn bool_and_zst() {
// TODO(#5): Use `try_transmute` in this test once it's available.
let candidate = ::zerocopy::Ptr::from_ref(&[2u8][..]);
let candidate = candidate.forget_aligned().forget_valid();
let candidate = candidate.forget_aligned();
// SAFETY: `&[u8]` consists entirely of initialized bytes.
let candidate = unsafe { candidate.assume_initialized() };

// SAFETY:
// - The cast `cast(p)` is implemented exactly as follows: `|p: *mut T| p as
Expand Down

0 comments on commit d489c56

Please sign in to comment.