Skip to content

Commit

Permalink
Rollup merge of rust-lang#106950 - the8472:fix-splice-miri, r=cuviper
Browse files Browse the repository at this point in the history
Don't do pointer arithmetic on pointers to deallocated memory

vec::Splice can invalidate the slice::Iter inside vec::Drain. So we replace them with dangling pointers which, unlike ones to deallocated memory, are allowed.

Fixes miri test failures.
Fixes rust-lang/miri#2759
  • Loading branch information
compiler-errors authored Jan 18, 2023
2 parents b9ce633 + 2d54b7c commit ba3c3d9
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
6 changes: 3 additions & 3 deletions library/alloc/src/vec/drain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ impl<T, A: Allocator> Drop for Drain<'_, T, A> {
}

// as_slice() must only be called when iter.len() is > 0 because
// vec::Splice modifies vec::Drain fields and may grow the vec which would invalidate
// the iterator's internal pointers. Creating a reference to deallocated memory
// is invalid even when it is zero-length
// it also gets touched by vec::Splice which may turn it into a dangling pointer
// which would make it and the vec pointer point to different allocations which would
// lead to invalid pointer arithmetic below.
let drop_ptr = iter.as_slice().as_ptr();

unsafe {
Expand Down
6 changes: 6 additions & 0 deletions library/alloc/src/vec/splice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ impl<I: Iterator, A: Allocator> ExactSizeIterator for Splice<'_, I, A> {}
impl<I: Iterator, A: Allocator> Drop for Splice<'_, I, A> {
fn drop(&mut self) {
self.drain.by_ref().for_each(drop);
// At this point draining is done and the only remaining tasks are splicing
// and moving things into the final place.
// Which means we can replace the slice::Iter with pointers that won't point to deallocated
// memory, so that Drain::drop is still allowed to call iter.len(), otherwise it would break
// the ptr.sub_ptr contract.
self.drain.iter = (&[]).iter();

unsafe {
if self.drain.tail_len == 0 {
Expand Down
6 changes: 6 additions & 0 deletions src/tools/miri/tests/pass/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ fn reverse() {
assert!(v[0].0 == 49);
}

fn miri_issue_2759() {
let mut input = "1".to_string();
input.replace_range(0..0, "0");
}

fn main() {
assert_eq!(vec_reallocate().len(), 5);

Expand Down Expand Up @@ -191,4 +196,5 @@ fn main() {
swap();
swap_remove();
reverse();
miri_issue_2759();
}

0 comments on commit ba3c3d9

Please sign in to comment.