From 57c1f4cf66ae2082a4be824bdde1aaac1b73c3ff Mon Sep 17 00:00:00 2001 From: Emil Hofstetter Date: Thu, 1 Dec 2022 20:54:24 -0500 Subject: [PATCH 1/3] change ClearOnDrop to hold MaybeUninit

--- src/clear.rs | 8 +++- src/clear_on_drop.rs | 107 +++++++++++++++++++++++++++++++++---------- 2 files changed, 88 insertions(+), 27 deletions(-) diff --git a/src/clear.rs b/src/clear.rs index 5c3f70c..7056e19 100644 --- a/src/clear.rs +++ b/src/clear.rs @@ -65,7 +65,11 @@ where let ptr = self as *mut Self; ptr::drop_in_place(ptr); ptr::write_bytes(ptr as *mut u8, 0, size); - hide_mem_impl::(ptr); + + if !cfg!(miri) { + hide_mem_impl::(ptr); + } + Self::initialize(ptr); } } @@ -135,7 +139,7 @@ macro_rules! array_impl_zerosafe { } // Implement for fixed-size arrays of ZeroSafe up to 64 -array_impl_zerosafe!{ +array_impl_zerosafe! { 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 diff --git a/src/clear_on_drop.rs b/src/clear_on_drop.rs index 83ef2f7..4abc90b 100644 --- a/src/clear_on_drop.rs +++ b/src/clear_on_drop.rs @@ -1,10 +1,10 @@ use core::borrow::{Borrow, BorrowMut}; use core::cmp::Ordering; -use core::fmt; use core::hash::{Hash, Hasher}; -use core::mem; +use core::mem::{size_of_val, MaybeUninit}; use core::ops::{Deref, DerefMut}; use core::ptr; +use core::{fmt, mem}; use crate::clear::Clear; @@ -37,7 +37,7 @@ where P: DerefMut, P::Target: Clear, { - _place: P, + _place: MaybeUninit

, } impl

ClearOnDrop

@@ -56,7 +56,9 @@ where /// back, use `ClearOnDrop::into_place(...)` instead of a borrow. #[inline] pub fn new(place: P) -> Self { - ClearOnDrop { _place: place } + ClearOnDrop { + _place: MaybeUninit::new(place), + } } /// Consumes the `ClearOnDrop`, returning the `place` after clearing. @@ -78,11 +80,11 @@ where /// `c.into_uncleared_place()`. This is so that there is no conflict /// with a method on the inner type. #[inline] - pub fn into_uncleared_place(c: Self) -> P { + pub fn into_uncleared_place(mut c: Self) -> P { unsafe { let place = ptr::read(&c._place); - mem::forget(c); - place + ptr::write(&mut c._place, MaybeUninit::zeroed()); + place.assume_init() } } } @@ -95,14 +97,19 @@ where #[inline] fn clone(&self) -> Self { ClearOnDrop { - _place: Clone::clone(&self._place), + _place: MaybeUninit::new(unsafe { Clone::clone(&self._place.assume_init_ref()) }), } } #[inline] fn clone_from(&mut self, source: &Self) { self.clear(); - Clone::clone_from(&mut self._place, &source._place) + unsafe { + Clone::clone_from( + &mut self._place.assume_init_ref(), + &source._place.assume_init_ref(), + ) + } } } @@ -126,7 +133,7 @@ where #[inline] fn deref(&self) -> &Self::Target { - Deref::deref(&self._place) + unsafe { Deref::deref(self._place.assume_init_ref()) } } } @@ -137,7 +144,7 @@ where { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { - DerefMut::deref_mut(&mut self._place) + unsafe { DerefMut::deref_mut(self._place.assume_init_mut()) } } } @@ -148,7 +155,15 @@ where { #[inline] fn drop(&mut self) { - self.clear(); + let ptr = self._place.as_ptr() as *mut u8; + unsafe { + if (0..mem::size_of::>() as isize) + .fold(0, |acc, i| acc + *ptr.offset(i) as i32) + != 0 + { + self.clear(); + } + } } } @@ -161,7 +176,7 @@ where { #[inline] fn as_ref(&self) -> &T { - AsRef::as_ref(&self._place) + unsafe { AsRef::as_ref(self._place.assume_init_ref()) } } } @@ -172,7 +187,7 @@ where { #[inline] fn as_mut(&mut self) -> &mut T { - AsMut::as_mut(&mut self._place) + unsafe { AsMut::as_mut(self._place.assume_init_mut()) } } } @@ -190,7 +205,7 @@ where { #[inline] fn borrow(&self) -> &T { - Borrow::borrow(&self._place) + unsafe { Borrow::borrow(self._place.assume_init_ref()) } } } @@ -202,7 +217,7 @@ where { #[inline] fn borrow_mut(&mut self) -> &mut T { - BorrowMut::borrow_mut(&mut self._place) + unsafe { BorrowMut::borrow_mut(self._place.assume_init_mut()) } } } @@ -215,7 +230,7 @@ where { #[inline] fn hash(&self, state: &mut H) { - Hash::hash(&self._place, state) + unsafe { Hash::hash(self._place.assume_init_ref(), state) } } } @@ -230,12 +245,22 @@ where { #[inline] fn eq(&self, other: &ClearOnDrop) -> bool { - PartialEq::eq(&self._place, &other._place) + unsafe { + PartialEq::eq( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } #[inline] fn ne(&self, other: &ClearOnDrop) -> bool { - PartialEq::ne(&self._place, &other._place) + unsafe { + PartialEq::ne( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } } @@ -255,27 +280,52 @@ where { #[inline] fn partial_cmp(&self, other: &ClearOnDrop) -> Option { - PartialOrd::partial_cmp(&self._place, &other._place) + unsafe { + PartialOrd::partial_cmp( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } #[inline] fn lt(&self, other: &ClearOnDrop) -> bool { - PartialOrd::lt(&self._place, &other._place) + unsafe { + PartialOrd::lt( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } #[inline] fn le(&self, other: &ClearOnDrop) -> bool { - PartialOrd::le(&self._place, &other._place) + unsafe { + PartialOrd::le( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } #[inline] fn gt(&self, other: &ClearOnDrop) -> bool { - PartialOrd::gt(&self._place, &other._place) + unsafe { + PartialOrd::gt( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } #[inline] fn ge(&self, other: &ClearOnDrop) -> bool { - PartialOrd::ge(&self._place, &other._place) + unsafe { + PartialOrd::ge( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } } @@ -286,7 +336,12 @@ where { #[inline] fn cmp(&self, other: &Self) -> Ordering { - Ord::cmp(&self._place, &other._place) + unsafe { + Ord::cmp( + &self._place.assume_init_ref(), + &other._place.assume_init_ref(), + ) + } } } @@ -320,6 +375,7 @@ mod tests { assert_eq!(clear.data, DATA); } + #[cfg(not(miri))] #[test] fn into_box() { let place: Box = Box::new(Default::default()); @@ -389,6 +445,7 @@ mod tests { assert_eq!(&clear[..], "test"); } + #[cfg(not(miri))] #[test] fn into_string() { let place: String = "test".into(); From c7140a024d457e0132c406cecf87104fe520bee8 Mon Sep 17 00:00:00 2001 From: Emil Hofstetter Date: Thu, 1 Dec 2022 21:47:15 -0500 Subject: [PATCH 2/3] change to ManuallyDrop --- src/clear_on_drop.rs | 97 +++++++++++++------------------------------- src/hide.rs | 1 + 2 files changed, 29 insertions(+), 69 deletions(-) diff --git a/src/clear_on_drop.rs b/src/clear_on_drop.rs index 4abc90b..afedc31 100644 --- a/src/clear_on_drop.rs +++ b/src/clear_on_drop.rs @@ -1,7 +1,7 @@ use core::borrow::{Borrow, BorrowMut}; use core::cmp::Ordering; use core::hash::{Hash, Hasher}; -use core::mem::{size_of_val, MaybeUninit}; +use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; use core::ptr; use core::{fmt, mem}; @@ -37,7 +37,7 @@ where P: DerefMut, P::Target: Clear, { - _place: MaybeUninit

, + _place: ManuallyDrop

, } impl

ClearOnDrop

@@ -57,7 +57,7 @@ where #[inline] pub fn new(place: P) -> Self { ClearOnDrop { - _place: MaybeUninit::new(place), + _place: ManuallyDrop::new(place), } } @@ -83,8 +83,12 @@ where pub fn into_uncleared_place(mut c: Self) -> P { unsafe { let place = ptr::read(&c._place); - ptr::write(&mut c._place, MaybeUninit::zeroed()); - place.assume_init() + ptr::write_bytes( + &mut c._place as *mut _ as *mut u8, + 0, + mem::size_of::>(), + ); + ManuallyDrop::into_inner(place) } } } @@ -97,19 +101,14 @@ where #[inline] fn clone(&self) -> Self { ClearOnDrop { - _place: MaybeUninit::new(unsafe { Clone::clone(&self._place.assume_init_ref()) }), + _place: Clone::clone(&self._place), } } #[inline] fn clone_from(&mut self, source: &Self) { self.clear(); - unsafe { - Clone::clone_from( - &mut self._place.assume_init_ref(), - &source._place.assume_init_ref(), - ) - } + Clone::clone_from(&mut self._place, &source._place) } } @@ -133,7 +132,7 @@ where #[inline] fn deref(&self) -> &Self::Target { - unsafe { Deref::deref(self._place.assume_init_ref()) } + Deref::deref(&self._place as &P) } } @@ -144,7 +143,7 @@ where { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { - unsafe { DerefMut::deref_mut(self._place.assume_init_mut()) } + DerefMut::deref_mut(&mut self._place as &mut P) } } @@ -155,9 +154,9 @@ where { #[inline] fn drop(&mut self) { - let ptr = self._place.as_ptr() as *mut u8; + let ptr = &mut self._place as *mut _ as *mut u8; unsafe { - if (0..mem::size_of::>() as isize) + if (0..mem::size_of::>() as isize) .fold(0, |acc, i| acc + *ptr.offset(i) as i32) != 0 { @@ -176,7 +175,7 @@ where { #[inline] fn as_ref(&self) -> &T { - unsafe { AsRef::as_ref(self._place.assume_init_ref()) } + AsRef::as_ref(&self._place as &P) } } @@ -187,7 +186,7 @@ where { #[inline] fn as_mut(&mut self) -> &mut T { - unsafe { AsMut::as_mut(self._place.assume_init_mut()) } + AsMut::as_mut(&mut self._place as &mut P) } } @@ -205,7 +204,7 @@ where { #[inline] fn borrow(&self) -> &T { - unsafe { Borrow::borrow(self._place.assume_init_ref()) } + Borrow::borrow(&self._place as &P) } } @@ -217,7 +216,7 @@ where { #[inline] fn borrow_mut(&mut self) -> &mut T { - unsafe { BorrowMut::borrow_mut(self._place.assume_init_mut()) } + BorrowMut::borrow_mut(&mut self._place as &mut P) } } @@ -230,7 +229,7 @@ where { #[inline] fn hash(&self, state: &mut H) { - unsafe { Hash::hash(self._place.assume_init_ref(), state) } + Hash::hash(&self._place as &P, state) } } @@ -245,22 +244,12 @@ where { #[inline] fn eq(&self, other: &ClearOnDrop) -> bool { - unsafe { - PartialEq::eq( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialEq::eq(&self._place as &P, &other._place as &Q) } #[inline] fn ne(&self, other: &ClearOnDrop) -> bool { - unsafe { - PartialEq::ne( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialEq::ne(&self._place as &P, &other._place as &Q) } } @@ -280,52 +269,27 @@ where { #[inline] fn partial_cmp(&self, other: &ClearOnDrop) -> Option { - unsafe { - PartialOrd::partial_cmp( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialOrd::partial_cmp(&self._place as &P, &other._place as &Q) } #[inline] fn lt(&self, other: &ClearOnDrop) -> bool { - unsafe { - PartialOrd::lt( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialOrd::lt(&self._place as &P, &other._place as &Q) } #[inline] fn le(&self, other: &ClearOnDrop) -> bool { - unsafe { - PartialOrd::le( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialOrd::le(&self._place as &P, &other._place as &Q) } #[inline] fn gt(&self, other: &ClearOnDrop) -> bool { - unsafe { - PartialOrd::gt( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialOrd::gt(&self._place as &P, &other._place as &Q) } #[inline] fn ge(&self, other: &ClearOnDrop) -> bool { - unsafe { - PartialOrd::ge( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + PartialOrd::ge(&self._place as &P, &other._place as &Q) } } @@ -336,12 +300,7 @@ where { #[inline] fn cmp(&self, other: &Self) -> Ordering { - unsafe { - Ord::cmp( - &self._place.assume_init_ref(), - &other._place.assume_init_ref(), - ) - } + Ord::cmp(&self._place as &P, &other._place as &P) } } diff --git a/src/hide.rs b/src/hide.rs index 91dca40..0e3bf25 100644 --- a/src/hide.rs +++ b/src/hide.rs @@ -88,6 +88,7 @@ mod impls { } #[cfg(test)] +#[cfg(not(miri))] mod tests { struct Place { data: [u32; 4], From ae2f17cf52d0e06ac925cb0406ff94fa5ed16e05 Mon Sep 17 00:00:00 2001 From: Emil Hofstetter Date: Thu, 1 Dec 2022 22:02:00 -0500 Subject: [PATCH 3/3] manually drop in drop --- src/clear_on_drop.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/clear_on_drop.rs b/src/clear_on_drop.rs index afedc31..54f69fc 100644 --- a/src/clear_on_drop.rs +++ b/src/clear_on_drop.rs @@ -161,6 +161,7 @@ where != 0 { self.clear(); + ManuallyDrop::drop(&mut self._place); } } }