From 4b281c909e9f0fce68a3f2201c093769158fca95 Mon Sep 17 00:00:00 2001 From: kadiwa Date: Sun, 7 Jan 2024 14:38:27 +0100 Subject: [PATCH] alloc: avoid creating references in Rc/Arc impls --- library/alloc/src/rc.rs | 37 +++++++++++++++++++++++-------------- library/alloc/src/sync.rs | 31 ++++++++++++++++++------------- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 263b1449de156..e1d1e540a831e 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1628,7 +1628,7 @@ impl Rc { pub unsafe fn get_mut_unchecked(this: &mut Self) -> &mut T { // We are careful to *not* create a reference covering the "count" fields, as // this would conflict with accesses to the reference counts (e.g. by `Weak`). - unsafe { &mut (*this.ptr.as_ptr()).value } + unsafe { &mut *ptr::addr_of_mut!((*this.ptr.as_ptr()).value) } } #[inline] @@ -1885,10 +1885,10 @@ impl Rc { // Initialize the RcBox let inner = mem_to_rcbox(ptr.as_non_null_ptr().as_ptr()); unsafe { - debug_assert_eq!(Layout::for_value(&*inner), layout); + debug_assert_eq!(Layout::for_value_raw(inner), layout); - ptr::write(&mut (*inner).strong, Cell::new(1)); - ptr::write(&mut (*inner).weak, Cell::new(1)); + ptr::write(ptr::addr_of_mut!((*inner).strong), Cell::new(1)); + ptr::write(ptr::addr_of_mut!((*inner).weak), Cell::new(1)); } Ok(inner) @@ -1902,7 +1902,7 @@ impl Rc { // Allocate for the `RcBox` using the given value. unsafe { Rc::::allocate_for_layout( - Layout::for_value(&*ptr), + Layout::for_value_raw(ptr), |layout| alloc.allocate(layout), |mem| mem.with_metadata_of(ptr as *const RcBox), ) @@ -1918,7 +1918,7 @@ impl Rc { // Copy value as bytes ptr::copy_nonoverlapping( &*src as *const T as *const u8, - &mut (*ptr).value as *mut _ as *mut u8, + ptr::addr_of_mut!((*ptr).value) as *mut u8, value_size, ); @@ -1952,7 +1952,11 @@ impl Rc<[T]> { unsafe fn copy_from_slice(v: &[T]) -> Rc<[T]> { unsafe { let ptr = Self::allocate_for_slice(v.len()); - ptr::copy_nonoverlapping(v.as_ptr(), &mut (*ptr).value as *mut [T] as *mut T, v.len()); + ptr::copy_nonoverlapping( + v.as_ptr(), + ptr::addr_of_mut!((*ptr).value) as *mut T, + v.len(), + ); Self::from_ptr(ptr) } } @@ -1987,15 +1991,15 @@ impl Rc<[T]> { let ptr = Self::allocate_for_slice(len); let mem = ptr as *mut _ as *mut u8; - let layout = Layout::for_value(&*ptr); + let layout = Layout::for_value_raw(ptr); // Pointer to first element - let elems = &mut (*ptr).value as *mut [T] as *mut T; + let elems = ptr::addr_of_mut!((*ptr).value) as *mut T; let mut guard = Guard { mem: NonNull::new_unchecked(mem), elems, layout, n_elems: 0 }; - for (i, item) in iter.enumerate() { - ptr::write(elems.add(i), item); + for item in iter { + ptr::write(elems.add(guard.n_elems), item); guard.n_elems += 1; } @@ -2524,7 +2528,7 @@ impl From> for Rc<[T], A> { let (vec_ptr, len, cap, alloc) = v.into_raw_parts_with_alloc(); let rc_ptr = Self::allocate_for_slice_in(len, &alloc); - ptr::copy_nonoverlapping(vec_ptr, &mut (*rc_ptr).value as *mut [T] as *mut T, len); + ptr::copy_nonoverlapping(vec_ptr, ptr::addr_of_mut!((*rc_ptr).value) as *mut T, len); // Create a `Vec` with length 0, to deallocate the buffer // without dropping its contents or the allocator @@ -3099,7 +3103,10 @@ impl Weak { // is dropped, the data field will be dropped in-place). Some(unsafe { let ptr = self.ptr.as_ptr(); - WeakInner { strong: &(*ptr).strong, weak: &(*ptr).weak } + WeakInner { + strong: &*ptr::addr_of!((*ptr).strong), + weak: &*ptr::addr_of!((*ptr).weak), + } }) } } @@ -3499,7 +3506,9 @@ impl DerefMut for UniqueRc { // SAFETY: This pointer was allocated at creation time so we know it is valid. We know we // have unique ownership and therefore it's safe to make a mutable reference because // `UniqueRc` owns the only strong reference to itself. - unsafe { &mut (*self.ptr.as_ptr()).value } + // We are careful to *not* create a reference covering the "count" fields, as + // this would conflict with accesses to the weak reference count. + unsafe { &mut *ptr::addr_of_mut!((*self.ptr.as_ptr()).value) } } } diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 5273b3cb2dafa..bf21569a0ae73 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1828,11 +1828,11 @@ impl Arc { mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner, ) -> *mut ArcInner { let inner = mem_to_arcinner(ptr.as_non_null_ptr().as_ptr()); - debug_assert_eq!(unsafe { Layout::for_value(&*inner) }, layout); + debug_assert_eq!(unsafe { Layout::for_value_raw(inner) }, layout); unsafe { - ptr::write(&mut (*inner).strong, atomic::AtomicUsize::new(1)); - ptr::write(&mut (*inner).weak, atomic::AtomicUsize::new(1)); + ptr::write(ptr::addr_of_mut!((*inner).strong), atomic::AtomicUsize::new(1)); + ptr::write(ptr::addr_of_mut!((*inner).weak), atomic::AtomicUsize::new(1)); } inner @@ -1847,7 +1847,7 @@ impl Arc { // Allocate for the `ArcInner` using the given value. unsafe { Arc::allocate_for_layout( - Layout::for_value(&*ptr), + Layout::for_value_raw(ptr), |layout| alloc.allocate(layout), |mem| mem.with_metadata_of(ptr as *const ArcInner), ) @@ -1863,7 +1863,7 @@ impl Arc { // Copy value as bytes ptr::copy_nonoverlapping( &*src as *const T as *const u8, - &mut (*ptr).data as *mut _ as *mut u8, + ptr::addr_of_mut!((*ptr).data) as *mut u8, value_size, ); @@ -1898,7 +1898,7 @@ impl Arc<[T]> { unsafe { let ptr = Self::allocate_for_slice(v.len()); - ptr::copy_nonoverlapping(v.as_ptr(), &mut (*ptr).data as *mut [T] as *mut T, v.len()); + ptr::copy_nonoverlapping(v.as_ptr(), ptr::addr_of_mut!((*ptr).data) as *mut T, v.len()); Self::from_ptr(ptr) } @@ -1934,15 +1934,15 @@ impl Arc<[T]> { let ptr = Self::allocate_for_slice(len); let mem = ptr as *mut _ as *mut u8; - let layout = Layout::for_value(&*ptr); + let layout = Layout::for_value_raw(ptr); // Pointer to first element - let elems = &mut (*ptr).data as *mut [T] as *mut T; + let elems = ptr::addr_of_mut!((*ptr).data) as *mut T; let mut guard = Guard { mem: NonNull::new_unchecked(mem), elems, layout, n_elems: 0 }; - for (i, item) in iter.enumerate() { - ptr::write(elems.add(i), item); + for item in iter { + ptr::write(elems.add(guard.n_elems), item); guard.n_elems += 1; } @@ -2305,7 +2305,7 @@ impl Arc { pub unsafe fn get_mut_unchecked(this: &mut Self) -> &mut T { // We are careful to *not* create a reference covering the "count" fields, as // this would alias with concurrent access to the reference counts (e.g. by `Weak`). - unsafe { &mut (*this.ptr.as_ptr()).data } + unsafe { &mut *ptr::addr_of_mut!((*this.ptr.as_ptr()).data) } } /// Determine whether this is the unique reference (including weak refs) to @@ -2849,7 +2849,12 @@ impl Weak { // We are careful to *not* create a reference covering the "data" field, as // the field may be mutated concurrently (for example, if the last `Arc` // is dropped, the data field will be dropped in-place). - Some(unsafe { WeakInner { strong: &(*ptr).strong, weak: &(*ptr).weak } }) + Some(unsafe { + WeakInner { + strong: &*ptr::addr_of!((*ptr).strong), + weak: &*ptr::addr_of!((*ptr).weak), + } + }) } } @@ -3380,7 +3385,7 @@ impl From> for Arc<[T], A> { let (vec_ptr, len, cap, alloc) = v.into_raw_parts_with_alloc(); let rc_ptr = Self::allocate_for_slice_in(len, &alloc); - ptr::copy_nonoverlapping(vec_ptr, &mut (*rc_ptr).data as *mut [T] as *mut T, len); + ptr::copy_nonoverlapping(vec_ptr, ptr::addr_of_mut!((*rc_ptr).data) as *mut T, len); // Create a `Vec` with length 0, to deallocate the buffer // without dropping its contents or the allocator