From 4a647a5084fa9100cb506185fa9ee9ed454374ab Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Mon, 1 Jun 2026 12:25:20 +0000 Subject: [PATCH] Don't drop uninit memory when panics --- library/core/src/iter/adapters/map_windows.rs | 31 +++++++----- .../tests/iter/adapters/map_windows.rs | 47 ++++++++++++++++++- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/library/core/src/iter/adapters/map_windows.rs b/library/core/src/iter/adapters/map_windows.rs index bcda315f7a4a0..5c12600eac19d 100644 --- a/library/core/src/iter/adapters/map_windows.rs +++ b/library/core/src/iter/adapters/map_windows.rs @@ -1,5 +1,5 @@ use crate::iter::FusedIterator; -use crate::mem::{MaybeUninit, SizedTypeProperties}; +use crate::mem::{ManuallyDrop, MaybeUninit, SizedTypeProperties}; use crate::{fmt, ptr}; /// An iterator over the mapped windows of another iterator. @@ -30,14 +30,17 @@ struct MapWindowsInner { buffer: Option>, } -// `Buffer` uses two times of space to reduce moves among the iterations. -// `Buffer` is semantically `[MaybeUninit; 2 * N]`. However, due -// to limitations of const generics, we use this different type. Note that -// it has the same underlying memory layout. +/// `Buffer` is semantically `[MaybeUninit; 2 * N]`. This helps +/// reduce moves while iterating. However, due +/// to limitations of const generics, we use this different type. Note that +/// it has the same underlying memory layout. +/// +/// # Safety invariant +/// +/// `self.buffer[self.start..self.start + N]` must be initialized, +/// with all other elements being uninitialized. This also +/// implies that `self.start <= N`. struct Buffer { - // Invariant: `self.buffer[self.start..self.start + N]` is initialized, - // with all other elements being uninitialized. This also - // implies that `self.start <= N`. buffer: [[MaybeUninit; N]; 2], start: usize, } @@ -194,12 +197,18 @@ impl Buffer { impl Clone for Buffer { fn clone(&self) -> Self { - let mut buffer = Buffer { + // Use `ManuallyDrop` until buffer is fully written to avoid dropping uninitialized elements on panic. + // (See `Buffer` rustdoc for safety invariant) + let mut buffer = ManuallyDrop::new(Buffer { buffer: [[const { MaybeUninit::uninit() }; N], [const { MaybeUninit::uninit() }; N]], start: self.start, - }; + }); + + // `clone()` could panic; `ManuallyDrop` guards against that. buffer.as_uninit_array_mut().write(self.as_array_ref().clone()); - buffer + + // We initialized the buffer above, so we are good now + ManuallyDrop::into_inner(buffer) } } diff --git a/library/coretests/tests/iter/adapters/map_windows.rs b/library/coretests/tests/iter/adapters/map_windows.rs index 994ec087e5e8b..fa84f9d1c9cb4 100644 --- a/library/coretests/tests/iter/adapters/map_windows.rs +++ b/library/coretests/tests/iter/adapters/map_windows.rs @@ -5,7 +5,7 @@ use std::sync::atomic::Ordering::SeqCst; mod drop_checks { //! These tests mainly make sure the elements are correctly dropped. - use std::sync::atomic::Ordering::SeqCst; + use std::sync::atomic::Ordering::{Relaxed, SeqCst}; use std::sync::atomic::{AtomicBool, AtomicUsize}; #[derive(Debug)] @@ -143,6 +143,51 @@ mod drop_checks { check::<5>(5, 1); check::<5>(5, 4); } + + /// Regression test for #156501 + #[test] + fn panicking_clone() { + static CLONE_COUNTER: AtomicUsize = AtomicUsize::new(0); + static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0); + + struct PanickingClone(u8); + + impl Clone for PanickingClone { + fn clone(&self) -> Self { + if CLONE_COUNTER.fetch_add(1, Relaxed) == 3 { + panic!( + "⚞(· <:::> ·)⚟ aaaaaah its the turbofish monster!!! its gonna eat us all!!!1!" + ); + } + + Self(self.0) + } + } + + impl Drop for PanickingClone { + fn drop(&mut self) { + assert_eq!(self.0, 42); + + DROP_COUNTER.fetch_add(1, Relaxed); + } + } + + let array = [const { PanickingClone(42) }; 17]; + let mut iter = array.into_iter().map_windows::<_, (), 17>(|_| ()); + + // initialize the buffer + iter.next(); + + let result = std::panic::catch_unwind(|| { + // now do the clones and panic. + // this should't try to drop uninitialized memory + let _ = iter.clone(); + }); + + assert!(result.is_err()); + + assert_eq!(DROP_COUNTER.load(Relaxed), 3); + } } #[test]