about summary refs log tree commit diff
diff options
context:
space:
mode:
authorBen Kimock <kimockb@gmail.com>2021-12-06 20:44:02 -0500
committerMark Rousskov <mark.simulacrum@gmail.com>2021-12-16 10:31:46 -0500
commit3a0fa0375d9ace4b0a06438e2c7ce2d52d11fc2e (patch)
tree9c28d2626d50714f621ff1f16373d8d8949423d5
parentf8402169aaa12e7bbb9630796a8caec90a3055ca (diff)
downloadrust-3a0fa0375d9ace4b0a06438e2c7ce2d52d11fc2e.tar.gz
rust-3a0fa0375d9ace4b0a06438e2c7ce2d52d11fc2e.zip
Fix SB problems in slice sorting
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.
-rw-r--r--library/core/src/slice/sort.rs37
1 files changed, 20 insertions, 17 deletions
diff --git a/library/core/src/slice/sort.rs b/library/core/src/slice/sort.rs
index 60b39295caf..b5e6083c663 100644
--- a/library/core/src/slice/sort.rs
+++ b/library/core/src/slice/sort.rs
@@ -33,8 +33,8 @@ where
     F: FnMut(&T, &T) -> bool,
 {
     let len = v.len();
-    // SAFETY: The unsafe operations below involves indexing without a bound check (`get_unchecked` and `get_unchecked_mut`)
-    // and copying memory (`ptr::copy_nonoverlapping`).
+    // SAFETY: The unsafe operations below involves indexing without a bounds check (by offsetting a
+    // pointer) and copying memory (`ptr::copy_nonoverlapping`).
     //
     // a. Indexing:
     //  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
             // into the slice.
             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) };
-            ptr::copy_nonoverlapping(v.get_unchecked(1), v.get_unchecked_mut(0), 1);
+            let v = v.as_mut_ptr();
+            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 {
-                if !is_less(v.get_unchecked(i), &*tmp) {
+                if !is_less(&*v.add(i), &*tmp) {
                     break;
                 }
 
                 // 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);
-                hole.dest = v.get_unchecked_mut(i);
+                ptr::copy_nonoverlapping(v.add(i), v.add(i - 1), 1);
+                hole.dest = v.add(i);
             }
             // `hole` gets dropped and thus copies `tmp` into the remaining hole in `v`.
         }
@@ -78,8 +79,8 @@ where
     F: FnMut(&T, &T) -> bool,
 {
     let len = v.len();
-    // SAFETY: The unsafe operations below involves indexing without a bound check (`get_unchecked` and `get_unchecked_mut`)
-    // and copying memory (`ptr::copy_nonoverlapping`).
+    // SAFETY: The unsafe operations below involves indexing without a bound check (by offsetting a
+    // pointer) and copying memory (`ptr::copy_nonoverlapping`).
     //
     // a. Indexing:
     //  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
             // into the slice.
             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) };
-            ptr::copy_nonoverlapping(v.get_unchecked(len - 2), v.get_unchecked_mut(len - 1), 1);
+            let v = v.as_mut_ptr();
+            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() {
-                if !is_less(&*tmp, v.get_unchecked(i)) {
+                if !is_less(&*tmp, &*v.add(i)) {
                     break;
                 }
 
                 // 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);
-                hole.dest = v.get_unchecked_mut(i);
+                ptr::copy_nonoverlapping(v.add(i), v.add(i + 1), 1);
+                hole.dest = v.add(i);
             }
             // `hole` gets dropped and thus copies `tmp` into the remaining hole in `v`.
         }
@@ -302,7 +304,7 @@ where
         if start_l == end_l {
             // Trace `block_l` elements from the left side.
             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;
 
             for i in 0..block_l {
@@ -328,7 +330,7 @@ where
         if start_r == end_r {
             // Trace `block_r` elements from the right side.
             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;
 
             for i in 0..block_r {
@@ -579,7 +581,8 @@ where
 
             // Swap the found pair of out-of-order elements.
             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;
         }
     }