diff --git a/library/core/src/iter/adapters/map_windows.rs b/library/core/src/iter/adapters/map_windows.rs index 097a0745c61c7..958a410682694 100644 --- a/library/core/src/iter/adapters/map_windows.rs +++ b/library/core/src/iter/adapters/map_windows.rs @@ -194,11 +194,19 @@ impl Buffer { impl Clone for Buffer { fn clone(&self) -> Self { + // Clone the active window into a stack-local first: if `T::clone` + // panics partway through, no `Buffer` has been constructed yet, so + // `Buffer::drop` cannot run over uninitialized slots. + let array = self.as_array_ref().clone(); + + // From here through `MaybeUninit::write` below, `buffer` exists with + // its `Drop` armed over `[start..start + N]` uninitialized slots. + // This is sound only because no panic-capable code runs in between. let mut buffer = Buffer { buffer: [[const { MaybeUninit::uninit() }; N], [const { MaybeUninit::uninit() }; N]], start: self.start, }; - buffer.as_uninit_array_mut().write(self.as_array_ref().clone()); + buffer.as_uninit_array_mut().write(array); buffer } } diff --git a/library/coretests/tests/iter/adapters/map_windows.rs b/library/coretests/tests/iter/adapters/map_windows.rs index 994ec087e5e8b..78922aad553a3 100644 --- a/library/coretests/tests/iter/adapters/map_windows.rs +++ b/library/coretests/tests/iter/adapters/map_windows.rs @@ -76,6 +76,65 @@ mod drop_checks { info.check(); } + /// `DropCheck`-style element whose `Clone::clone` panics on the + /// `panic_on`-th call (1-indexed). Used to exercise panic-safety of + /// adapters that internally clone an active window of `T`s. + struct ClonePanicker<'a> { + info: &'a DropInfo, + clone_count: &'a AtomicUsize, + panic_on: usize, + was_dropped: bool, + } + + impl<'a> ClonePanicker<'a> { + fn new(info: &'a DropInfo, clone_count: &'a AtomicUsize, panic_on: usize) -> Self { + info.alive_count.fetch_add(1, SeqCst); + Self { info, clone_count, panic_on, was_dropped: false } + } + } + + impl<'a> Clone for ClonePanicker<'a> { + fn clone(&self) -> Self { + let n = self.clone_count.fetch_add(1, SeqCst) + 1; + if n == self.panic_on { + panic!("intended panic in clone"); + } + Self::new(self.info, self.clone_count, self.panic_on) + } + } + + impl Drop for ClonePanicker<'_> { + fn drop(&mut self) { + if self.was_dropped { + self.info.dropped_twice.store(true, SeqCst); + } + self.was_dropped = true; + self.info.alive_count.fetch_sub(1, SeqCst); + } + } + + /// Build a `map_windows::<_, _, N>` over `source_len` `ClonePanicker`s, + /// advance the iterator `advance` times (so the internal `Buffer`'s + /// `start` reaches the desired offset), then assert that cloning the + /// iterator panics on the `panic_on`-th `Clone::clone` call. + /// `check_drops` then verifies no double-drop and no leak. + #[track_caller] + fn check_clone_panic(source_len: usize, advance: usize, panic_on: usize) { + check_drops(|info| { + let clone_count = AtomicUsize::new(0); + let mut iter = (0..source_len) + .map(|_| ClonePanicker::new(info, &clone_count, panic_on)) + .map_windows(|_: &[_; N]| ()); + for _ in 0..advance { + iter.next(); + } + let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let _clone = iter.clone(); + })); + assert!(r.is_err(), "clone should have panicked at element {panic_on}"); + }); + } + #[test] fn no_iter_panic_n1() { check::<1>(0, 100); @@ -143,6 +202,39 @@ mod drop_checks { check::<5>(5, 1); check::<5>(5, 4); } + + /// Regression test for : + /// a panic inside `Clone::clone` during `Buffer::clone` must not drop + /// uninitialized slots and must not double-drop already-cloned elements. + #[test] + fn panic_in_clone() { + // `start == 0` (initial buffer state after the first `next()`). + // Panic at every position within the window for several window sizes. + check_clone_panic::<1>(1, 1, 1); + + check_clone_panic::<2>(2, 1, 1); + check_clone_panic::<2>(2, 1, 2); + + check_clone_panic::<3>(3, 1, 1); + check_clone_panic::<3>(3, 1, 3); + + check_clone_panic::<4>(4, 1, 1); + check_clone_panic::<4>(4, 1, 2); + check_clone_panic::<4>(4, 1, 3); + check_clone_panic::<4>(4, 1, 4); + + // `start == N` (buffer half-wrapped: `as_uninit_array_mut()` writes + // into the second half of the backing storage). This is a + // physically distinct destination region from the `start == 0` case. + check_clone_panic::<2>(4, 3, 1); + check_clone_panic::<2>(4, 3, 2); + + check_clone_panic::<3>(6, 4, 1); + check_clone_panic::<3>(6, 4, 3); + + check_clone_panic::<4>(8, 5, 1); + check_clone_panic::<4>(8, 5, 4); + } } #[test]