From 728938346b02a9688c44253c19b15baa7551fd80 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Wed, 8 Jul 2020 16:23:38 -0400 Subject: [PATCH 1/6] Adjust rc::Weak::from_raw to support unsized T --- library/alloc/src/rc.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index f998e49dcfcde..27e6e11090353 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1841,17 +1841,18 @@ impl Weak { /// [`new`]: Weak::new #[stable(feature = "weak_into_raw", since = "1.45.0")] pub unsafe fn from_raw(ptr: *const T) -> Self { - if ptr.is_null() { - Self::new() - } else { - // See Rc::from_raw for details - unsafe { - let offset = data_offset(ptr); - let fake_ptr = ptr as *mut RcBox; - let ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset)); - Weak { ptr: NonNull::new(ptr).expect("Invalid pointer passed to from_raw") } - } - } + // SAFETY: data_offset is safe to call, because this pointer originates from a Weak. + // See Weak::as_ptr for context on how the input pointer is derived. + let offset = unsafe { data_offset(ptr) }; + + // Reverse the offset to find the original RcBox. + // SAFETY: we use wrapping_offset here because the pointer may be dangling (iff T: Sized). + let ptr = unsafe { + set_data_ptr(ptr as *mut RcBox, (ptr as *mut u8).wrapping_offset(-offset)) + }; + + // SAFETY: we now have recovered the original Weak pointer, so can create the Weak. + Weak { ptr: unsafe { NonNull::new_unchecked(ptr) } } } } From 0c61ce2cf0560577923abafab2e5bfce14516525 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Wed, 8 Jul 2020 16:24:56 -0400 Subject: [PATCH 2/6] ?Sized bounds for rc::Weak::as_ptr and friends --- library/alloc/src/rc.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 27e6e11090353..1d4ce8e268294 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1721,7 +1721,21 @@ impl Weak { pub fn new() -> Weak { Weak { ptr: NonNull::new(usize::MAX as *mut RcBox).expect("MAX is not 0") } } +} + +pub(crate) fn is_dangling(ptr: NonNull) -> bool { + let address = ptr.as_ptr() as *mut () as usize; + address == usize::MAX +} + +/// Helper type to allow accessing the reference counts without +/// making any assertions about the data field. +struct WeakInner<'a> { + weak: &'a Cell, + strong: &'a Cell, +} +impl Weak { /// Returns a raw pointer to the object `T` pointed to by this `Weak`. /// /// The pointer is valid only if there are some strong references. The pointer may be dangling, @@ -1854,21 +1868,7 @@ impl Weak { // SAFETY: we now have recovered the original Weak pointer, so can create the Weak. Weak { ptr: unsafe { NonNull::new_unchecked(ptr) } } } -} - -pub(crate) fn is_dangling(ptr: NonNull) -> bool { - let address = ptr.as_ptr() as *mut () as usize; - address == usize::MAX -} - -/// Helper type to allow accessing the reference counts without -/// making any assertions about the data field. -struct WeakInner<'a> { - weak: &'a Cell, - strong: &'a Cell, -} -impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Rc`], delaying /// dropping of the inner value if successful. /// From 5e7406c9569dce75a042ce079918cf03cfca842a Mon Sep 17 00:00:00 2001 From: CAD97 Date: Tue, 14 Jul 2020 15:17:55 -0400 Subject: [PATCH 3/6] Adjust sync::Weak::from_raw to support unsized T --- library/alloc/src/sync.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 6a240fbb42a99..06dec6f01f8d3 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1629,17 +1629,18 @@ impl Weak { /// [`forget`]: std::mem::forget #[stable(feature = "weak_into_raw", since = "1.45.0")] pub unsafe fn from_raw(ptr: *const T) -> Self { - if ptr.is_null() { - Self::new() - } else { - // See Arc::from_raw for details - unsafe { - let offset = data_offset(ptr); - let fake_ptr = ptr as *mut ArcInner; - let ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset)); - Weak { ptr: NonNull::new(ptr).expect("Invalid pointer passed to from_raw") } - } - } + // SAFETY: data_offset is safe to call, because this pointer originates from a Weak. + // See Weak::as_ptr for context on how the input pointer is derived. + let offset = unsafe { data_offset(ptr) }; + + // Reverse the offset to find the original ArcInner. + // SAFETY: we use wrapping_offset here because the pointer may be dangling (iff T: Sized) + let ptr = unsafe { + set_data_ptr(ptr as *mut ArcInner, (ptr as *mut u8).wrapping_offset(-offset)) + }; + + // SAFETY: we now have recovered the original Weak pointer, so can create the Weak. + unsafe { Weak { ptr: NonNull::new_unchecked(ptr) } } } } From 9d9903c5a50bb1f5b5fc3045b86172279eff7d30 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Tue, 14 Jul 2020 15:22:48 -0400 Subject: [PATCH 4/6] Allow Weak::as_ptr and friends for unsized T --- library/alloc/src/sync.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 06dec6f01f8d3..f064881717d63 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1509,7 +1509,16 @@ impl Weak { pub fn new() -> Weak { Weak { ptr: NonNull::new(usize::MAX as *mut ArcInner).expect("MAX is not 0") } } +} +/// Helper type to allow accessing the reference counts without +/// making any assertions about the data field. +struct WeakInner<'a> { + weak: &'a atomic::AtomicUsize, + strong: &'a atomic::AtomicUsize, +} + +impl Weak { /// Returns a raw pointer to the object `T` pointed to by this `Weak`. /// /// The pointer is valid only if there are some strong references. The pointer may be dangling, @@ -1642,16 +1651,7 @@ impl Weak { // SAFETY: we now have recovered the original Weak pointer, so can create the Weak. unsafe { Weak { ptr: NonNull::new_unchecked(ptr) } } } -} -/// Helper type to allow accessing the reference counts without -/// making any assertions about the data field. -struct WeakInner<'a> { - weak: &'a atomic::AtomicUsize, - strong: &'a atomic::AtomicUsize, -} - -impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Arc`], delaying /// dropping of the inner value if successful. /// From 3d07108d3600dff50e564f57dd390337dbe14d55 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Tue, 14 Jul 2020 15:35:14 -0400 Subject: [PATCH 5/6] Add tests for weak into/from raw --- library/alloc/src/rc/tests.rs | 42 +++++++++++++++++++++++++++++++++ library/alloc/src/sync/tests.rs | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index fed48a59f809e..bb5c3f4f90433 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -190,6 +190,48 @@ fn test_into_from_raw_unsized() { assert_eq!(rc2.to_string(), "123"); } +#[test] +fn into_from_weak_raw() { + let x = Rc::new(box "hello"); + let y = Rc::downgrade(&x); + + let y_ptr = Weak::into_raw(y); + unsafe { + assert_eq!(**y_ptr, "hello"); + + let y = Weak::from_raw(y_ptr); + let y_up = Weak::upgrade(&y).unwrap(); + assert_eq!(**y_up, "hello"); + drop(y_up); + + assert_eq!(Rc::try_unwrap(x).map(|x| *x), Ok("hello")); + } +} + +#[test] +fn test_into_from_weak_raw_unsized() { + use std::fmt::Display; + use std::string::ToString; + + let arc: Rc = Rc::from("foo"); + let weak: Weak = Rc::downgrade(&arc); + + let ptr = Weak::into_raw(weak.clone()); + let weak2 = unsafe { Weak::from_raw(ptr) }; + + assert_eq!(unsafe { &*ptr }, "foo"); + assert!(weak.ptr_eq(&weak2)); + + let arc: Rc = Rc::new(123); + let weak: Weak = Rc::downgrade(&arc); + + let ptr = Weak::into_raw(weak.clone()); + let weak2 = unsafe { Weak::from_raw(ptr) }; + + assert_eq!(unsafe { &*ptr }.to_string(), "123"); + assert!(weak.ptr_eq(&weak2)); +} + #[test] fn get_mut() { let mut x = Rc::new(3); diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index d25171716061d..77f328d48f94d 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -140,6 +140,48 @@ fn test_into_from_raw_unsized() { assert_eq!(arc2.to_string(), "123"); } +#[test] +fn into_from_weak_raw() { + let x = Arc::new(box "hello"); + let y = Arc::downgrade(&x); + + let y_ptr = Weak::into_raw(y); + unsafe { + assert_eq!(**y_ptr, "hello"); + + let y = Weak::from_raw(y_ptr); + let y_up = Weak::upgrade(&y).unwrap(); + assert_eq!(**y_up, "hello"); + drop(y_up); + + assert_eq!(Arc::try_unwrap(x).map(|x| *x), Ok("hello")); + } +} + +#[test] +fn test_into_from_weak_raw_unsized() { + use std::fmt::Display; + use std::string::ToString; + + let arc: Arc = Arc::from("foo"); + let weak: Weak = Arc::downgrade(&arc); + + let ptr = Weak::into_raw(weak.clone()); + let weak2 = unsafe { Weak::from_raw(ptr) }; + + assert_eq!(unsafe { &*ptr }, "foo"); + assert!(weak.ptr_eq(&weak2)); + + let arc: Arc = Arc::new(123); + let weak: Weak = Arc::downgrade(&arc); + + let ptr = Weak::into_raw(weak.clone()); + let weak2 = unsafe { Weak::from_raw(ptr) }; + + assert_eq!(unsafe { &*ptr }.to_string(), "123"); + assert!(weak.ptr_eq(&weak2)); +} + #[test] fn test_cowarc_clone_make_mut() { let mut cow0 = Arc::new(75); From e27ef130c1d1cc7d8c3779a4e66e413a7a943ad4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 3 Oct 2020 12:15:26 +0200 Subject: [PATCH 6/6] grammar nit --- 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 1d4ce8e268294..0a2fe46e3ff95 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1860,7 +1860,7 @@ impl Weak { let offset = unsafe { data_offset(ptr) }; // Reverse the offset to find the original RcBox. - // SAFETY: we use wrapping_offset here because the pointer may be dangling (iff T: Sized). + // SAFETY: we use wrapping_offset here because the pointer may be dangling (but only if T: Sized). let ptr = unsafe { set_data_ptr(ptr as *mut RcBox, (ptr as *mut u8).wrapping_offset(-offset)) }; diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index f064881717d63..f7873b34c446e 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1643,7 +1643,7 @@ impl Weak { let offset = unsafe { data_offset(ptr) }; // Reverse the offset to find the original ArcInner. - // SAFETY: we use wrapping_offset here because the pointer may be dangling (iff T: Sized) + // SAFETY: we use wrapping_offset here because the pointer may be dangling (but only if T: Sized) let ptr = unsafe { set_data_ptr(ptr as *mut ArcInner, (ptr as *mut u8).wrapping_offset(-offset)) };