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.

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 some mem::forget's with ManuallyDrop #127733

Merged
merged 1 commit into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 22 additions & 25 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ use core::intrinsics::abort;
#[cfg(not(no_global_oom_handling))]
use core::iter;
use core::marker::{PhantomData, Unsize};
use core::mem::{self, align_of_val_raw, forget, ManuallyDrop};
use core::mem::{self, align_of_val_raw, ManuallyDrop};
use core::ops::{CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn, Receiver};
use core::panic::{RefUnwindSafe, UnwindSafe};
#[cfg(not(no_global_oom_handling))]
Expand Down Expand Up @@ -908,19 +908,18 @@ impl<T, A: Allocator> Rc<T, A> {
#[stable(feature = "rc_unique", since = "1.4.0")]
pub fn try_unwrap(this: Self) -> Result<T, Self> {
if Rc::strong_count(&this) == 1 {
unsafe {
let val = ptr::read(&*this); // copy the contained object
let alloc = ptr::read(&this.alloc); // copy the allocator

// Indicate to Weaks that they can't be promoted by decrementing
// the strong count, and then remove the implicit "strong weak"
// pointer while also handling drop logic by just crafting a
// fake Weak.
this.inner().dec_strong();
let _weak = Weak { ptr: this.ptr, alloc };
forget(this);
Ok(val)
}
let this = ManuallyDrop::new(this);

let val: T = unsafe { ptr::read(&**this) }; // copy the contained object
let alloc: A = unsafe { ptr::read(&this.alloc) }; // copy the allocator

// Indicate to Weaks that they can't be promoted by decrementing
// the strong count, and then remove the implicit "strong weak"
// pointer while also handling drop logic by just crafting a
// fake Weak.
this.inner().dec_strong();
let _weak = Weak { ptr: this.ptr, alloc };
Ok(val)
} else {
Err(this)
}
Expand Down Expand Up @@ -1354,9 +1353,8 @@ impl<T: ?Sized, A: Allocator> Rc<T, A> {
#[stable(feature = "rc_raw", since = "1.17.0")]
#[rustc_never_returns_null_ptr]
pub fn into_raw(this: Self) -> *const T {
let ptr = Self::as_ptr(&this);
mem::forget(this);
ptr
let this = ManuallyDrop::new(this);
Self::as_ptr(&*this)
}

/// Consumes the `Rc`, returning the wrapped pointer and allocator.
Expand Down Expand Up @@ -2127,7 +2125,7 @@ impl<T> Rc<[T]> {
}

// All clear. Forget the guard so it doesn't free the new RcBox.
forget(guard);
mem::forget(guard);

Self::from_ptr(ptr)
}
Expand Down Expand Up @@ -3080,9 +3078,7 @@ impl<T: ?Sized, A: Allocator> Weak<T, A> {
#[must_use = "losing the pointer will leak memory"]
#[stable(feature = "weak_into_raw", since = "1.45.0")]
pub fn into_raw(self) -> *const T {
let result = self.as_ptr();
mem::forget(self);
result
mem::ManuallyDrop::new(self).as_ptr()
}

/// Consumes the `Weak<T>`, returning the wrapped pointer and allocator.
Expand Down Expand Up @@ -3762,10 +3758,11 @@ impl<T: ?Sized, A: Allocator> UniqueRcUninit<T, A> {
/// # Safety
///
/// The data must have been initialized (by writing to [`Self::data_ptr()`]).
unsafe fn into_rc(mut self) -> Rc<T, A> {
let ptr = self.ptr;
let alloc = self.alloc.take().unwrap();
mem::forget(self);
unsafe fn into_rc(self) -> Rc<T, A> {
let mut this = ManuallyDrop::new(self);
let ptr = this.ptr;
let alloc = this.alloc.take().unwrap();

// SAFETY: The pointer is valid as per `UniqueRcUninit::new`, and the caller is responsible
// for having initialized the data.
unsafe { Rc::from_ptr_in(ptr.as_ptr(), alloc) }
Expand Down
36 changes: 16 additions & 20 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use core::intrinsics::abort;
#[cfg(not(no_global_oom_handling))]
use core::iter;
use core::marker::{PhantomData, Unsize};
use core::mem::{self, align_of_val_raw};
use core::mem::{self, align_of_val_raw, ManuallyDrop};
use core::ops::{CoerceUnsized, Deref, DerefPure, DispatchFromDyn, Receiver};
use core::panic::{RefUnwindSafe, UnwindSafe};
use core::pin::Pin;
Expand Down Expand Up @@ -960,16 +960,14 @@ impl<T, A: Allocator> Arc<T, A> {

acquire!(this.inner().strong);

unsafe {
let elem = ptr::read(&this.ptr.as_ref().data);
let alloc = ptr::read(&this.alloc); // copy the allocator
let this = ManuallyDrop::new(this);
let elem: T = unsafe { ptr::read(&this.ptr.as_ref().data) };
let alloc: A = unsafe { ptr::read(&this.alloc) }; // copy the allocator

// Make a weak pointer to clean up the implicit strong-weak reference
let _weak = Weak { ptr: this.ptr, alloc };
mem::forget(this);
// Make a weak pointer to clean up the implicit strong-weak reference
let _weak = Weak { ptr: this.ptr, alloc };

Ok(elem)
}
Ok(elem)
}

/// Returns the inner value, if the `Arc` has exactly one strong reference.
Expand Down Expand Up @@ -1493,9 +1491,8 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
#[stable(feature = "rc_raw", since = "1.17.0")]
#[rustc_never_returns_null_ptr]
pub fn into_raw(this: Self) -> *const T {
let ptr = Self::as_ptr(&this);
mem::forget(this);
ptr
let this = ManuallyDrop::new(this);
Self::as_ptr(&*this)
}

/// Consumes the `Arc`, returning the wrapped pointer and allocator.
Expand Down Expand Up @@ -2801,9 +2798,7 @@ impl<T: ?Sized, A: Allocator> Weak<T, A> {
#[must_use = "losing the pointer will leak memory"]
#[stable(feature = "weak_into_raw", since = "1.45.0")]
pub fn into_raw(self) -> *const T {
let result = self.as_ptr();
mem::forget(self);
result
ManuallyDrop::new(self).as_ptr()
}

/// Consumes the `Weak<T>`, returning the wrapped pointer and allocator.
Expand Down Expand Up @@ -3875,13 +3870,14 @@ impl<T: ?Sized, A: Allocator> UniqueArcUninit<T, A> {
/// # Safety
///
/// The data must have been initialized (by writing to [`Self::data_ptr()`]).
unsafe fn into_arc(mut self) -> Arc<T, A> {
let ptr = self.ptr;
let alloc = self.alloc.take().unwrap();
mem::forget(self);
unsafe fn into_arc(self) -> Arc<T, A> {
let mut this = ManuallyDrop::new(self);
let ptr = this.ptr.as_ptr();
let alloc = this.alloc.take().unwrap();

// SAFETY: The pointer is valid as per `UniqueArcUninit::new`, and the caller is responsible
// for having initialized the data.
unsafe { Arc::from_ptr_in(ptr.as_ptr(), alloc) }
unsafe { Arc::from_ptr_in(ptr, alloc) }
}
}

Expand Down
15 changes: 5 additions & 10 deletions library/core/src/task/wake.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
#![stable(feature = "futures_api", since = "1.36.0")]

use crate::mem::transmute;

use crate::any::Any;
use crate::fmt;
use crate::marker::PhantomData;
use crate::mem::{transmute, ManuallyDrop};
use crate::panic::AssertUnwindSafe;
use crate::ptr;

Expand Down Expand Up @@ -465,16 +464,14 @@ impl Waker {
pub fn wake(self) {
// The actual wakeup call is delegated through a virtual function call
// to the implementation which is defined by the executor.
let wake = self.waker.vtable.wake;
let data = self.waker.data;

// Don't call `drop` -- the waker will be consumed by `wake`.
crate::mem::forget(self);
let this = ManuallyDrop::new(self);

// SAFETY: This is safe because `Waker::from_raw` is the only way
// to initialize `wake` and `data` requiring the user to acknowledge
// that the contract of `RawWaker` is upheld.
unsafe { (wake)(data) };
unsafe { (this.waker.vtable.wake)(this.waker.data) };
}

/// Wake up the task associated with this `Waker` without consuming the `Waker`.
Expand Down Expand Up @@ -726,16 +723,14 @@ impl LocalWaker {
pub fn wake(self) {
// The actual wakeup call is delegated through a virtual function call
// to the implementation which is defined by the executor.
let wake = self.waker.vtable.wake;
let data = self.waker.data;

// Don't call `drop` -- the waker will be consumed by `wake`.
crate::mem::forget(self);
let this = ManuallyDrop::new(self);

// SAFETY: This is safe because `Waker::from_raw` is the only way
// to initialize `wake` and `data` requiring the user to acknowledge
// that the contract of `RawWaker` is upheld.
unsafe { (wake)(data) };
unsafe { (this.waker.vtable.wake)(this.waker.data) };
}

/// Wake up the task associated with this `LocalWaker` without consuming the `LocalWaker`.
Expand Down
11 changes: 5 additions & 6 deletions library/proc_macro/src/bridge/buffer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Buffer management for same-process client<->server communication.

use std::io::{self, Write};
use std::mem;
use std::mem::{self, ManuallyDrop};
use std::ops::{Deref, DerefMut};
use std::slice;

Expand Down Expand Up @@ -129,17 +129,16 @@ impl Drop for Buffer {
}

impl From<Vec<u8>> for Buffer {
fn from(mut v: Vec<u8>) -> Self {
fn from(v: Vec<u8>) -> Self {
let mut v = ManuallyDrop::new(v);
let (data, len, capacity) = (v.as_mut_ptr(), v.len(), v.capacity());
mem::forget(v);

// This utility function is nested in here because it can *only*
// be safely called on `Buffer`s created by *this* `proc_macro`.
fn to_vec(b: Buffer) -> Vec<u8> {
unsafe {
let Buffer { data, len, capacity, .. } = b;
mem::forget(b);
Vec::from_raw_parts(data, len, capacity)
let b = ManuallyDrop::new(b);
Vec::from_raw_parts(b.data, b.len, b.capacity)
}
}

Expand Down
4 changes: 1 addition & 3 deletions library/proc_macro/src/bridge/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ macro_rules! define_client_handles {

impl<S> Encode<S> for $oty {
fn encode(self, w: &mut Writer, s: &mut S) {
let handle = self.handle;
mem::forget(self);
handle.encode(w, s);
mem::ManuallyDrop::new(self).handle.encode(w, s);
}
}

Expand Down
6 changes: 2 additions & 4 deletions library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::fmt;
use crate::fs;
use crate::io;
use crate::marker::PhantomData;
use crate::mem::forget;
use crate::mem::ManuallyDrop;
#[cfg(not(any(target_arch = "wasm32", target_env = "sgx", target_os = "hermit")))]
use crate::sys::cvt;
use crate::sys_common::{AsInner, FromInner, IntoInner};
Expand Down Expand Up @@ -141,9 +141,7 @@ impl AsRawFd for OwnedFd {
impl IntoRawFd for OwnedFd {
#[inline]
fn into_raw_fd(self) -> RawFd {
let fd = self.fd;
forget(self);
fd
ManuallyDrop::new(self).fd
}
}

Expand Down
6 changes: 2 additions & 4 deletions library/std/src/os/solid/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@

use crate::fmt;
use crate::marker::PhantomData;
use crate::mem::forget;
use crate::mem::ManuallyDrop;
use crate::net;
use crate::sys;
use crate::sys_common::{self, AsInner, FromInner, IntoInner};
Expand Down Expand Up @@ -149,9 +149,7 @@ impl AsRawFd for OwnedFd {
impl IntoRawFd for OwnedFd {
#[inline]
fn into_raw_fd(self) -> RawFd {
let fd = self.fd;
forget(self);
fd
ManuallyDrop::new(self).fd
}
}

Expand Down
6 changes: 2 additions & 4 deletions library/std/src/os/windows/io/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::fmt;
use crate::fs;
use crate::io;
use crate::marker::PhantomData;
use crate::mem::{forget, ManuallyDrop};
use crate::mem::ManuallyDrop;
use crate::ptr;
use crate::sys;
use crate::sys::cvt;
Expand Down Expand Up @@ -319,9 +319,7 @@ impl AsRawHandle for OwnedHandle {
impl IntoRawHandle for OwnedHandle {
#[inline]
fn into_raw_handle(self) -> RawHandle {
let handle = self.handle;
forget(self);
handle
ManuallyDrop::new(self).handle
}
}

Expand Down
7 changes: 2 additions & 5 deletions library/std/src/os/windows/io/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use super::raw::{AsRawSocket, FromRawSocket, IntoRawSocket, RawSocket};
use crate::fmt;
use crate::io;
use crate::marker::PhantomData;
use crate::mem;
use crate::mem::forget;
use crate::mem::{self, ManuallyDrop};
use crate::sys;
#[cfg(not(target_vendor = "uwp"))]
use crate::sys::cvt;
Expand Down Expand Up @@ -191,9 +190,7 @@ impl AsRawSocket for OwnedSocket {
impl IntoRawSocket for OwnedSocket {
#[inline]
fn into_raw_socket(self) -> RawSocket {
let socket = self.socket;
forget(self);
socket
ManuallyDrop::new(self).socket
}
}

Expand Down
6 changes: 2 additions & 4 deletions library/std/src/sys/pal/hermit/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use super::hermit_abi;
use crate::ffi::CStr;
use crate::io;
use crate::mem;
use crate::mem::ManuallyDrop;
use crate::num::NonZero;
use crate::ptr;
use crate::time::Duration;
Expand Down Expand Up @@ -90,9 +90,7 @@ impl Thread {

#[inline]
pub fn into_id(self) -> Tid {
let id = self.tid;
mem::forget(self);
id
ManuallyDrop::new(self).tid
}
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/sgx/abi/tls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ impl Tls {
#[allow(unused)]
pub unsafe fn activate_persistent(self: Box<Self>) {
// FIXME: Needs safety information. See entry.S for `set_tls_ptr` definition.
unsafe { set_tls_ptr(core::ptr::addr_of!(*self) as _) };
mem::forget(self);
let ptr = Box::into_raw(self).cast_const().cast::<u8>();
unsafe { set_tls_ptr(ptr) };
}

unsafe fn current<'a>() -> &'a Tls {
Expand Down
7 changes: 3 additions & 4 deletions library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::cell::UnsafeCell;
use crate::cmp;
use crate::convert::TryInto;
use crate::intrinsics;
use crate::mem;
use crate::mem::{self, ManuallyDrop};
use crate::ops::{CoerceUnsized, Deref, DerefMut, Index, IndexMut};
use crate::ptr::{self, NonNull};
use crate::slice;
Expand Down Expand Up @@ -176,6 +176,7 @@ unsafe impl<T: UserSafeSized> UserSafe for [T] {
/// are used solely to indicate intent: a mutable reference is for writing to
/// user memory, an immutable reference for reading from user memory.
#[unstable(feature = "sgx_platform", issue = "56975")]
#[repr(transparent)]
pub struct UserRef<T: ?Sized>(UnsafeCell<T>);
/// An owned type in userspace memory. `User<T>` is equivalent to `Box<T>` in
/// enclave memory. Access to the memory is only allowed by copying to avoid
Expand Down Expand Up @@ -266,9 +267,7 @@ where
/// Converts this value into a raw pointer. The value will no longer be
/// automatically freed.
pub fn into_raw(self) -> *mut T {
let ret = self.0;
mem::forget(self);
ret.as_ptr() as _
ManuallyDrop::new(self).0.as_ptr() as _
}
}

Expand Down
Loading
Loading