Skip to content

Commit e02c475

Browse files
committed
Auto merge of #67339 - CAD97:rc-provenance, r=sfackler
Use pointer offset instead of deref for A/Rc::into_raw Internals thread: https://internals.rust-lang.org/t/rc-and-internal-mutability/11463/2?u=cad97 The current implementation of (`A`)`Rc::into_raw` uses the `Deref::deref` implementation to get the pointer-to-data that is returned. This is problematic in the proposed Stacked Borrow rules, as this only gets shared provenance over the data location. (Note that the strong/weak counts are `UnsafeCell` (`Cell`/`Atomic`) so shared provenance can still mutate them, but the data itself is not.) When promoted back to a real reference counted pointer, the restored pointer can be used for mutation through `::get_mut` (if it is the only surviving reference). However, this mutates through a pointer ultimately derived from a `&T` borrow, violating the Stacked Borrow rules. There are three known potential solutions to this issue: - Stacked Borrows is wrong, liballoc is correct. - Fully admit (`A`)`Rc` as an "internal mutability" type and store the data payload in an `UnsafeCell` like the strong/weak counts are. (Note: this is not needed generally since the `RcBox`/`ArcInner` is stored behind a shared `NonNull` which maintains shared write provenance as a raw pointer.) - Adjust `into_raw` to do direct manipulation of the pointer (like `from_raw`) so that it maintains write provenance and doesn't derive the pointer from a reference. This PR implements the third option, as recommended by @RalfJung. Potential future work: provide `as_raw` and `clone_raw` associated functions to allow the [`&T` -> (`A`)`Rc<T>` pattern](https://internals.rust-lang.org/t/rc-and-internal-mutability/11463/2?u=cad97) to be used soundly without creating (`A`)`Rc` from references.
2 parents 3291ae3 + eb77f7e commit e02c475

File tree

2 files changed

+24
-4
lines changed

2 files changed

+24
-4
lines changed

src/liballoc/rc.rs

+12-2
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,19 @@ impl<T: ?Sized> Rc<T> {
565565
/// ```
566566
#[stable(feature = "rc_raw", since = "1.17.0")]
567567
pub fn into_raw(this: Self) -> *const T {
568-
let ptr: *const T = &*this;
568+
let ptr: *mut RcBox<T> = NonNull::as_ptr(this.ptr);
569+
let fake_ptr = ptr as *mut T;
569570
mem::forget(this);
570-
ptr
571+
572+
// SAFETY: This cannot go through Deref::deref.
573+
// Instead, we manually offset the pointer rather than manifesting a reference.
574+
// This is so that the returned pointer retains the same provenance as our pointer.
575+
// This is required so that e.g. `get_mut` can write through the pointer
576+
// after the Rc is recovered through `from_raw`.
577+
unsafe {
578+
let offset = data_offset(&(*ptr).value);
579+
set_data_ptr(fake_ptr, (ptr as *mut u8).offset(offset))
580+
}
571581
}
572582

573583
/// Constructs an `Rc` from a raw pointer.

src/liballoc/sync.rs

+12-2
Original file line numberDiff line numberDiff line change
@@ -545,9 +545,19 @@ impl<T: ?Sized> Arc<T> {
545545
/// ```
546546
#[stable(feature = "rc_raw", since = "1.17.0")]
547547
pub fn into_raw(this: Self) -> *const T {
548-
let ptr: *const T = &*this;
548+
let ptr: *mut ArcInner<T> = NonNull::as_ptr(this.ptr);
549+
let fake_ptr = ptr as *mut T;
549550
mem::forget(this);
550-
ptr
551+
552+
// SAFETY: This cannot go through Deref::deref.
553+
// Instead, we manually offset the pointer rather than manifesting a reference.
554+
// This is so that the returned pointer retains the same provenance as our pointer.
555+
// This is required so that e.g. `get_mut` can write through the pointer
556+
// after the Arc is recovered through `from_raw`.
557+
unsafe {
558+
let offset = data_offset(&(*ptr).data);
559+
set_data_ptr(fake_ptr, (ptr as *mut u8).offset(offset))
560+
}
551561
}
552562

553563
/// Constructs an `Arc` from a raw pointer.

0 commit comments

Comments
 (0)