From e6396bca013b1b4cc1a177ef11e8fd5c270a4ea5 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Sat, 11 May 2024 09:54:32 -0500 Subject: [PATCH] Use a single static for all default slice Arcs. Also adds debug_asserts in Drop for Weak/Arc that the shared static is not being "dropped"/"deallocated". --- library/alloc/src/sync.rs | 77 ++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 29 deletions(-) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index a925b544bc2..9081ea5c679 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -2468,6 +2468,15 @@ fn drop(&mut self) { // [2]: (https://github.com/rust-lang/rust/pull/41714) acquire!(self.inner().strong); + // Make sure we aren't trying to "drop" the shared static for empty slices + // used by Default::default. + debug_assert!( + !ptr::addr_eq(self.ptr.as_ptr(), &STATIC_INNER_SLICE.inner), + "Arcs backed by a static should never be reach a strong count of 0. \ + Likely decrement_strong_count or from_raw were called too many times.", + ); + + unsafe { self.drop_slow(); } @@ -3059,6 +3068,15 @@ fn drop(&mut self) { if inner.weak.fetch_sub(1, Release) == 1 { acquire!(inner.weak); + + // Make sure we aren't trying to "deallocate" the shared static for empty slices + // used by Default::default. + debug_assert!( + !ptr::addr_eq(self.ptr.as_ptr(), &STATIC_INNER_SLICE.inner), + "Arc/Weaks backed by a static should never be deallocated. \ + Likely decrement_strong_count or from_raw were called too many times.", + ); + unsafe { self.alloc.deallocate(self.ptr.cast(), Layout::for_value_raw(self.ptr.as_ptr())) } @@ -3300,6 +3318,28 @@ fn default() -> Arc { } } +/// Struct to hold the static `ArcInner` used for empty `Arc` as +/// returned by `Default::default`. +/// +/// Layout notes: +/// * `repr(align(16))` so we can use it for `[T]` with `align_of::() <= 16`. +/// * `repr(C)` so `inner` is at offset 0 (and thus guaranteed to actually be aligned to 16). +/// * `[u8; 1]` (to be initialized with 0) so it can be used for `Arc`. +#[repr(C, align(16))] +struct SliceArcInnerForStatic { + inner: ArcInner<[u8; 1]>, +} +const MAX_STATIC_INNER_SLICE_ALIGNMENT: usize = 16; + +static STATIC_INNER_SLICE: SliceArcInnerForStatic = SliceArcInnerForStatic { + inner: ArcInner { + strong: atomic::AtomicUsize::new(1), + weak: atomic::AtomicUsize::new(1), + data: [0], + }, +}; + + #[cfg(not(no_global_oom_handling))] #[stable(feature = "more_rc_default_impls", since = "CURRENT_RUSTC_VERSION")] impl Default for Arc { @@ -3324,12 +3364,7 @@ impl Default for Arc { #[inline] fn default() -> Self { use core::ffi::CStr; - static STATIC_INNER_CSTR: ArcInner<[u8; 1]> = ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data: [0], - }; - let inner: NonNull> = NonNull::from(&STATIC_INNER_CSTR); + let inner: NonNull> = NonNull::from(&STATIC_INNER_SLICE.inner); let inner: NonNull> = NonNull::new(inner.as_ptr() as *mut ArcInner).unwrap(); // `this` semantically is the Arc "owned" by the static, so make sure not to drop it. let this: mem::ManuallyDrop> = unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) }; @@ -3345,31 +3380,15 @@ impl Default for Arc<[T]> { /// This may or may not share an allocation with other Arcs. #[inline] fn default() -> Self { - let alignment_of_t: usize = mem::align_of::(); - // We only make statics for the lowest five alignments. - // Alignments greater than that will use dynamic allocation. - macro_rules! use_static_inner_for_alignments { - ($($alignment:literal),*) => { - $(if alignment_of_t == $alignment { - // Note: this must be in a new scope because static and type names are unhygenic. - #[repr(align($alignment))] - struct Aligned; - static ALIGNED_STATIC_INNER: ArcInner = ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data: Aligned, - }; - let inner: NonNull> = NonNull::from(&ALIGNED_STATIC_INNER); - let inner: NonNull> = inner.cast(); - // `this` semantically is the Arc "owned" by the static, so make sure not to drop it. - let this: mem::ManuallyDrop> = unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) }; - return (*this).clone(); - })* - }; + if mem::align_of::() <= MAX_STATIC_INNER_SLICE_ALIGNMENT { + let inner: NonNull> = NonNull::from(&STATIC_INNER_SLICE.inner); + let inner: NonNull> = inner.cast(); + // `this` semantically is the Arc "owned" by the static, so make sure not to drop it. + let this: mem::ManuallyDrop> = unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) }; + return (*this).clone(); } - use_static_inner_for_alignments!(1, 2, 4, 8, 16); - // If T's alignment is not one of the ones we have a static for, make a new unique allocation. + // If T's alignment is too large for the static, make a new unique allocation. let arr: [T; 0] = []; Arc::from(arr) }