diff --git a/library/core/src/slice/sort.rs b/library/core/src/slice/sort.rs index 8a31388fbdb..60b39295caf 100644 --- a/library/core/src/slice/sort.rs +++ b/library/core/src/slice/sort.rs @@ -6,8 +6,6 @@ //! Unstable sorting is compatible with libcore because it doesn't allocate memory, unlike our //! stable sorting implementation. -// ignore-tidy-undocumented-unsafe - use crate::cmp; use crate::mem::{self, MaybeUninit}; use crate::ptr; @@ -291,6 +289,9 @@ where } else if start_r < end_r { block_l = rem; } else { + // There were the same number of elements to switch on both blocks during the last + // iteration, so there are no remaining elements on either block. Cover the remaining + // items with roughly equally-sized blocks. block_l = rem / 2; block_r = rem - block_l; } @@ -437,6 +438,17 @@ where // Move its remaining out-of-order elements to the far right. debug_assert_eq!(width(l, r), block_l); while start_l < end_l { + // remaining-elements-safety + // SAFETY: while the loop condition holds there are still elements in `offsets_l`, so it + // is safe to point `end_l` to the previous element. + // + // The `ptr::swap` is safe if both its arguments are valid for reads and writes: + // - Per the debug assert above, the distance between `l` and `r` is `block_l` + // elements, so there can be at most `block_l` remaining offsets between `start_l` + // and `end_l`. This means `r` will be moved at most `block_l` steps back, which + // makes the `r.offset` calls valid (at that point `l == r`). + // - `offsets_l` contains valid offsets into `v` collected during the partitioning of + // the last block, so the `l.offset` calls are valid. unsafe { end_l = end_l.offset(-1); ptr::swap(l.offset(*end_l as isize), r.offset(-1)); @@ -449,6 +461,7 @@ where // Move its remaining out-of-order elements to the far left. debug_assert_eq!(width(l, r), block_r); while start_r < end_r { + // SAFETY: See the reasoning in [remaining-elements-safety]. unsafe { end_r = end_r.offset(-1); ptr::swap(l, r.offset(-(*end_r as isize) - 1)); @@ -481,6 +494,8 @@ where // Read the pivot into a stack-allocated variable for efficiency. If a following comparison // operation panics, the pivot will be automatically written back into the slice. + + // SAFETY: `pivot` is a reference to the first element of `v`, so `ptr::read` is safe. let mut tmp = mem::ManuallyDrop::new(unsafe { ptr::read(pivot) }); let _pivot_guard = CopyOnDrop { src: &mut *tmp, dest: pivot }; let pivot = &*tmp; @@ -646,6 +661,12 @@ where if len >= 8 { // Swaps indices so that `v[a] <= v[b]`. + // SAFETY: `len >= 8` so there are at least two elements in the neighborhoods of + // `a`, `b` and `c`. This means the three calls to `sort_adjacent` result in + // corresponding calls to `sort3` with valid 3-item neighborhoods around each + // pointer, which in turn means the calls to `sort2` are done with valid + // references. Thus the `v.get_unchecked` calls are safe, as is the `ptr::swap` + // call. let mut sort2 = |a: &mut usize, b: &mut usize| unsafe { if is_less(v.get_unchecked(*b), v.get_unchecked(*a)) { ptr::swap(a, b);