Skip to content

Commit 0e48cef

Browse files
authored
Rollup merge of rust-lang#125283 - zachs18:arc-default-shared, r=dtolnay
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". As per rust-lang#124640 (review) r? dtolnay
2 parents 798d7a7 + 3299823 commit 0e48cef

File tree

1 file changed

+56
-31
lines changed

1 file changed

+56
-31
lines changed

library/alloc/src/sync.rs

+56-31
Original file line numberDiff line numberDiff line change
@@ -2468,6 +2468,14 @@ unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Arc<T, A> {
24682468
// [2]: (https://github.com/rust-lang/rust/pull/41714)
24692469
acquire!(self.inner().strong);
24702470

2471+
// Make sure we aren't trying to "drop" the shared static for empty slices
2472+
// used by Default::default.
2473+
debug_assert!(
2474+
!ptr::addr_eq(self.ptr.as_ptr(), &STATIC_INNER_SLICE.inner),
2475+
"Arcs backed by a static should never reach a strong count of 0. \
2476+
Likely decrement_strong_count or from_raw were called too many times.",
2477+
);
2478+
24712479
unsafe {
24722480
self.drop_slow();
24732481
}
@@ -3059,6 +3067,15 @@ unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Weak<T, A> {
30593067

30603068
if inner.weak.fetch_sub(1, Release) == 1 {
30613069
acquire!(inner.weak);
3070+
3071+
// Make sure we aren't trying to "deallocate" the shared static for empty slices
3072+
// used by Default::default.
3073+
debug_assert!(
3074+
!ptr::addr_eq(self.ptr.as_ptr(), &STATIC_INNER_SLICE.inner),
3075+
"Arc/Weaks backed by a static should never be deallocated. \
3076+
Likely decrement_strong_count or from_raw were called too many times.",
3077+
);
3078+
30623079
unsafe {
30633080
self.alloc.deallocate(self.ptr.cast(), Layout::for_value_raw(self.ptr.as_ptr()))
30643081
}
@@ -3300,6 +3317,28 @@ impl<T: Default> Default for Arc<T> {
33003317
}
33013318
}
33023319

3320+
/// Struct to hold the static `ArcInner` used for empty `Arc<str/CStr/[T]>` as
3321+
/// returned by `Default::default`.
3322+
///
3323+
/// Layout notes:
3324+
/// * `repr(align(16))` so we can use it for `[T]` with `align_of::<T>() <= 16`.
3325+
/// * `repr(C)` so `inner` is at offset 0 (and thus guaranteed to actually be aligned to 16).
3326+
/// * `[u8; 1]` (to be initialized with 0) so it can be used for `Arc<CStr>`.
3327+
#[repr(C, align(16))]
3328+
struct SliceArcInnerForStatic {
3329+
inner: ArcInner<[u8; 1]>,
3330+
}
3331+
#[cfg(not(no_global_oom_handling))]
3332+
const MAX_STATIC_INNER_SLICE_ALIGNMENT: usize = 16;
3333+
3334+
static STATIC_INNER_SLICE: SliceArcInnerForStatic = SliceArcInnerForStatic {
3335+
inner: ArcInner {
3336+
strong: atomic::AtomicUsize::new(1),
3337+
weak: atomic::AtomicUsize::new(1),
3338+
data: [0],
3339+
},
3340+
};
3341+
33033342
#[cfg(not(no_global_oom_handling))]
33043343
#[stable(feature = "more_rc_default_impls", since = "CURRENT_RUSTC_VERSION")]
33053344
impl Default for Arc<str> {
@@ -3324,15 +3363,12 @@ impl Default for Arc<core::ffi::CStr> {
33243363
#[inline]
33253364
fn default() -> Self {
33263365
use core::ffi::CStr;
3327-
static STATIC_INNER_CSTR: ArcInner<[u8; 1]> = ArcInner {
3328-
strong: atomic::AtomicUsize::new(1),
3329-
weak: atomic::AtomicUsize::new(1),
3330-
data: [0],
3331-
};
3332-
let inner: NonNull<ArcInner<[u8]>> = NonNull::from(&STATIC_INNER_CSTR);
3333-
let inner: NonNull<ArcInner<CStr>> = NonNull::new(inner.as_ptr() as *mut ArcInner<CStr>).unwrap();
3366+
let inner: NonNull<ArcInner<[u8]>> = NonNull::from(&STATIC_INNER_SLICE.inner);
3367+
let inner: NonNull<ArcInner<CStr>> =
3368+
NonNull::new(inner.as_ptr() as *mut ArcInner<CStr>).unwrap();
33343369
// `this` semantically is the Arc "owned" by the static, so make sure not to drop it.
3335-
let this: mem::ManuallyDrop<Arc<CStr>> = unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) };
3370+
let this: mem::ManuallyDrop<Arc<CStr>> =
3371+
unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) };
33363372
(*this).clone()
33373373
}
33383374
}
@@ -3345,31 +3381,20 @@ impl<T> Default for Arc<[T]> {
33453381
/// This may or may not share an allocation with other Arcs.
33463382
#[inline]
33473383
fn default() -> Self {
3348-
let alignment_of_t: usize = mem::align_of::<T>();
3349-
// We only make statics for the lowest five alignments.
3350-
// Alignments greater than that will use dynamic allocation.
3351-
macro_rules! use_static_inner_for_alignments {
3352-
($($alignment:literal),*) => {
3353-
$(if alignment_of_t == $alignment {
3354-
// Note: this must be in a new scope because static and type names are unhygenic.
3355-
#[repr(align($alignment))]
3356-
struct Aligned;
3357-
static ALIGNED_STATIC_INNER: ArcInner<Aligned> = ArcInner {
3358-
strong: atomic::AtomicUsize::new(1),
3359-
weak: atomic::AtomicUsize::new(1),
3360-
data: Aligned,
3361-
};
3362-
let inner: NonNull<ArcInner<Aligned>> = NonNull::from(&ALIGNED_STATIC_INNER);
3363-
let inner: NonNull<ArcInner<[T; 0]>> = inner.cast();
3364-
// `this` semantically is the Arc "owned" by the static, so make sure not to drop it.
3365-
let this: mem::ManuallyDrop<Arc<[T; 0]>> = unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) };
3366-
return (*this).clone();
3367-
})*
3368-
};
3384+
if mem::align_of::<T>() <= MAX_STATIC_INNER_SLICE_ALIGNMENT {
3385+
// We take a reference to the whole struct instead of the ArcInner<[u8; 1]> inside it so
3386+
// we don't shrink the range of bytes the ptr is allowed to access under Stacked Borrows.
3387+
// (Miri complains on 32-bit targets with Arc<[Align16]> otherwise.)
3388+
// (Note that NonNull::from(&STATIC_INNER_SLICE.inner) is fine under Tree Borrows.)
3389+
let inner: NonNull<SliceArcInnerForStatic> = NonNull::from(&STATIC_INNER_SLICE);
3390+
let inner: NonNull<ArcInner<[T; 0]>> = inner.cast();
3391+
// `this` semantically is the Arc "owned" by the static, so make sure not to drop it.
3392+
let this: mem::ManuallyDrop<Arc<[T; 0]>> =
3393+
unsafe { mem::ManuallyDrop::new(Arc::from_inner(inner)) };
3394+
return (*this).clone();
33693395
}
3370-
use_static_inner_for_alignments!(1, 2, 4, 8, 16);
33713396

3372-
// If T's alignment is not one of the ones we have a static for, make a new unique allocation.
3397+
// If T's alignment is too large for the static, make a new unique allocation.
33733398
let arr: [T; 0] = [];
33743399
Arc::from(arr)
33753400
}

0 commit comments

Comments
 (0)