fix IntoIter::drop on high-alignment ZST

This commit is contained in:
Ralf Jung 2022-12-23 15:08:56 +01:00
parent 14e2fe4039
commit d0f404d77a
3 changed files with 23 additions and 17 deletions

View File

@ -40,7 +40,9 @@ pub struct IntoIter<
// to avoid dropping the allocator twice we need to wrap it into ManuallyDrop
pub(super) alloc: ManuallyDrop<A>,
pub(super) ptr: *const T,
pub(super) end: *const T,
pub(super) end: *const T, // If T is a ZST, this is actually ptr+len. This encoding is picked so that
// ptr == end is a quick test for the Iterator being empty, that works
// for both ZST and non-ZST.
}
#[stable(feature = "vec_intoiter_debug", since = "1.13.0")]
@ -132,7 +134,9 @@ impl<T, A: Allocator> IntoIter<T, A> {
/// Forgets to Drop the remaining elements while still allowing the backing allocation to be freed.
pub(crate) fn forget_remaining_elements(&mut self) {
self.ptr = self.end;
// For th ZST case, it is crucial that we mutate `end` here, not `ptr`.
// `ptr` must stay aligned, while `end` may be unaligned.
self.end = self.ptr;
}
#[cfg(not(no_global_oom_handling))]
@ -184,10 +188,9 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
if self.ptr == self.end {
None
} else if T::IS_ZST {
// purposefully don't use 'ptr.offset' because for
// vectors with 0-size elements this would return the
// same pointer.
self.ptr = self.ptr.wrapping_byte_add(1);
// `ptr` has to stay where it is to remain aligned, so we reduce the length by 1 by
// reducing the `end`.
self.end = self.end.wrapping_byte_sub(1);
// Make up a value of this ZST.
Some(unsafe { mem::zeroed() })
@ -214,10 +217,8 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
let step_size = self.len().min(n);
let to_drop = ptr::slice_from_raw_parts_mut(self.ptr as *mut T, step_size);
if T::IS_ZST {
// SAFETY: due to unchecked casts of unsigned amounts to signed offsets the wraparound
// effectively results in unsigned pointers representing positions 0..usize::MAX,
// which is valid for ZSTs.
self.ptr = self.ptr.wrapping_byte_add(step_size);
// See `next` for why we sub `end` here.
self.end = self.end.wrapping_byte_sub(step_size);
} else {
// SAFETY: the min() above ensures that step_size is in bounds
self.ptr = unsafe { self.ptr.add(step_size) };

View File

@ -639,6 +639,7 @@ Definite bugs found:
* [Data race in `thread::scope`](https://github.com/rust-lang/rust/issues/98498)
* [`regex` incorrectly handling unaligned `Vec<u8>` buffers](https://www.reddit.com/r/rust/comments/vq3mmu/comment/ienc7t0?context=3)
* [Incorrect use of `compare_exchange_weak` in `once_cell`](https://github.com/matklad/once_cell/issues/186)
* [Dropping with unaligned pointers in `vec::IntoIter`](https://github.com/rust-lang/rust/pull/106084)
Violations of [Stacked Borrows] found that are likely bugs (but Stacked Borrows is currently just an experiment):

View File

@ -37,15 +37,19 @@ fn vec_into_iter() -> u8 {
}
fn vec_into_iter_rev() -> u8 {
vec![1, 2, 3, 4].into_iter().map(|x| x * x).fold(0, |x, y| x + y)
vec![1, 2, 3, 4].into_iter().rev().map(|x| x * x).fold(0, |x, y| x + y)
}
fn vec_into_iter_zst() -> usize {
vec![[0u64; 0], [0u64; 0]].into_iter().rev().map(|x| x.len()).sum()
fn vec_into_iter_zst() {
for _ in vec![[0u64; 0]].into_iter() {}
let v = vec![[0u64; 0], [0u64; 0]].into_iter().map(|x| x.len()).sum::<usize>();
assert_eq!(v, 0);
}
fn vec_into_iter_rev_zst() -> usize {
vec![[0u64; 0], [0u64; 0]].into_iter().rev().map(|x| x.len()).sum()
fn vec_into_iter_rev_zst() {
for _ in vec![[0u64; 0]; 5].into_iter().rev() {}
let v = vec![[0u64; 0], [0u64; 0]].into_iter().rev().map(|x| x.len()).sum::<usize>();
assert_eq!(v, 0);
}
fn vec_iter_and_mut() {
@ -150,8 +154,8 @@ fn main() {
assert_eq!(vec_into_iter(), 30);
assert_eq!(vec_into_iter_rev(), 30);
vec_iter_and_mut();
assert_eq!(vec_into_iter_zst(), 0);
assert_eq!(vec_into_iter_rev_zst(), 0);
vec_into_iter_zst();
vec_into_iter_rev_zst();
vec_iter_and_mut_rev();
assert_eq!(make_vec().capacity(), 4);