| From 3834e7b7393bf1a0d7df02ccd1d2e896c1465769 Mon Sep 17 00:00:00 2001 |
| From: The8472 <git@infinite-source.de> |
| Date: Mon, 29 Mar 2021 04:22:34 +0200 |
| Subject: [PATCH 1/2] add testcase for double-drop during Vec in-place |
| collection |
| |
| --- |
| library/alloc/tests/vec.rs | 38 +++++++++++++++++++++++++++++++++++++- |
| 1 file changed, 37 insertions(+), 1 deletion(-) |
| |
| diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs |
| index 5c7ff67bc621..4cdb7eefcdf1 100644 |
| --- a/library/alloc/tests/vec.rs |
| +++ b/library/alloc/tests/vec.rs |
| @@ -954,7 +954,7 @@ fn test_from_iter_specialization_head_tail_drop() { |
| } |
| |
| #[test] |
| -fn test_from_iter_specialization_panic_drop() { |
| +fn test_from_iter_specialization_panic_during_iteration_drops() { |
| let drop_count: Vec<_> = (0..=2).map(|_| Rc::new(())).collect(); |
| let src: Vec<_> = drop_count.iter().cloned().collect(); |
| let iter = src.into_iter(); |
| @@ -977,6 +977,42 @@ fn test_from_iter_specialization_panic_drop() { |
| ); |
| } |
| |
| +#[test] |
| +fn test_from_iter_specialization_panic_during_drop_leaks() { |
| + static mut DROP_COUNTER: usize = 0; |
| + |
| + #[derive(Debug)] |
| + enum Droppable { |
| + DroppedTwice(Box<i32>), |
| + PanicOnDrop, |
| + } |
| + |
| + impl Drop for Droppable { |
| + fn drop(&mut self) { |
| + match self { |
| + Droppable::DroppedTwice(_) => { |
| + unsafe { |
| + DROP_COUNTER += 1; |
| + } |
| + println!("Dropping!") |
| + } |
| + Droppable::PanicOnDrop => { |
| + if !std::thread::panicking() { |
| + panic!(); |
| + } |
| + } |
| + } |
| + } |
| + } |
| + |
| + let _ = std::panic::catch_unwind(AssertUnwindSafe(|| { |
| + let v = vec![Droppable::DroppedTwice(Box::new(123)), Droppable::PanicOnDrop]; |
| + let _ = v.into_iter().take(0).collect::<Vec<_>>(); |
| + })); |
| + |
| + assert_eq!(unsafe { DROP_COUNTER }, 1); |
| +} |
| + |
| #[test] |
| fn test_cow_from() { |
| let borrowed: &[_] = &["borrowed", "(slice)"]; |
| -- |
| 2.31.1 |
| |
| |
| From 8e2706343e1ce1c5a2d3a2ceaaaa010aaeb21d93 Mon Sep 17 00:00:00 2001 |
| From: The8472 <git@infinite-source.de> |
| Date: Mon, 29 Mar 2021 04:22:48 +0200 |
| Subject: [PATCH 2/2] fix double-drop in in-place collect specialization |
| |
| --- |
| library/alloc/src/vec/into_iter.rs | 27 ++++++++++++++------- |
| library/alloc/src/vec/source_iter_marker.rs | 4 +-- |
| 2 files changed, 20 insertions(+), 11 deletions(-) |
| |
| diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs |
| index f131d06bb18f..74adced53f6d 100644 |
| --- a/library/alloc/src/vec/into_iter.rs |
| +++ b/library/alloc/src/vec/into_iter.rs |
| @@ -85,20 +85,29 @@ fn as_raw_mut_slice(&mut self) -> *mut [T] { |
| ptr::slice_from_raw_parts_mut(self.ptr as *mut T, self.len()) |
| } |
| |
| - pub(super) fn drop_remaining(&mut self) { |
| - unsafe { |
| - ptr::drop_in_place(self.as_mut_slice()); |
| - } |
| - self.ptr = self.end; |
| - } |
| + /// Drops remaining elements and relinquishes the backing allocation. |
| + /// |
| + /// This is roughly equivalent to the following, but more efficient |
| + /// |
| + /// ``` |
| + /// # let mut into_iter = Vec::<u8>::with_capacity(10).into_iter(); |
| + /// (&mut into_iter).for_each(core::mem::drop); |
| + /// unsafe { core::ptr::write(&mut into_iter, Vec::new().into_iter()); } |
| + /// ``` |
| + pub(super) fn forget_allocation_drop_remaining(&mut self) { |
| + let remaining = self.as_raw_mut_slice(); |
| |
| - /// Relinquishes the backing allocation, equivalent to |
| - /// `ptr::write(&mut self, Vec::new().into_iter())` |
| - pub(super) fn forget_allocation(&mut self) { |
| + // overwrite the individual fields instead of creating a new |
| + // struct and then overwriting &mut self. |
| + // this creates less assembly |
| self.cap = 0; |
| self.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; |
| self.ptr = self.buf.as_ptr(); |
| self.end = self.buf.as_ptr(); |
| + |
| + unsafe { |
| + ptr::drop_in_place(remaining); |
| + } |
| } |
| } |
| |
| diff --git a/library/alloc/src/vec/source_iter_marker.rs b/library/alloc/src/vec/source_iter_marker.rs |
| index 8c0e95559fa1..9301f7a5184e 100644 |
| --- a/library/alloc/src/vec/source_iter_marker.rs |
| +++ b/library/alloc/src/vec/source_iter_marker.rs |
| @@ -78,9 +78,9 @@ impl<T, I> SpecFromIter<T, I> for Vec<T> |
| } |
| |
| // drop any remaining values at the tail of the source |
| - src.drop_remaining(); |
| // but prevent drop of the allocation itself once IntoIter goes out of scope |
| - src.forget_allocation(); |
| + // if the drop panics then we also leak any elements collected into dst_buf |
| + src.forget_allocation_drop_remaining(); |
| |
| let vec = unsafe { |
| let len = dst.offset_from(dst_buf) as usize; |
| -- |
| 2.31.1 |
| |