Skip to content

Commit e28d8e5

Browse files
committed
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.
1 parent 32ec40c commit e28d8e5

File tree

3 files changed

+25
-12
lines changed

3 files changed

+25
-12
lines changed

library/alloc/src/vec/in_place_collect.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -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>,
@@ -262,7 +263,7 @@ where
262263
);
263264
}
264265

265-
// The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`.
266+
// The ownership of the source allocation and the new `T` values is temporarily moved into `dst_guard`.
266267
// This is safe because
267268
// * `forget_allocation_drop_remaining` immediately forgets the allocation
268269
// before any panic can occur in order to avoid any double free, and then proceeds to drop
@@ -273,7 +274,8 @@ where
273274
// Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
274275
// contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
275276
// module documentation why this is ok anyway.
276-
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap: dst_cap };
277+
let dst_guard =
278+
InPlaceDstDataSrcBufDrop { ptr: dst_buf, len, src_cap, src: PhantomData::<I::Src> };
277279
src.forget_allocation_drop_remaining();
278280

279281
// Adjust the allocation if the alignment didn't match or the source had a capacity in bytes

library/alloc/src/vec/in_place_drop.rs

+19-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,24 @@ 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+
let _drop_allocation = unsafe {
44+
RawVec::<Src>::from_raw_parts_in(self.ptr.cast::<Src>(), self.src_cap, Global)
45+
};
46+
unsafe {
47+
drop_in_place(core::ptr::slice_from_raw_parts_mut::<Dest>(self.ptr, self.len));
48+
}
3849
}
3950
}

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)