Skip to content

Commit 56d11a4

Browse files
authored
Rollup merge of #92092 - saethlin:fix-sort-guards-sb, r=danielhenrymantilla
Drop guards in slice sorting derive src pointers from &mut T, which is invalidated by interior mutation in comparison I tried to run https://github.com/rust-lang/miri-test-libstd on `alloc` with `-Zmiri-track-raw-pointers`, and got a failure on the test `slice::panic_safe`. The test failure has nothing to do with panic safety, it's from how the test tests for panic safety. I minimized the test failure into this very silly program: ```rust use std::cell::Cell; use std::cmp::Ordering; #[derive(Clone)] struct Evil(Cell<usize>); fn main() { let mut input = vec![Evil(Cell::new(0)); 3]; // Hits the bug pattern via CopyOnDrop in core input.sort_unstable_by(|a, _b| { a.0.set(0); Ordering::Less }); // Hits the bug pattern via InsertionHole in alloc input.sort_by(|_a, b| { b.0.set(0); Ordering::Less }); } ``` To fix this, I'm just removing the mutability/uniqueness where it wasn't required.
2 parents 936ce3d + a5a91c8 commit 56d11a4

File tree

2 files changed

+12
-12
lines changed

2 files changed

+12
-12
lines changed

library/alloc/src/slice.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ where
892892
// performance than with the 2nd method.
893893
//
894894
// All methods were benchmarked, and the 3rd showed best results. So we chose that one.
895-
let mut tmp = mem::ManuallyDrop::new(ptr::read(&v[0]));
895+
let tmp = mem::ManuallyDrop::new(ptr::read(&v[0]));
896896

897897
// Intermediate state of the insertion process is always tracked by `hole`, which
898898
// serves two purposes:
@@ -904,7 +904,7 @@ where
904904
// If `is_less` panics at any point during the process, `hole` will get dropped and
905905
// fill the hole in `v` with `tmp`, thus ensuring that `v` still holds every object it
906906
// initially held exactly once.
907-
let mut hole = InsertionHole { src: &mut *tmp, dest: &mut v[1] };
907+
let mut hole = InsertionHole { src: &*tmp, dest: &mut v[1] };
908908
ptr::copy_nonoverlapping(&v[1], &mut v[0], 1);
909909

910910
for i in 2..v.len() {
@@ -920,7 +920,7 @@ where
920920

921921
// When dropped, copies from `src` into `dest`.
922922
struct InsertionHole<T> {
923-
src: *mut T,
923+
src: *const T,
924924
dest: *mut T,
925925
}
926926

library/core/src/slice/sort.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::ptr;
1212

1313
/// When dropped, copies from `src` into `dest`.
1414
struct CopyOnDrop<T> {
15-
src: *mut T,
15+
src: *const T,
1616
dest: *mut T,
1717
}
1818

@@ -54,9 +54,9 @@ where
5454
// Read the first element into a stack-allocated variable. If a following comparison
5555
// operation panics, `hole` will get dropped and automatically write the element back
5656
// into the slice.
57-
let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(0)));
57+
let tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(0)));
5858
let v = v.as_mut_ptr();
59-
let mut hole = CopyOnDrop { src: &mut *tmp, dest: v.add(1) };
59+
let mut hole = CopyOnDrop { src: &*tmp, dest: v.add(1) };
6060
ptr::copy_nonoverlapping(v.add(1), v.add(0), 1);
6161

6262
for i in 2..len {
@@ -100,9 +100,9 @@ where
100100
// Read the last element into a stack-allocated variable. If a following comparison
101101
// operation panics, `hole` will get dropped and automatically write the element back
102102
// into the slice.
103-
let mut tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(len - 1)));
103+
let tmp = mem::ManuallyDrop::new(ptr::read(v.get_unchecked(len - 1)));
104104
let v = v.as_mut_ptr();
105-
let mut hole = CopyOnDrop { src: &mut *tmp, dest: v.add(len - 2) };
105+
let mut hole = CopyOnDrop { src: &*tmp, dest: v.add(len - 2) };
106106
ptr::copy_nonoverlapping(v.add(len - 2), v.add(len - 1), 1);
107107

108108
for i in (0..len - 2).rev() {
@@ -498,8 +498,8 @@ where
498498
// operation panics, the pivot will be automatically written back into the slice.
499499

500500
// SAFETY: `pivot` is a reference to the first element of `v`, so `ptr::read` is safe.
501-
let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
502-
let _pivot_guard = CopyOnDrop { src: &mut *tmp, dest: pivot };
501+
let tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
502+
let _pivot_guard = CopyOnDrop { src: &*tmp, dest: pivot };
503503
let pivot = &*tmp;
504504

505505
// Find the first pair of out-of-order elements.
@@ -551,8 +551,8 @@ where
551551
// Read the pivot into a stack-allocated variable for efficiency. If a following comparison
552552
// operation panics, the pivot will be automatically written back into the slice.
553553
// SAFETY: The pointer here is valid because it is obtained from a reference to a slice.
554-
let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
555-
let _pivot_guard = CopyOnDrop { src: &mut *tmp, dest: pivot };
554+
let tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) });
555+
let _pivot_guard = CopyOnDrop { src: &*tmp, dest: pivot };
556556
let pivot = &*tmp;
557557

558558
// Now partition the slice.

0 commit comments

Comments
 (0)