mirror of
https://github.com/rust-lang/rust.git
synced 2025-02-03 10:33:34 +00:00
Rollup merge of #101310 - zachs18:rc_get_unchecked_mut_docs_soundness, r=Mark-Simulacrum
Clarify and restrict when `{Arc,Rc}::get_unchecked_mut` is allowed.
(Tracking issue for `{Arc,Rc}::get_unchecked_mut`: #63292)
(I'm using `Rc` in this comment, but it applies for `Arc` all the same).
As currently documented, `Rc::get_unchecked_mut` can lead to unsoundness when multiple `Rc`/`Weak` pointers to the same allocation exist. The current documentation only requires that other `Rc`/`Weak` pointers to the same allocation "must not be dereferenced for the duration of the returned borrow". This can lead to unsoundness in (at least) two ways: variance, and `Rc<str>`/`Rc<[u8]>` aliasing. ([playground link](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=d7e2d091c389f463d121630ab0a37320)).
This PR changes the documentation of `Rc::get_unchecked_mut` to restrict usage to when all `Rc<T>`/`Weak<T>` have the exact same `T` (including lifetimes). I believe this is sufficient to prevent unsoundness, while still allowing `get_unchecked_mut` to be called on an aliased `Rc` as long as the safety contract is upheld by the caller.
## Alternatives
* A less strict, but still sound alternative would be to say that the caller must only write values which are valid for all aliased `Rc`/`Weak` inner types. (This was [mentioned](https://github.com/rust-lang/rust/issues/63292#issuecomment-568284090) in the tracking issue). This may be too complicated to clearly express in the documentation.
* A more strict alternative would be to say that there must not be any aliased `Rc`/`Weak` pointers, i.e. it is required that get_mut would return `Some(_)`. (This was also mentioned in the tracking issue). There is at least one codebase that this would cause to become unsound ([here](be5a164d77/src/memtable.rs (L166)
), where additional locking is used to ensure unique access to an aliased `Rc<T>`; I saw this because it was linked on the tracking issue).
This commit is contained in:
commit
b4513ce6f8
@ -1091,10 +1091,11 @@ impl<T: ?Sized> Rc<T> {
|
||||
///
|
||||
/// # Safety
|
||||
///
|
||||
/// Any other `Rc` or [`Weak`] pointers to the same allocation must not be dereferenced
|
||||
/// for the duration of the returned borrow.
|
||||
/// This is trivially the case if no such pointers exist,
|
||||
/// for example immediately after `Rc::new`.
|
||||
/// If any other `Rc` or [`Weak`] pointers to the same allocation exist, then
|
||||
/// they must be must not be dereferenced or have active borrows for the duration
|
||||
/// of the returned borrow, and their inner type must be exactly the same as the
|
||||
/// inner type of this Rc (including lifetimes). This is trivially the case if no
|
||||
/// such pointers exist, for example immediately after `Rc::new`.
|
||||
///
|
||||
/// # Examples
|
||||
///
|
||||
@ -1109,6 +1110,38 @@ impl<T: ?Sized> Rc<T> {
|
||||
/// }
|
||||
/// assert_eq!(*x, "foo");
|
||||
/// ```
|
||||
/// Other `Rc` pointers to the same allocation must be to the same type.
|
||||
/// ```no_run
|
||||
/// #![feature(get_mut_unchecked)]
|
||||
///
|
||||
/// use std::rc::Rc;
|
||||
///
|
||||
/// let x: Rc<str> = Rc::from("Hello, world!");
|
||||
/// let mut y: Rc<[u8]> = x.clone().into();
|
||||
/// unsafe {
|
||||
/// // this is Undefined Behavior, because x's inner type is str, not [u8]
|
||||
/// Rc::get_mut_unchecked(&mut y).fill(0xff); // 0xff is invalid in UTF-8
|
||||
/// }
|
||||
/// println!("{}", &*x); // Invalid UTF-8 in a str
|
||||
/// ```
|
||||
/// Other `Rc` pointers to the same allocation must be to the exact same type, including lifetimes.
|
||||
/// ```no_run
|
||||
/// #![feature(get_mut_unchecked)]
|
||||
///
|
||||
/// use std::rc::Rc;
|
||||
///
|
||||
/// let x: Rc<&str> = Rc::new("Hello, world!");
|
||||
/// {
|
||||
/// let s = String::from("Oh, no!");
|
||||
/// let mut y: Rc<&str> = x.clone().into();
|
||||
/// unsafe {
|
||||
/// // this is Undefined Behavior, because x's inner type
|
||||
/// // is &'long str, not &'short str
|
||||
/// *Rc::get_mut_unchecked(&mut y) = &s;
|
||||
/// }
|
||||
/// }
|
||||
/// println!("{}", &*x); // Use-after-free
|
||||
/// ```
|
||||
#[inline]
|
||||
#[unstable(feature = "get_mut_unchecked", issue = "63292")]
|
||||
pub unsafe fn get_mut_unchecked(this: &mut Self) -> &mut T {
|
||||
|
@ -1587,10 +1587,11 @@ impl<T: ?Sized> Arc<T> {
|
||||
///
|
||||
/// # Safety
|
||||
///
|
||||
/// Any other `Arc` or [`Weak`] pointers to the same allocation must not be dereferenced
|
||||
/// for the duration of the returned borrow.
|
||||
/// This is trivially the case if no such pointers exist,
|
||||
/// for example immediately after `Arc::new`.
|
||||
/// If any other `Arc` or [`Weak`] pointers to the same allocation exist, then
|
||||
/// they must be must not be dereferenced or have active borrows for the duration
|
||||
/// of the returned borrow, and their inner type must be exactly the same as the
|
||||
/// inner type of this Rc (including lifetimes). This is trivially the case if no
|
||||
/// such pointers exist, for example immediately after `Arc::new`.
|
||||
///
|
||||
/// # Examples
|
||||
///
|
||||
@ -1605,6 +1606,38 @@ impl<T: ?Sized> Arc<T> {
|
||||
/// }
|
||||
/// assert_eq!(*x, "foo");
|
||||
/// ```
|
||||
/// Other `Arc` pointers to the same allocation must be to the same type.
|
||||
/// ```no_run
|
||||
/// #![feature(get_mut_unchecked)]
|
||||
///
|
||||
/// use std::sync::Arc;
|
||||
///
|
||||
/// let x: Arc<str> = Arc::from("Hello, world!");
|
||||
/// let mut y: Arc<[u8]> = x.clone().into();
|
||||
/// unsafe {
|
||||
/// // this is Undefined Behavior, because x's inner type is str, not [u8]
|
||||
/// Arc::get_mut_unchecked(&mut y).fill(0xff); // 0xff is invalid in UTF-8
|
||||
/// }
|
||||
/// println!("{}", &*x); // Invalid UTF-8 in a str
|
||||
/// ```
|
||||
/// Other `Arc` pointers to the same allocation must be to the exact same type, including lifetimes.
|
||||
/// ```no_run
|
||||
/// #![feature(get_mut_unchecked)]
|
||||
///
|
||||
/// use std::sync::Arc;
|
||||
///
|
||||
/// let x: Arc<&str> = Arc::new("Hello, world!");
|
||||
/// {
|
||||
/// let s = String::from("Oh, no!");
|
||||
/// let mut y: Arc<&str> = x.clone().into();
|
||||
/// unsafe {
|
||||
/// // this is Undefined Behavior, because x's inner type
|
||||
/// // is &'long str, not &'short str
|
||||
/// *Arc::get_mut_unchecked(&mut y) = &s;
|
||||
/// }
|
||||
/// }
|
||||
/// println!("{}", &*x); // Use-after-free
|
||||
/// ```
|
||||
#[inline]
|
||||
#[unstable(feature = "get_mut_unchecked", issue = "63292")]
|
||||
pub unsafe fn get_mut_unchecked(this: &mut Self) -> &mut T {
|
||||
|
Loading…
Reference in New Issue
Block a user