Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace mem::forget with ManuallyDrop::new #3515

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Replace mem::forget with ManuallyDrop::new #3515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53b410b
Replace `mem::forget` with `ManuallyDrop::new`
danielhenrymantilla-codespace Jun 8, 2023
12026a5
::core
Manishearth Jun 8, 2023
4df8c11
fix assertion
Manishearth Jun 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions utils/zerovec/src/ule/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,11 @@ pub fn encode_varule_to_box<S: EncodeAsVarULE<T>, T: VarULE + ?Sized>(x: &S) ->
// zero-fill the vector to avoid uninitialized data UB
vec.resize(x.encode_var_ule_len(), 0);
x.encode_var_ule_write(&mut vec);
let boxed = vec.into_boxed_slice();
let boxed = mem::ManuallyDrop::new(vec.into_boxed_slice());
unsafe {
// Safety: `ptr` is a box, and `T` is a VarULE which guarantees it has the same memory layout as `[u8]`
// and can be recouped via from_byte_slice_unchecked()
let ptr: *mut T = T::from_byte_slice_unchecked(&boxed) as *const T as *mut T;
mem::forget(boxed);

// Safety: we can construct an owned version since we have mem::forgotten the older owner
Box::from_raw(ptr)
Expand Down
5 changes: 2 additions & 3 deletions utils/zerovec/src/ule/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,12 @@ pub unsafe trait VarULE: 'static {
#[inline]
fn to_boxed(&self) -> Box<Self> {
let bytesvec = self.as_byte_slice().to_owned().into_boxed_slice();
let bytesvec = mem::ManuallyDrop::new(bytesvec);
unsafe {
// Get the pointer representation
let ptr: *mut Self =
Self::from_byte_slice_unchecked(&bytesvec) as *const Self as *mut Self;
assert_eq!(Layout::for_value(&*ptr), Layout::for_value(&*bytesvec));
// Forget the allocation
mem::forget(bytesvec);
assert_eq!(Layout::for_value(&*ptr), Layout::for_value(&**bytesvec));
// Transmute the pointer to an owned pointer
Box::from_raw(ptr)
}
Expand Down
44 changes: 22 additions & 22 deletions utils/zerovec/src/yoke_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ unsafe impl<'a, T: 'static + AsULE + ?Sized> Yokeable<'a> for ZeroVec<'static, T
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand All @@ -61,8 +61,8 @@ unsafe impl<'a, T: 'static + VarULE + ?Sized> Yokeable<'a> for VarZeroVec<'stati
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand All @@ -89,8 +89,8 @@ unsafe impl<'a> Yokeable<'a> for FlexZeroVec<'static> {
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand Down Expand Up @@ -127,16 +127,16 @@ where
unsafe {
// Similar problem as transform(), but we need to use ptr::read since
// the compiler isn't sure of the sizes
let ptr: *const Self::Output = (&self as *const Self).cast();
mem::forget(self);
let this = mem::ManuallyDrop::new(self);
let ptr: *const Self::Output = (&*this as *const Self).cast();
ptr::read(ptr)
}
}
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand Down Expand Up @@ -173,16 +173,16 @@ where
unsafe {
// Similar problem as transform(), but we need to use ptr::read since
// the compiler isn't sure of the sizes
let ptr: *const Self::Output = (&self as *const Self).cast();
mem::forget(self);
let this = mem::ManuallyDrop::new(self);
let ptr: *const Self::Output = (&*this as *const Self).cast();
ptr::read(ptr)
}
}
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand Down Expand Up @@ -221,16 +221,16 @@ where
unsafe {
// Similar problem as transform(), but we need to use ptr::read since
// the compiler isn't sure of the sizes
let ptr: *const Self::Output = (&self as *const Self).cast();
mem::forget(self);
let this = mem::ManuallyDrop::new(self);
let ptr: *const Self::Output = (&*this as *const Self).cast();
ptr::read(ptr)
}
}
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
robertbastian marked this conversation as resolved.
Show resolved Hide resolved
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand Down Expand Up @@ -269,16 +269,16 @@ where
unsafe {
// Similar problem as transform(), but we need to use ptr::read since
// the compiler isn't sure of the sizes
let ptr: *const Self::Output = (&self as *const Self).cast();
mem::forget(self);
let this = mem::ManuallyDrop::new(self);
let ptr: *const Self::Output = (&*this as *const Self).cast();
ptr::read(ptr)
}
}
#[inline]
unsafe fn make(from: Self::Output) -> Self {
debug_assert!(mem::size_of::<Self::Output>() == mem::size_of::<Self>());
let ptr: *const Self = (&from as *const Self::Output).cast();
mem::forget(from);
let from = mem::ManuallyDrop::new(from);
let ptr: *const Self = (&*from as *const Self::Output).cast();
ptr::read(ptr)
}
#[inline]
Expand Down
22 changes: 10 additions & 12 deletions utils/zerovec/src/zerovec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use core::marker::PhantomData;
use core::mem;
use core::num::NonZeroUsize;
use core::ops::Deref;
use core::ptr;

/// A zero-copy, byte-aligned vector for fixed-width types.
///
Expand Down Expand Up @@ -286,14 +287,14 @@ where
/// If you have a slice of `&[T]`s, prefer using
/// [`Self::alloc_from_slice()`].
#[inline]
pub fn new_owned(mut vec: Vec<T::ULE>) -> Self {
pub fn new_owned(vec: Vec<T::ULE>) -> Self {
// Deconstruct the vector into parts
// This is the only part of the code that goes from Vec
// to ZeroVec, all other such operations should use this function
let slice: &mut [T::ULE] = &mut vec;
let slice = slice as *mut [_];
let capacity = vec.capacity();
mem::forget(vec);
let len = vec.len();
let ptr = mem::ManuallyDrop::new(vec).as_mut_ptr();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this is interesting, I was trying to figure out how best to fix the problem here

let slice = ptr::slice_from_raw_parts_mut(ptr, len);
Self {
vector: EyepatchHackVector {
buf: slice,
Expand Down Expand Up @@ -894,21 +895,18 @@ where
/// the logical equivalent of this type's internal representation
#[inline]
pub fn into_cow(self) -> Cow<'a, [T::ULE]> {
if self.is_owned() {
let this = mem::ManuallyDrop::new(self);
if this.is_owned() {
let vec = unsafe {
// safe to call: we know it's owned,
// and we mem::forget self immediately afterwards
self.vector.get_vec()
// and `self`/`this` are thenceforth no longer used or dropped
{ this }.vector.get_vec()
};
mem::forget(self);
Cow::Owned(vec)
} else {
// We can extend the lifetime of the slice to 'a
// since we know it is borrowed
let slice = unsafe { self.vector.as_arbitrary_slice() };
// The borrowed destructor is a no-op, but we want to prevent
// the check being run
mem::forget(self);
let slice = unsafe { { this }.vector.as_arbitrary_slice() };
Cow::Borrowed(slice)
}
}
Expand Down