Skip to content

Mark Arc::from_inner / Rc::from_inner as unsafe #89741

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

Merged
merged 1 commit into from
Nov 21, 2021
Merged
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
51 changes: 31 additions & 20 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
@@ -333,12 +333,12 @@ impl<T: ?Sized> Rc<T> {
unsafe { self.ptr.as_ref() }
}

fn from_inner(ptr: NonNull<RcBox<T>>) -> Self {
unsafe fn from_inner(ptr: NonNull<RcBox<T>>) -> Self {
Self { ptr, phantom: PhantomData }
}

unsafe fn from_ptr(ptr: *mut RcBox<T>) -> Self {
Self::from_inner(unsafe { NonNull::new_unchecked(ptr) })
unsafe { Self::from_inner(NonNull::new_unchecked(ptr)) }
}
}

@@ -359,9 +359,11 @@ impl<T> Rc<T> {
// pointers, which ensures that the weak destructor never frees
// the allocation while the strong destructor is running, even
// if the weak pointer is stored inside the strong one.
Self::from_inner(
Box::leak(box RcBox { strong: Cell::new(1), weak: Cell::new(1), value }).into(),
)
unsafe {
Self::from_inner(
Box::leak(box RcBox { strong: Cell::new(1), weak: Cell::new(1), value }).into(),
)
}
}

/// Constructs a new `Rc<T>` using a weak reference to itself. Attempting
@@ -412,16 +414,16 @@ impl<T> Rc<T> {
// otherwise.
let data = data_fn(&weak);

unsafe {
let strong = unsafe {
let inner = init_ptr.as_ptr();
ptr::write(ptr::addr_of_mut!((*inner).value), data);

let prev_value = (*inner).strong.get();
debug_assert_eq!(prev_value, 0, "No prior strong references should exist");
(*inner).strong.set(1);
}

let strong = Rc::from_inner(init_ptr);
Rc::from_inner(init_ptr)
};

// Strong references should collectively own a shared weak reference,
// so don't run the destructor for our old weak reference.
@@ -511,10 +513,12 @@ impl<T> Rc<T> {
// pointers, which ensures that the weak destructor never frees
// the allocation while the strong destructor is running, even
// if the weak pointer is stored inside the strong one.
Ok(Self::from_inner(
Box::leak(Box::try_new(RcBox { strong: Cell::new(1), weak: Cell::new(1), value })?)
.into(),
))
unsafe {
Ok(Self::from_inner(
Box::leak(Box::try_new(RcBox { strong: Cell::new(1), weak: Cell::new(1), value })?)
.into(),
))
}
}

/// Constructs a new `Rc` with uninitialized contents, returning an error if the allocation fails
@@ -733,7 +737,7 @@ impl<T> Rc<mem::MaybeUninit<T>> {
#[unstable(feature = "new_uninit", issue = "63291")]
#[inline]
pub unsafe fn assume_init(self) -> Rc<T> {
Rc::from_inner(mem::ManuallyDrop::new(self).ptr.cast())
unsafe { Rc::from_inner(mem::ManuallyDrop::new(self).ptr.cast()) }
}
}

@@ -1199,9 +1203,11 @@ impl Rc<dyn Any> {
/// ```
pub fn downcast<T: Any>(self) -> Result<Rc<T>, Rc<dyn Any>> {
if (*self).is::<T>() {
let ptr = self.ptr.cast::<RcBox<T>>();
forget(self);
Ok(Rc::from_inner(ptr))
unsafe {
let ptr = self.ptr.cast::<RcBox<T>>();
forget(self);
Ok(Rc::from_inner(ptr))
}
} else {
Err(self)
}
@@ -1474,8 +1480,10 @@ impl<T: ?Sized> Clone for Rc<T> {
/// ```
#[inline]
fn clone(&self) -> Rc<T> {
self.inner().inc_strong();
Self::from_inner(self.ptr)
unsafe {
self.inner().inc_strong();
Self::from_inner(self.ptr)
}
}
}

@@ -2225,11 +2233,14 @@ impl<T: ?Sized> Weak<T> {
#[stable(feature = "rc_weak", since = "1.4.0")]
pub fn upgrade(&self) -> Option<Rc<T>> {
let inner = self.inner()?;

if inner.strong() == 0 {
None
} else {
inner.inc_strong();
Some(Rc::from_inner(self.ptr))
unsafe {
inner.inc_strong();
Some(Rc::from_inner(self.ptr))
}
}
}

26 changes: 14 additions & 12 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
@@ -252,7 +252,7 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Arc<U>> for Arc<T> {}
impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Arc<U>> for Arc<T> {}

impl<T: ?Sized> Arc<T> {
fn from_inner(ptr: NonNull<ArcInner<T>>) -> Self {
unsafe fn from_inner(ptr: NonNull<ArcInner<T>>) -> Self {
Self { ptr, phantom: PhantomData }
}

@@ -348,7 +348,7 @@ impl<T> Arc<T> {
weak: atomic::AtomicUsize::new(1),
data,
};
Self::from_inner(Box::leak(x).into())
unsafe { Self::from_inner(Box::leak(x).into()) }
}

/// Constructs a new `Arc<T>` using a weak reference to itself. Attempting
@@ -397,7 +397,7 @@ impl<T> Arc<T> {

// Now we can properly initialize the inner value and turn our weak
// reference into a strong reference.
unsafe {
let strong = unsafe {
let inner = init_ptr.as_ptr();
ptr::write(ptr::addr_of_mut!((*inner).data), data);

@@ -415,9 +415,9 @@ impl<T> Arc<T> {
// possible with safe code alone.
let prev_value = (*inner).strong.fetch_add(1, Release);
debug_assert_eq!(prev_value, 0, "No prior strong references should exist");
}

let strong = Arc::from_inner(init_ptr);
Arc::from_inner(init_ptr)
};

// Strong references should collectively own a shared weak reference,
// so don't run the destructor for our old weak reference.
@@ -526,7 +526,7 @@ impl<T> Arc<T> {
weak: atomic::AtomicUsize::new(1),
data,
})?;
Ok(Self::from_inner(Box::leak(x).into()))
unsafe { Ok(Self::from_inner(Box::leak(x).into())) }
}

/// Constructs a new `Arc` with uninitialized contents, returning an error
@@ -737,7 +737,7 @@ impl<T> Arc<mem::MaybeUninit<T>> {
#[unstable(feature = "new_uninit", issue = "63291")]
#[inline]
pub unsafe fn assume_init(self) -> Arc<T> {
Arc::from_inner(mem::ManuallyDrop::new(self).ptr.cast())
unsafe { Arc::from_inner(mem::ManuallyDrop::new(self).ptr.cast()) }
}
}

@@ -1327,7 +1327,7 @@ impl<T: ?Sized> Clone for Arc<T> {
abort();
}

Self::from_inner(self.ptr)
unsafe { Self::from_inner(self.ptr) }
}
}

@@ -1654,9 +1654,11 @@ impl Arc<dyn Any + Send + Sync> {
T: Any + Send + Sync + 'static,
{
if (*self).is::<T>() {
let ptr = self.ptr.cast::<ArcInner<T>>();
mem::forget(self);
Ok(Arc::from_inner(ptr))
unsafe {
let ptr = self.ptr.cast::<ArcInner<T>>();
mem::forget(self);
Ok(Arc::from_inner(ptr))
}
} else {
Err(self)
}
@@ -1880,7 +1882,7 @@ impl<T: ?Sized> Weak<T> {
// value can be initialized after `Weak` references have already been created. In that case, we
// expect to observe the fully initialized value.
match inner.strong.compare_exchange_weak(n, n + 1, Acquire, Relaxed) {
Ok(_) => return Some(Arc::from_inner(self.ptr)), // null checked above
Ok(_) => return Some(unsafe { Arc::from_inner(self.ptr) }), // null checked above
Err(old) => n = old,
}
}