Skip to content

Commit 4b02c24

Browse files
authored
Rollup merge of rust-lang#120145 - the8472:fix-inplace-dest-drop, r=cuviper
fix: Drop guard was deallocating with the incorrect size InPlaceDstBufDrop holds onto the allocation before the shrinking happens which means it must deallocate the destination elements but the source allocation. Thanks ``@cuviper`` for spotting this.
2 parents 3f10dc7 + 5796b3c commit 4b02c24

File tree

3 files changed

+25
-13
lines changed

3 files changed

+25
-13
lines changed

library/alloc/src/vec/in_place_collect.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
//! This is handled by the [`InPlaceDrop`] guard for sink items (`U`) and by
7373
//! [`vec::IntoIter::forget_allocation_drop_remaining()`] for remaining source items (`T`).
7474
//!
75-
//! If dropping any remaining source item (`T`) panics then [`InPlaceDstBufDrop`] will handle dropping
75+
//! If dropping any remaining source item (`T`) panics then [`InPlaceDstDataSrcBufDrop`] will handle dropping
7676
//! the already collected sink items (`U`) and freeing the allocation.
7777
//!
7878
//! [`vec::IntoIter::forget_allocation_drop_remaining()`]: super::IntoIter::forget_allocation_drop_remaining()
@@ -158,11 +158,12 @@ use crate::alloc::{handle_alloc_error, Global};
158158
use core::alloc::Allocator;
159159
use core::alloc::Layout;
160160
use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce};
161+
use core::marker::PhantomData;
161162
use core::mem::{self, ManuallyDrop, SizedTypeProperties};
162163
use core::num::NonZeroUsize;
163164
use core::ptr::{self, NonNull};
164165

165-
use super::{InPlaceDrop, InPlaceDstBufDrop, SpecFromIter, SpecFromIterNested, Vec};
166+
use super::{InPlaceDrop, InPlaceDstDataSrcBufDrop, SpecFromIter, SpecFromIterNested, Vec};
166167

167168
const fn in_place_collectible<DEST, SRC>(
168169
step_merge: Option<NonZeroUsize>,
@@ -265,7 +266,7 @@ where
265266
);
266267
}
267268

268-
// The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`.
269+
// The ownership of the source allocation and the new `T` values is temporarily moved into `dst_guard`.
269270
// This is safe because
270271
// * `forget_allocation_drop_remaining` immediately forgets the allocation
271272
// before any panic can occur in order to avoid any double free, and then proceeds to drop
@@ -276,7 +277,8 @@ where
276277
// Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
277278
// contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
278279
// module documentation why this is ok anyway.
279-
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap: dst_cap };
280+
let dst_guard =
281+
InPlaceDstDataSrcBufDrop { ptr: dst_buf, len, src_cap, src: PhantomData::<I::Src> };
280282
src.forget_allocation_drop_remaining();
281283

282284
// Adjust the allocation if the source had a capacity in bytes that wasn't a multiple

library/alloc/src/vec/in_place_drop.rs

+18-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1-
use core::ptr::{self};
1+
use core::marker::PhantomData;
2+
use core::ptr::{self, drop_in_place};
23
use core::slice::{self};
34

5+
use crate::alloc::Global;
6+
use crate::raw_vec::RawVec;
7+
48
// A helper struct for in-place iteration that drops the destination slice of iteration,
59
// i.e. the head. The source slice (the tail) is dropped by IntoIter.
610
pub(super) struct InPlaceDrop<T> {
@@ -23,17 +27,23 @@ impl<T> Drop for InPlaceDrop<T> {
2327
}
2428
}
2529

26-
// A helper struct for in-place collection that drops the destination allocation and elements,
27-
// to avoid leaking them if some other destructor panics.
28-
pub(super) struct InPlaceDstBufDrop<T> {
29-
pub(super) ptr: *mut T,
30+
// A helper struct for in-place collection that drops the destination items together with
31+
// the source allocation - i.e. before the reallocation happened - to avoid leaking them
32+
// if some other destructor panics.
33+
pub(super) struct InPlaceDstDataSrcBufDrop<Src, Dest> {
34+
pub(super) ptr: *mut Dest,
3035
pub(super) len: usize,
31-
pub(super) cap: usize,
36+
pub(super) src_cap: usize,
37+
pub(super) src: PhantomData<Src>,
3238
}
3339

34-
impl<T> Drop for InPlaceDstBufDrop<T> {
40+
impl<Src, Dest> Drop for InPlaceDstDataSrcBufDrop<Src, Dest> {
3541
#[inline]
3642
fn drop(&mut self) {
37-
unsafe { super::Vec::from_raw_parts(self.ptr, self.len, self.cap) };
43+
unsafe {
44+
let _drop_allocation =
45+
RawVec::<Src>::from_raw_parts_in(self.ptr.cast::<Src>(), self.src_cap, Global);
46+
drop_in_place(core::ptr::slice_from_raw_parts_mut::<Dest>(self.ptr, self.len));
47+
};
3848
}
3949
}

library/alloc/src/vec/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ use self::set_len_on_drop::SetLenOnDrop;
123123
mod set_len_on_drop;
124124

125125
#[cfg(not(no_global_oom_handling))]
126-
use self::in_place_drop::{InPlaceDrop, InPlaceDstBufDrop};
126+
use self::in_place_drop::{InPlaceDrop, InPlaceDstDataSrcBufDrop};
127127

128128
#[cfg(not(no_global_oom_handling))]
129129
mod in_place_drop;

0 commit comments

Comments
 (0)