Skip to content

Commit

Permalink
Rollup merge of #127733 - GrigorenkoPV:don't-forget, r=Amanieu
Browse files Browse the repository at this point in the history
Replace some `mem::forget`'s with `ManuallyDrop`

              > but I would like to see a larger effort to replace all uses of `mem::forget`.

_Originally posted by `@saethlin` in #127584 (comment)

So,
r? `@saethlin`

Sorry, I have finished writing all of this before I got your response.
  • Loading branch information
matthiaskrgr authored Jul 24, 2024
2 parents ce523d6 + f6fe7e4 commit 34abb96
Show file tree
Hide file tree
Showing 20 changed files with 88 additions and 132 deletions.
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 @@ -148,9 +148,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 @@ -48,7 +48,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 @@ -148,9 +148,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

0 comments on commit 34abb96

Please sign in to comment.