Skip to content

Commit

Permalink
crux-mir: Avoid pointer arithmetic in vec IntoIter
Browse files Browse the repository at this point in the history
  • Loading branch information
qsctr committed Dec 12, 2023
1 parent a8839cd commit f933e70
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
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

0 comments on commit f933e70

Please sign in to comment.