Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crux-mir: Avoid pointer arithmetic in vec::IntoIter #1157

Merged
merged 2 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crux-mir/lib/Patches.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ identify all of the code that was changed in each patch.
in sync with `ptr` and `end`, and avoids the need for pointer
arithmetic.

* Avoid pointer arithmetic in `vec` `IntoIter` (last applied: December 11, 2023)

The same problem as above exists for `vec::into_iter::IntoIter::size_hint`,
and we fix it in the same way by adding a `len` field to `IntoIter`.

* Implement `str::as_bytes` via `crucible_identity_transmute` (last applied: May 16, 2023)

This is necessary to avoid a gnarly use of `transmute`.
Expand Down
17 changes: 12 additions & 5 deletions crux-mir/lib/alloc/src/vec/into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub struct IntoIter<
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.
pub(super) len: usize,
}

#[stable(feature = "vec_intoiter_debug", since = "1.13.0")]
Expand Down Expand Up @@ -124,6 +125,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
self.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) };
self.ptr = self.buf.as_ptr();
self.end = self.buf.as_ptr();
self.len = 0;

// Dropping the remaining elements can panic, so this needs to be
// done only after updating the other fields.
Expand All @@ -137,6 +139,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
// 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;
self.len = 0;
}

#[cfg(not(no_global_oom_handling))]
Expand Down Expand Up @@ -191,24 +194,22 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
// `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);
self.len -= 1;

// Make up a value of this ZST.
Some(unsafe { mem::zeroed() })
} else {
let old = self.ptr;
self.ptr = unsafe { self.ptr.add(1) };
self.len -= 1;

Some(unsafe { ptr::read(old) })
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
let exact = if T::IS_ZST {
self.end.addr().wrapping_sub(self.ptr.addr())
} else {
unsafe { self.end.sub_ptr(self.ptr) }
};
let exact = self.len;
(exact, Some(exact))
}

Expand All @@ -223,6 +224,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
// SAFETY: the min() above ensures that step_size is in bounds
self.ptr = unsafe { self.ptr.add(step_size) };
}
self.len -= step_size;
// SAFETY: the min() above ensures that step_size is in bounds
unsafe {
ptr::drop_in_place(to_drop);
Expand Down Expand Up @@ -252,6 +254,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
}

self.end = self.end.wrapping_byte_sub(N);
self.len -= N;
// Safety: ditto
return Ok(unsafe { raw_ary.transpose().assume_init() });
}
Expand All @@ -271,6 +274,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
return unsafe {
ptr::copy_nonoverlapping(self.ptr, raw_ary.as_mut_ptr() as *mut T, N);
self.ptr = self.ptr.add(N);
self.len -= N;
Ok(raw_ary.transpose().assume_init())
};
}
Expand Down Expand Up @@ -302,11 +306,13 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
} else if T::IS_ZST {
// See above for why 'ptr.offset' isn't used
self.end = self.end.wrapping_byte_sub(1);
self.len -= 1;

// Make up a value of this ZST.
Some(unsafe { mem::zeroed() })
} else {
self.end = unsafe { self.end.sub(1) };
self.len -= 1;

Some(unsafe { ptr::read(self.end) })
}
Expand All @@ -322,6 +328,7 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
// SAFETY: same as for advance_by()
self.end = unsafe { self.end.sub(step_size) };
}
self.len -= step_size;
let to_drop = ptr::slice_from_raw_parts_mut(self.end as *mut T, step_size);
// SAFETY: same as for advance_by()
unsafe {
Expand Down
1 change: 1 addition & 0 deletions crux-mir/lib/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2794,6 +2794,7 @@ impl<T, A: Allocator> IntoIterator for Vec<T, A> {
alloc,
ptr: begin,
end,
len: me.len(),
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions crux-mir/test/conc_eval/vec/into_iter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![cfg_attr(not(with_main), no_std)]
// Test iterator for 0-length vec

#[cfg(not(with_main))] extern crate std;
#[cfg(not(with_main))] use std::vec;

pub fn f(count: u32) -> u32 {
let mut v1 = vec![];
(0..count).for_each(|i| v1.push(i));
let mut sum = 0;
for i in v1 {
sum += i;
}
sum
}

pub static ARG: u32 = 0;

#[cfg(with_main)] pub fn main() { println!("{:?}", f(ARG)); }
#[cfg(not(with_main))] #[cfg_attr(crux, crux::test)] fn crux_test() -> u32 { f(ARG) }
Loading