Remove .forget_exclusive from Ptr::try_into_valid (#1257)

This causes methods like `TryFromBytes::try_mut_from` to fail compilation during post-monomorphization when used with types containing `UnsafeCell`s. This wasn't caught because our tests hadn't been updated to exercise this condition. This commit also updates the tests so this can't slip through in the future.
google · May 14, 2024 · 6659998 · 6659998
1 parent 5b095e0
commit 6659998
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 33 deletions.
diff --git a/src/impls.rs b/src/impls.rs
@@ -1107,12 +1107,6 @@ mod tests {
                     &self,
                     bytes: &'bytes [u8],
                 ) -> Option<Option<&'bytes T>>;
-
-                #[allow(clippy::needless_lifetimes)]
-                fn test_try_from_mut<'bytes>(
-                    &self,
-                    bytes: &'bytes mut [u8],
-                ) -> Option<Option<&'bytes mut T>>;
             }
 
             impl<T: TryFromBytes + Immutable + KnownLayout + ?Sized> TestTryFromRef<T> for AutorefWrapper<T> {
@@ -1123,14 +1117,6 @@ mod tests {
                 ) -> Option<Option<&'bytes T>> {
                     Some(T::try_ref_from(bytes).ok())
                 }
-
-                #[allow(clippy::needless_lifetimes)]
-                fn test_try_from_mut<'bytes>(
-                    &self,
-                    bytes: &'bytes mut [u8],
-                ) -> Option<Option<&'bytes mut T>> {
-                    Some(T::try_mut_from(bytes).ok())
-                }
             }
 
             pub(super) trait TestTryReadFrom<T> {
@@ -1224,16 +1210,6 @@ mod tests {
                         None
                     }
 
-                    #[allow(clippy::needless_lifetimes)]
-                    fn test_try_from_mut<'bytes>(&mut self, _bytes: &'bytes mut [u8]) -> Option<Option<&'bytes mut $ty>> {
-                        assert_on_allowlist!(
-                            test_try_from_mut($ty):
-                            ManuallyDrop<[UnsafeCell<bool>]>
-                        );
-
-                        None
-                    }
-
                     fn test_try_read_from(&mut self, _bytes: &[u8]) -> Option<Option<&$ty>> {
                         assert_on_allowlist!(
                             test_try_read_from($ty):
@@ -1342,10 +1318,8 @@ mod tests {
                         let bytes_mut = &mut vec.as_mut_slice()[offset..offset+size];
                         bytes_mut.copy_from_slice(bytes);
 
-                        let res = ww.test_try_from_mut(bytes_mut);
-                        if let Some(res) = res {
-                            assert!(res.is_some(), "{}::try_mut_from({:?}): got `None`, expected `Some`", stringify!($ty), val);
-                        }
+                        let res = <$ty as TryFromBytes>::try_mut_from(bytes_mut);
+                        assert!(res.is_ok(), "{}::try_mut_from({:?}): got `Err`, expected `Ok`", stringify!($ty), val);
                     }
 
                     let res = bytes.and_then(|bytes| ww.test_try_read_from(bytes));
@@ -1365,10 +1339,8 @@ mod tests {
                         assert!(res.is_none(), "{}::try_ref_from({:?}): got Some, expected None", stringify!($ty), c);
                     }
 
-                    let res = w.test_try_from_mut(c);
-                    if let Some(res) = res {
-                        assert!(res.is_none(), "{}::try_mut_from({:?}): got Some, expected None", stringify!($ty), c);
-                    }
+                    let res = <$ty as TryFromBytes>::try_mut_from(c);
+                    assert!(res.is_err(), "{}::try_mut_from({:?}): got Ok, expected Err", stringify!($ty), c);
 
                     let res = w.test_try_read_from(c);
                     if let Some(res) = res {

diff --git a/src/pointer/ptr.rs b/src/pointer/ptr.rs
@@ -874,7 +874,7 @@ 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_exclusive().forget_aligned()) {
+            if T::is_bit_valid(self.reborrow().forget_aligned()) {
                 // SAFETY: If `T::is_bit_valid`, code may assume that `self`
                 // contains a bit-valid instance of `Self`.
                 Ok(unsafe { self.assume_valid() })