Rollup merge of #71107 - vorner:weak-into-raw-dangling, r=Amanieu

Address concerns of weak-into-raw

This should address the standing concerns in https://github.com/rust-lang/rust/issues/60728#issuecomment-612525616.

I've still left the ability to create a new dangling pointer from `null`, as I feel like this is the natural behaviour to expect, but I'm fine removing that too. I've modified the documentation to allow the `as_ptr` or `into_ptr` to return whatever garbage in case of a dangling pointer. I've also removed the guarantee to be able to do `from_raw(as_ptr)` from the documentation (but it would still work right now).

I've renamed the method and added implementations for `Rc`/`Arc`.

I've also tried if I can just „enable“ unsized types. I believe the current interface is compatible with them. But the inner implementation will be a bit challenging ‒ I can't use the `data_offset` as is used by `Rc` or `Arc` because it AFAIK „touches“ (creates a reference to) the live value of `T` ‒ and in case of `Weak`, it might be completely bogus or already dead ‒ so that would be UB.

`./x.py test tidy` is completely mad on my own system all over the code base :-(. I'll just hope it goes through CI, or will fix as necessary.

Is it OK if I ask @Amanieu for review, as the concerns are from you?

~r @Amanieu
This commit is contained in:
Dylan DPC 2020-04-19 15:12:36 +02:00 committed by GitHub
commit 4d11c3fe30
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 80 additions and 54 deletions

View File

@ -569,9 +569,33 @@ impl<T: ?Sized> Rc<T> {
/// ```
#[stable(feature = "rc_raw", since = "1.17.0")]
pub fn into_raw(this: Self) -> *const T {
let ptr = Self::as_ptr(&this);
mem::forget(this);
ptr
}
/// Provides a raw pointer to the data.
///
/// The counts are not affected in any way and the `Rc` is not consumed. The pointer is valid
/// for as long there are strong counts in the `Rc`.
///
/// # Examples
///
/// ```
/// #![feature(weak_into_raw)]
///
/// use std::rc::Rc;
///
/// let x = Rc::new("hello".to_owned());
/// let y = Rc::clone(&x);
/// let x_ptr = Rc::as_ptr(&x);
/// assert_eq!(x_ptr, Rc::as_ptr(&y));
/// assert_eq!(unsafe { &*x_ptr }, "hello");
/// ```
#[unstable(feature = "weak_into_raw", issue = "60728")]
pub fn as_ptr(this: &Self) -> *const T {
let ptr: *mut RcBox<T> = NonNull::as_ptr(this.ptr);
let fake_ptr = ptr as *mut T;
mem::forget(this);
// SAFETY: This cannot go through Deref::deref.
// Instead, we manually offset the pointer rather than manifesting a reference.
@ -1644,8 +1668,8 @@ impl<T> Weak<T> {
/// Returns a raw pointer to the object `T` pointed to by this `Weak<T>`.
///
/// The pointer is valid only if there are some strong references. The pointer may be dangling
/// or even [`null`] otherwise.
/// The pointer is valid only if there are some strong references. The pointer may be dangling,
/// unaligned or even [`null`] otherwise.
///
/// # Examples
///
@ -1658,31 +1682,22 @@ impl<T> Weak<T> {
/// let strong = Rc::new("hello".to_owned());
/// let weak = Rc::downgrade(&strong);
/// // Both point to the same object
/// assert!(ptr::eq(&*strong, weak.as_raw()));
/// assert!(ptr::eq(&*strong, weak.as_ptr()));
/// // The strong here keeps it alive, so we can still access the object.
/// assert_eq!("hello", unsafe { &*weak.as_raw() });
/// assert_eq!("hello", unsafe { &*weak.as_ptr() });
///
/// drop(strong);
/// // But not any more. We can do weak.as_raw(), but accessing the pointer would lead to
/// // But not any more. We can do weak.as_ptr(), but accessing the pointer would lead to
/// // undefined behaviour.
/// // assert_eq!("hello", unsafe { &*weak.as_raw() });
/// // assert_eq!("hello", unsafe { &*weak.as_ptr() });
/// ```
///
/// [`null`]: ../../std/ptr/fn.null.html
#[unstable(feature = "weak_into_raw", issue = "60728")]
pub fn as_raw(&self) -> *const T {
match self.inner() {
None => ptr::null(),
Some(inner) => {
let offset = data_offset_sized::<T>();
let ptr = inner as *const RcBox<T>;
// Note: while the pointer we create may already point to dropped value, the
// allocation still lives (it must hold the weak point as long as we are alive).
// Therefore, the offset is OK to do, it won't get out of the allocation.
let ptr = unsafe { (ptr as *const u8).offset(offset) };
ptr as *const T
}
}
pub fn as_ptr(&self) -> *const T {
let offset = data_offset_sized::<T>();
let ptr = self.ptr.cast::<u8>().as_ptr().wrapping_offset(offset);
ptr as *const T
}
/// Consumes the `Weak<T>` and turns it into a raw pointer.
@ -1691,7 +1706,7 @@ impl<T> Weak<T> {
/// can be turned back into the `Weak<T>` with [`from_raw`].
///
/// The same restrictions of accessing the target of the pointer as with
/// [`as_raw`] apply.
/// [`as_ptr`] apply.
///
/// # Examples
///
@ -1712,10 +1727,10 @@ impl<T> Weak<T> {
/// ```
///
/// [`from_raw`]: struct.Weak.html#method.from_raw
/// [`as_raw`]: struct.Weak.html#method.as_raw
/// [`as_ptr`]: struct.Weak.html#method.as_ptr
#[unstable(feature = "weak_into_raw", issue = "60728")]
pub fn into_raw(self) -> *const T {
let result = self.as_raw();
let result = self.as_ptr();
mem::forget(self);
result
}
@ -1730,9 +1745,8 @@ impl<T> Weak<T> {
///
/// # Safety
///
/// The pointer must have originated from the [`into_raw`] (or [`as_raw`], provided there was
/// a corresponding [`forget`] on the `Weak<T>`) and must still own its potential weak reference
/// count.
/// The pointer must have originated from the [`into_raw`] and must still own its potential
/// weak reference count.
///
/// It is allowed for the strong count to be 0 at the time of calling this, but the weak count
/// must be non-zero or the pointer must have originated from a dangling `Weak<T>` (one created
@ -1765,7 +1779,6 @@ impl<T> Weak<T> {
/// [`upgrade`]: struct.Weak.html#method.upgrade
/// [`Rc`]: struct.Rc.html
/// [`Weak`]: struct.Weak.html
/// [`as_raw`]: struct.Weak.html#method.as_raw
/// [`new`]: struct.Weak.html#method.new
/// [`forget`]: ../../std/mem/fn.forget.html
#[unstable(feature = "weak_into_raw", issue = "60728")]

View File

@ -566,9 +566,33 @@ impl<T: ?Sized> Arc<T> {
/// ```
#[stable(feature = "rc_raw", since = "1.17.0")]
pub fn into_raw(this: Self) -> *const T {
let ptr = Self::as_ptr(&this);
mem::forget(this);
ptr
}
/// Provides a raw pointer to the data.
///
/// The counts are not affected in way and the `Arc` is not consumed. The pointer is valid for
/// as long as there are strong counts in the `Arc`.
///
/// # Examples
///
/// ```
/// #![feature(weak_into_raw)]
///
/// use std::sync::Arc;
///
/// let x = Arc::new("hello".to_owned());
/// let y = Arc::clone(&x);
/// let x_ptr = Arc::as_ptr(&x);
/// assert_eq!(x_ptr, Arc::as_ptr(&y));
/// assert_eq!(unsafe { &*x_ptr }, "hello");
/// ```
#[unstable(feature = "weak_into_raw", issue = "60728")]
pub fn as_ptr(this: &Self) -> *const T {
let ptr: *mut ArcInner<T> = NonNull::as_ptr(this.ptr);
let fake_ptr = ptr as *mut T;
mem::forget(this);
// SAFETY: This cannot go through Deref::deref.
// Instead, we manually offset the pointer rather than manifesting a reference.
@ -1340,8 +1364,8 @@ impl<T> Weak<T> {
/// Returns a raw pointer to the object `T` pointed to by this `Weak<T>`.
///
/// The pointer is valid only if there are some strong references. The pointer may be dangling
/// or even [`null`] otherwise.
/// The pointer is valid only if there are some strong references. The pointer may be dangling,
/// unaligned or even [`null`] otherwise.
///
/// # Examples
///
@ -1354,31 +1378,22 @@ impl<T> Weak<T> {
/// let strong = Arc::new("hello".to_owned());
/// let weak = Arc::downgrade(&strong);
/// // Both point to the same object
/// assert!(ptr::eq(&*strong, weak.as_raw()));
/// assert!(ptr::eq(&*strong, weak.as_ptr()));
/// // The strong here keeps it alive, so we can still access the object.
/// assert_eq!("hello", unsafe { &*weak.as_raw() });
/// assert_eq!("hello", unsafe { &*weak.as_ptr() });
///
/// drop(strong);
/// // But not any more. We can do weak.as_raw(), but accessing the pointer would lead to
/// // But not any more. We can do weak.as_ptr(), but accessing the pointer would lead to
/// // undefined behaviour.
/// // assert_eq!("hello", unsafe { &*weak.as_raw() });
/// // assert_eq!("hello", unsafe { &*weak.as_ptr() });
/// ```
///
/// [`null`]: ../../std/ptr/fn.null.html
#[unstable(feature = "weak_into_raw", issue = "60728")]
pub fn as_raw(&self) -> *const T {
match self.inner() {
None => ptr::null(),
Some(inner) => {
let offset = data_offset_sized::<T>();
let ptr = inner as *const ArcInner<T>;
// Note: while the pointer we create may already point to dropped value, the
// allocation still lives (it must hold the weak point as long as we are alive).
// Therefore, the offset is OK to do, it won't get out of the allocation.
let ptr = unsafe { (ptr as *const u8).offset(offset) };
ptr as *const T
}
}
pub fn as_ptr(&self) -> *const T {
let offset = data_offset_sized::<T>();
let ptr = self.ptr.cast::<u8>().as_ptr().wrapping_offset(offset);
ptr as *const T
}
/// Consumes the `Weak<T>` and turns it into a raw pointer.
@ -1387,7 +1402,7 @@ impl<T> Weak<T> {
/// can be turned back into the `Weak<T>` with [`from_raw`].
///
/// The same restrictions of accessing the target of the pointer as with
/// [`as_raw`] apply.
/// [`as_ptr`] apply.
///
/// # Examples
///
@ -1408,10 +1423,10 @@ impl<T> Weak<T> {
/// ```
///
/// [`from_raw`]: struct.Weak.html#method.from_raw
/// [`as_raw`]: struct.Weak.html#method.as_raw
/// [`as_ptr`]: struct.Weak.html#method.as_ptr
#[unstable(feature = "weak_into_raw", issue = "60728")]
pub fn into_raw(self) -> *const T {
let result = self.as_raw();
let result = self.as_ptr();
mem::forget(self);
result
}
@ -1427,9 +1442,8 @@ impl<T> Weak<T> {
///
/// # Safety
///
/// The pointer must have originated from the [`into_raw`] (or [`as_raw'], provided there was
/// a corresponding [`forget`] on the `Weak<T>`) and must still own its potential weak reference
/// count.
/// The pointer must have originated from the [`into_raw`] and must still own its potential
/// weak reference count.
///
/// It is allowed for the strong count to be 0 at the time of calling this, but the weak count
/// must be non-zero or the pointer must have originated from a dangling `Weak<T>` (one created
@ -1458,7 +1472,6 @@ impl<T> Weak<T> {
/// assert!(unsafe { Weak::from_raw(raw_2) }.upgrade().is_none());
/// ```
///
/// [`as_raw`]: struct.Weak.html#method.as_raw
/// [`new`]: struct.Weak.html#method.new
/// [`into_raw`]: struct.Weak.html#method.into_raw
/// [`upgrade`]: struct.Weak.html#method.upgrade