From 96650fc7143dbacdbb3711e2d60f06401e727cea Mon Sep 17 00:00:00 2001 From: Zachary S Date: Fri, 2 Sep 2022 01:34:34 -0500 Subject: [PATCH 1/4] Clarify and restrict when `{Arc,Rc}::get_mut_unchecked` is allowed. --- library/alloc/src/rc.rs | 9 +++++---- library/alloc/src/sync.rs | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index f3cbfe27b3eed..1e9ed3889e84f 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1091,10 +1091,11 @@ impl Rc { /// /// # 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 /// diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 37e07eb5998b3..2bb5030c51eb6 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1630,10 +1630,11 @@ impl Arc { /// /// # 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 /// From 8c38cb77090af4c876f1c021b86d6c184c80cc4f Mon Sep 17 00:00:00 2001 From: Zachary S Date: Fri, 2 Sep 2022 00:47:15 -0500 Subject: [PATCH 2/4] Add examples to show when `{Arc,Rc}::get_mut_unchecked` is disallowed. --- library/alloc/src/rc.rs | 32 ++++++++++++++++++++++++++++++++ library/alloc/src/sync.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 1e9ed3889e84f..8ac1b5b488ad9 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1110,6 +1110,38 @@ impl Rc { /// } /// assert_eq!(*x, "foo"); /// ``` + /// Other `Rc` pointers to the same allocation must be to the same type. + /// ```ignore + /// #![feature(get_mut_unchecked)] + /// + /// use std::rc::Rc; + /// + /// let x: Rc = 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); // Invliad UTF-8 in a str + /// ``` + /// Other `Rc` pointers to the same allocation must be to the exact same type, including lifetimes. + /// ```ignore + /// #![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 { diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 2bb5030c51eb6..e4d7bfdaeed69 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1649,6 +1649,38 @@ impl Arc { /// } /// assert_eq!(*x, "foo"); /// ``` + /// Other `Arc` pointers to the same allocation must be to the same type. + /// ```ignore + /// #![feature(get_mut_unchecked)] + /// + /// use std::sync::Arc; + /// + /// let x: Arc = 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); // Invliad UTF-8 in a str + /// ``` + /// Other `Arc` pointers to the same allocation must be to the exact same type, including lifetimes. + /// ```ignore + /// #![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 { From f542b068f24b4129ef99162f5bcf66e8290f9aac Mon Sep 17 00:00:00 2001 From: zachs18 <8355914+zachs18@users.noreply.github.com> Date: Fri, 2 Sep 2022 01:45:57 -0500 Subject: [PATCH 3/4] Apply suggestions from code review Fix spelling error. --- library/alloc/src/rc.rs | 2 +- library/alloc/src/sync.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 8ac1b5b488ad9..c1db035167b1e 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1122,7 +1122,7 @@ impl Rc { /// // 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); // Invliad UTF-8 in a str + /// println!("{}", &*x); // Invalid UTF-8 in a str /// ``` /// Other `Rc` pointers to the same allocation must be to the exact same type, including lifetimes. /// ```ignore diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index e4d7bfdaeed69..e32ce5a989172 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1661,7 +1661,7 @@ impl Arc { /// // 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); // Invliad UTF-8 in a str + /// println!("{}", &*x); // Invalid UTF-8 in a str /// ``` /// Other `Arc` pointers to the same allocation must be to the exact same type, including lifetimes. /// ```ignore From 734f7244723f579433b7defcd1fd1c2e66406737 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Fri, 2 Sep 2022 02:23:56 -0500 Subject: [PATCH 4/4] Change undefined-behavior doctests from ignore to no_run. --- library/alloc/src/rc.rs | 4 ++-- library/alloc/src/sync.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index c1db035167b1e..2f41ca689ba6c 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1111,7 +1111,7 @@ impl Rc { /// assert_eq!(*x, "foo"); /// ``` /// Other `Rc` pointers to the same allocation must be to the same type. - /// ```ignore + /// ```no_run /// #![feature(get_mut_unchecked)] /// /// use std::rc::Rc; @@ -1125,7 +1125,7 @@ impl Rc { /// println!("{}", &*x); // Invalid UTF-8 in a str /// ``` /// Other `Rc` pointers to the same allocation must be to the exact same type, including lifetimes. - /// ```ignore + /// ```no_run /// #![feature(get_mut_unchecked)] /// /// use std::rc::Rc; diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index e32ce5a989172..4ef3c528c4414 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1650,7 +1650,7 @@ impl Arc { /// assert_eq!(*x, "foo"); /// ``` /// Other `Arc` pointers to the same allocation must be to the same type. - /// ```ignore + /// ```no_run /// #![feature(get_mut_unchecked)] /// /// use std::sync::Arc; @@ -1664,7 +1664,7 @@ impl Arc { /// println!("{}", &*x); // Invalid UTF-8 in a str /// ``` /// Other `Arc` pointers to the same allocation must be to the exact same type, including lifetimes. - /// ```ignore + /// ```no_run /// #![feature(get_mut_unchecked)] /// /// use std::sync::Arc;