Skip to content

Commit 84e9608

Browse files
committed
Fix leak in Vec::extend_from_within
Previously vec's len was updated only after full copy, making the method leak if T::clone panic!s. This commit makes `Vec::extend_from_within` (or, more accurately, it's `T: Clone` specialization) update vec's len on every iteration, fixing the issue. `T: Copy` specialization was not affected by the issue b/c it doesn't call user specified code (as, e.g. `T::clone`), and instead calls `ptr::copy_nonoverlapping`.
1 parent ec7f258 commit 84e9608

File tree

1 file changed

+27
-16
lines changed

1 file changed

+27
-16
lines changed

library/alloc/src/vec/mod.rs

+27-16
Original file line numberDiff line numberDiff line change
@@ -1942,6 +1942,16 @@ impl<T, A: Allocator> Vec<T, A> {
19421942
#[unstable(feature = "vec_split_at_spare", issue = "81944")]
19431943
#[inline]
19441944
pub fn split_at_spare_mut(&mut self) -> (&mut [T], &mut [MaybeUninit<T>]) {
1945+
// SAFETY:
1946+
// - len is ignored and so never changed
1947+
let (init, spare, _) = unsafe{ self.split_at_spare_mut_with_len() };
1948+
(init, spare)
1949+
}
1950+
1951+
/// Safety: changing returned .2 (&mut usize) is considered the same as calling `.set_len(_)`.
1952+
///
1953+
/// This method is used to have unique access to all vec parts at once in `extend_from_within`.
1954+
unsafe fn split_at_spare_mut_with_len(&mut self) -> (&mut [T], &mut [MaybeUninit<T>], &mut usize) {
19451955
let Range { start: ptr, end: spare_ptr } = self.as_mut_ptr_range();
19461956
let spare_ptr = spare_ptr.cast::<MaybeUninit<T>>();
19471957
let spare_len = self.buf.capacity() - self.len;
@@ -1953,9 +1963,9 @@ impl<T, A: Allocator> Vec<T, A> {
19531963
let initialized = slice::from_raw_parts_mut(ptr, self.len);
19541964
let spare = slice::from_raw_parts_mut(spare_ptr, spare_len);
19551965

1956-
(initialized, spare)
1966+
(initialized, spare, &mut self.len)
19571967
}
1958-
}
1968+
}
19591969
}
19601970

19611971
impl<T: Clone, A: Allocator> Vec<T, A> {
@@ -2165,22 +2175,23 @@ trait ExtendFromWithinSpec {
21652175

21662176
impl<T: Clone, A: Allocator> ExtendFromWithinSpec for Vec<T, A> {
21672177
default unsafe fn spec_extend_from_within(&mut self, src: Range<usize>) {
2168-
let initialized = {
2169-
let (this, spare) = self.split_at_spare_mut();
2170-
2171-
// SAFETY:
2172-
// - caller guaratees that src is a valid index
2173-
let to_clone = unsafe { this.get_unchecked(src) };
2174-
2175-
to_clone.iter().cloned().zip(spare.iter_mut()).map(|(e, s)| s.write(e)).count()
2176-
};
2178+
// SAFETY:
2179+
// - len is increased only after initializing elements
2180+
let (this, spare, len) = unsafe { self.split_at_spare_mut_with_len() };
21772181

21782182
// SAFETY:
2179-
// - elements were just initialized
2180-
unsafe {
2181-
let new_len = self.len() + initialized;
2182-
self.set_len(new_len);
2183-
}
2183+
// - caller guaratees that src is a valid index
2184+
let to_clone = unsafe { this.get_unchecked(src) };
2185+
2186+
to_clone
2187+
.iter()
2188+
.cloned()
2189+
.zip(spare.iter_mut())
2190+
.map(|(src, dst)| dst.write(src))
2191+
// Note:
2192+
// - Element was just initialized with `MaybeUninit::write`, so it's ok to increace len
2193+
// - len is increased after each element to prevent leaks (see issue #82533)
2194+
.for_each(|_| *len += 1);
21842195
}
21852196
}
21862197

0 commit comments

Comments
 (0)