From 5796b3c16733f3d9cb2f3476ed1dc0fe05b14e25 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Fri, 19 Jan 2024 22:42:52 +0100 Subject: [PATCH] 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. --- library/alloc/src/vec/in_place_collect.rs | 10 +++++---- library/alloc/src/vec/in_place_drop.rs | 26 ++++++++++++++++------- library/alloc/src/vec/mod.rs | 2 +- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/library/alloc/src/vec/in_place_collect.rs b/library/alloc/src/vec/in_place_collect.rs index ec5f32539f2c2..633ca8c99b408 100644 --- a/library/alloc/src/vec/in_place_collect.rs +++ b/library/alloc/src/vec/in_place_collect.rs @@ -72,7 +72,7 @@ //! This is handled by the [`InPlaceDrop`] guard for sink items (`U`) and by //! [`vec::IntoIter::forget_allocation_drop_remaining()`] for remaining source items (`T`). //! -//! If dropping any remaining source item (`T`) panics then [`InPlaceDstBufDrop`] will handle dropping +//! If dropping any remaining source item (`T`) panics then [`InPlaceDstDataSrcBufDrop`] will handle dropping //! the already collected sink items (`U`) and freeing the allocation. //! //! [`vec::IntoIter::forget_allocation_drop_remaining()`]: super::IntoIter::forget_allocation_drop_remaining() @@ -158,11 +158,12 @@ use crate::alloc::{handle_alloc_error, Global}; use core::alloc::Allocator; use core::alloc::Layout; use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce}; +use core::marker::PhantomData; use core::mem::{self, ManuallyDrop, SizedTypeProperties}; use core::num::NonZeroUsize; use core::ptr::{self, NonNull}; -use super::{InPlaceDrop, InPlaceDstBufDrop, SpecFromIter, SpecFromIterNested, Vec}; +use super::{InPlaceDrop, InPlaceDstDataSrcBufDrop, SpecFromIter, SpecFromIterNested, Vec}; const fn in_place_collectible( step_merge: Option, @@ -262,7 +263,7 @@ where ); } - // The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`. + // The ownership of the source allocation and the new `T` values is temporarily moved into `dst_guard`. // This is safe because // * `forget_allocation_drop_remaining` immediately forgets the allocation // before any panic can occur in order to avoid any double free, and then proceeds to drop @@ -273,7 +274,8 @@ where // Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce // contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the // module documentation why this is ok anyway. - let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap: dst_cap }; + let dst_guard = + InPlaceDstDataSrcBufDrop { ptr: dst_buf, len, src_cap, src: PhantomData:: }; src.forget_allocation_drop_remaining(); // Adjust the allocation if the alignment didn't match or the source had a capacity in bytes diff --git a/library/alloc/src/vec/in_place_drop.rs b/library/alloc/src/vec/in_place_drop.rs index 25ca33c6a7bf0..40a540b57fc22 100644 --- a/library/alloc/src/vec/in_place_drop.rs +++ b/library/alloc/src/vec/in_place_drop.rs @@ -1,6 +1,10 @@ -use core::ptr::{self}; +use core::marker::PhantomData; +use core::ptr::{self, drop_in_place}; use core::slice::{self}; +use crate::alloc::Global; +use crate::raw_vec::RawVec; + // A helper struct for in-place iteration that drops the destination slice of iteration, // i.e. the head. The source slice (the tail) is dropped by IntoIter. pub(super) struct InPlaceDrop { @@ -23,17 +27,23 @@ impl Drop for InPlaceDrop { } } -// A helper struct for in-place collection that drops the destination allocation and elements, -// to avoid leaking them if some other destructor panics. -pub(super) struct InPlaceDstBufDrop { - pub(super) ptr: *mut T, +// A helper struct for in-place collection that drops the destination items together with +// the source allocation - i.e. before the reallocation happened - to avoid leaking them +// if some other destructor panics. +pub(super) struct InPlaceDstDataSrcBufDrop { + pub(super) ptr: *mut Dest, pub(super) len: usize, - pub(super) cap: usize, + pub(super) src_cap: usize, + pub(super) src: PhantomData, } -impl Drop for InPlaceDstBufDrop { +impl Drop for InPlaceDstDataSrcBufDrop { #[inline] fn drop(&mut self) { - unsafe { super::Vec::from_raw_parts(self.ptr, self.len, self.cap) }; + unsafe { + let _drop_allocation = + RawVec::::from_raw_parts_in(self.ptr.cast::(), self.src_cap, Global); + drop_in_place(core::ptr::slice_from_raw_parts_mut::(self.ptr, self.len)); + }; } } diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 35ea97bfe609c..48daec152286b 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -123,7 +123,7 @@ use self::set_len_on_drop::SetLenOnDrop; mod set_len_on_drop; #[cfg(not(no_global_oom_handling))] -use self::in_place_drop::{InPlaceDrop, InPlaceDstBufDrop}; +use self::in_place_drop::{InPlaceDrop, InPlaceDstDataSrcBufDrop}; #[cfg(not(no_global_oom_handling))] mod in_place_drop;