mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix SB problems in slice sorting

Browse Source 
Most of these problems originate in use of get_unchecked_mut.

When calling ptr::copy_nonoverlapping, using get_unchecked_mut for both
arguments causes the borrow created to make the second pointer to invalid the
first.

The pairs of identical MaybeUninit::slice_as_mut_ptr calls similarly
invalidate each other.

There was also a similar borrow invalidation problem with the use of
slice::get_unchecked_mut to derive the pointer for the CopyOnDrop.
This commit is contained in:
Ben Kimock 2021-12-06 20:44:02 -05:00 committed by Mark Rousskov
parent f8402169aa
commit 3a0fa0375d
1 changed files with 20 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
library/core/src/slice/sort.rs
View File

@ -33,8 +33,8 @@ where
F: FnMut(&T, &T) -> bool, F: FnMut(&T, &T) -> bool,
{ {
let len = v.len(); let len = v.len();
// SAFETY: The unsafe operations below involves indexing without a bound check (`get_unchecked` and `get_unchecked_mut`) // SAFETY: The unsafe operations below involves indexing without a bounds check (by offsetting a
// and copying memory (`ptr::copy_nonoverlapping`). // pointer) and copying memory (`ptr::copy_nonoverlapping`).
// //
// a. Indexing: // a. Indexing:
// 1. We checked the size of the array to >=2. // 1. We checked the size of the array to >=2.
@ -55,17 +55,18 @@ where
// operation panics, `hole` will get dropped and automatically write the element back // operation panics, `hole` will get dropped and automatically write the element back
// into the slice. // into the slice.
let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(0))); let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(0)));
let mut hole = CopyOnDrop { src: &mut *tmp, dest: v.get_unchecked_mut(1) }; let v = v.as_mut_ptr();
ptr::copy_nonoverlapping(v.get_unchecked(1), v.get_unchecked_mut(0), 1); let mut hole = CopyOnDrop { src: &mut *tmp, dest: v.add(1) };
ptr::copy_nonoverlapping(v.add(1), v.add(0), 1);
for i in 2..len { for i in 2..len {
if !is_less(v.get_unchecked(i), &*tmp) { if !is_less(&*v.add(i), &*tmp) {
break; break;
} }
// Move `i`-th element one place to the left, thus shifting the hole to the right. // Move `i`-th element one place to the left, thus shifting the hole to the right.
ptr::copy_nonoverlapping(v.get_unchecked(i), v.get_unchecked_mut(i - 1), 1); ptr::copy_nonoverlapping(v.add(i), v.add(i - 1), 1);
hole.dest = v.get_unchecked_mut(i); hole.dest = v.add(i);
} }
// `hole` gets dropped and thus copies `tmp` into the remaining hole in `v`. // `hole` gets dropped and thus copies `tmp` into the remaining hole in `v`.
} }
@ -78,8 +79,8 @@ where
F: FnMut(&T, &T) -> bool, F: FnMut(&T, &T) -> bool,
{ {
let len = v.len(); let len = v.len();
// SAFETY: The unsafe operations below involves indexing without a bound check (`get_unchecked` and `get_unchecked_mut`) // SAFETY: The unsafe operations below involves indexing without a bound check (by offsetting a
// and copying memory (`ptr::copy_nonoverlapping`). // pointer) and copying memory (`ptr::copy_nonoverlapping`).
// //
// a. Indexing: // a. Indexing:
// 1. We checked the size of the array to >= 2. // 1. We checked the size of the array to >= 2.
@ -100,17 +101,18 @@ where
// operation panics, `hole` will get dropped and automatically write the element back // operation panics, `hole` will get dropped and automatically write the element back
// into the slice. // into the slice.
let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(len - 1))); let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(len - 1)));
let mut hole = CopyOnDrop { src: &mut *tmp, dest: v.get_unchecked_mut(len - 2) }; let v = v.as_mut_ptr();
ptr::copy_nonoverlapping(v.get_unchecked(len - 2), v.get_unchecked_mut(len - 1), 1); let mut hole = CopyOnDrop { src: &mut *tmp, dest: v.add(len - 2) };
ptr::copy_nonoverlapping(v.add(len - 2), v.add(len - 1), 1);
for i in (0..len - 2).rev() { for i in (0..len - 2).rev() {
if !is_less(&*tmp, v.get_unchecked(i)) { if !is_less(&*tmp, &*v.add(i)) {
break; break;
} }
// Move `i`-th element one place to the right, thus shifting the hole to the left. // Move `i`-th element one place to the right, thus shifting the hole to the left.
ptr::copy_nonoverlapping(v.get_unchecked(i), v.get_unchecked_mut(i + 1), 1); ptr::copy_nonoverlapping(v.add(i), v.add(i + 1), 1);
hole.dest = v.get_unchecked_mut(i); hole.dest = v.add(i);
} }
// `hole` gets dropped and thus copies `tmp` into the remaining hole in `v`. // `hole` gets dropped and thus copies `tmp` into the remaining hole in `v`.
} }
@ -302,7 +304,7 @@ where
if start_l == end_l { if start_l == end_l {
// Trace `block_l` elements from the left side. // Trace `block_l` elements from the left side.
start_l = MaybeUninit::slice_as_mut_ptr(&mut offsets_l); start_l = MaybeUninit::slice_as_mut_ptr(&mut offsets_l);
end_l = MaybeUninit::slice_as_mut_ptr(&mut offsets_l); end_l = start_l;
let mut elem = l; let mut elem = l;
for i in 0..block_l { for i in 0..block_l {
@ -328,7 +330,7 @@ where
if start_r == end_r { if start_r == end_r {
// Trace `block_r` elements from the right side. // Trace `block_r` elements from the right side.
start_r = MaybeUninit::slice_as_mut_ptr(&mut offsets_r); start_r = MaybeUninit::slice_as_mut_ptr(&mut offsets_r);
end_r = MaybeUninit::slice_as_mut_ptr(&mut offsets_r); end_r = start_r;
let mut elem = r; let mut elem = r;
for i in 0..block_r { for i in 0..block_r {
@ -579,7 +581,8 @@ where
// Swap the found pair of out-of-order elements. // Swap the found pair of out-of-order elements.
r -= 1; r -= 1;
ptr::swap(v.get_unchecked_mut(l), v.get_unchecked_mut(r)); let ptr = v.as_mut_ptr();
ptr::swap(ptr.add(l), ptr.add(r));
l += 1; l += 1;
} }
} }