Skip to content

Commit

Permalink
Auto merge of #120863 - saethlin:slice-get-checked, r=<try>
Browse files Browse the repository at this point in the history 
Improve precondition checks for unchecked slice indexing

r? `@ghost`
  • Loading branch information
@bors
bors committed Feb 10, 2024
2 parents d44e3b9 a715c46 commit 8389ea2
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 296 deletions.
85 changes: 56 additions & 29 deletions library/core/src/slice/index.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 1,9 @@
//! Indexing implementations for `[T]`.

use crate::intrinsics::assert_unsafe_precondition;
use crate::intrinsics::const_eval_select;
use crate::intrinsics::unchecked_sub;
use crate::ops;
use crate::panic::debug_assert_nounwind;
use crate::ptr;

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -225,28 225,36 @@ unsafe impl<T> SliceIndex<[T]> for usize {

#[inline]
unsafe fn get_unchecked(self, slice: *const [T]) -> *const T {
debug_assert_nounwind!(
self < slice.len(),
"slice::get_unchecked requires that the index is within the slice",
);
// SAFETY: the caller guarantees that `slice` is not dangling, so it
// cannot be longer than `isize::MAX`. They also guarantee that
// `self` is in bounds of `slice` so `self` cannot overflow an `isize`,
// so the call to `add` is safe.
unsafe {
crate::hint::assert_unchecked(self < slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked requires that the index is within the slice",
(
this: usize = self,
len: usize = slice.len()
) => this < len
);
crate::intrinsics::assume(self < slice.len());
slice.as_ptr().add(self)
}
}

#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut T {
debug_assert_nounwind!(
self < slice.len(),
"slice::get_unchecked_mut requires that the index is within the slice",
);
// SAFETY: see comments for `get_unchecked` above.
unsafe { slice.as_mut_ptr().add(self) }
unsafe {
assert_unsafe_precondition!(
"slice::get_unchecked_mut requires that the index is within the slice",
(
this: usize = self,
len: usize = slice.len()
) => this < len
);
slice.as_mut_ptr().add(self)
}
}

#[inline]
Expand Down Expand Up @@ -290,25 298,36 @@ unsafe impl<T> SliceIndex<[T]> for ops::IndexRange {

#[inline]
unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] {
debug_assert_nounwind!(
self.end() <= slice.len(),
"slice::get_unchecked requires that the index is within the slice"
);
// SAFETY: the caller guarantees that `slice` is not dangling, so it
// cannot be longer than `isize::MAX`. They also guarantee that
// `self` is in bounds of `slice` so `self` cannot overflow an `isize`,
// so the call to `add` is safe.
unsafe { ptr::slice_from_raw_parts(slice.as_ptr().add(self.start()), self.len()) }
unsafe {
assert_unsafe_precondition!(
"slice::get_unchecked requires that the index is within the slice",
(
end: usize = self.end(),
len: usize = slice.len()
) => end <= len
);
ptr::slice_from_raw_parts(slice.as_ptr().add(self.start()), self.len())
}
}

#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] {
debug_assert_nounwind!(
self.end() <= slice.len(),
"slice::get_unchecked_mut requires that the index is within the slice",
);
// SAFETY: see comments for `get_unchecked` above.
unsafe { ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start()), self.len()) }
unsafe {
assert_unsafe_precondition!(
"slice::get_unchecked_mut requires that the index is within the slice",
(
end: usize = self.end(),
len: usize = slice.len()
) => end <= len
);

ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start()), self.len())
}
}

#[inline]
Expand Down Expand Up @@ -359,28 378,36 @@ unsafe impl<T> SliceIndex<[T]> for ops::Range<usize> {

#[inline]
unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] {
debug_assert_nounwind!(
self.end >= self.start && self.end <= slice.len(),
"slice::get_unchecked requires that the range is within the slice",
);
// SAFETY: the caller guarantees that `slice` is not dangling, so it
// cannot be longer than `isize::MAX`. They also guarantee that
// `self` is in bounds of `slice` so `self` cannot overflow an `isize`,
// so the call to `add` is safe and the length calculation cannot overflow.
unsafe {
assert_unsafe_precondition!(
"slice::get_unchecked requires that the range is within the slice",
(
end: usize = self.end,
start: usize = self.start,
len: usize = slice.len()
) => end >= start && end <= len
);
let new_len = unchecked_sub(self.end, self.start);
ptr::slice_from_raw_parts(slice.as_ptr().add(self.start), new_len)
}
}

#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] {
debug_assert_nounwind!(
self.end >= self.start && self.end <= slice.len(),
"slice::get_unchecked_mut requires that the range is within the slice",
);
// SAFETY: see comments for `get_unchecked` above.
unsafe {
assert_unsafe_precondition!(
"slice::get_unchecked_mut requires that the range is within the slice",
(
end: usize = self.end,
start: usize = self.start,
len: usize = slice.len()
) => end >= start && end <= len
);
let new_len = unchecked_sub(self.end, self.start);
ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start), new_len)
}
Expand Down
78 changes: 1 addition & 77 deletions tests/mir-opt/pre-codegen/slice_index.slice_get_mut_usize.PreCodegen.after.panic-abort.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,89 7,13 @@ fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> {
scope 1 (inlined core::slice::<impl [u32]>::get_mut::<usize>) {
debug self => _1;
debug index => _2;
scope 2 (inlined <usize as SliceIndex<[u32]>>::get_mut) {
debug self => _2;
debug slice => _1;
let mut _3: usize;
let mut _4: bool;
let mut _5: *mut [u32];
let mut _7: *mut u32;
let mut _8: &mut u32;
scope 3 {
scope 4 (inlined <usize as SliceIndex<[u32]>>::get_unchecked_mut) {
debug self => _2;
debug slice => _5;
let mut _6: *mut u32;
let mut _9: *mut [u32];
let mut _10: &[&str];
scope 5 {
scope 10 (inlined std::ptr::mut_ptr::<impl *mut [u32]>::as_mut_ptr) {
debug self => _5;
}
scope 11 (inlined std::ptr::mut_ptr::<impl *mut u32>::add) {
debug self => _6;
debug count => _2;
scope 12 {
}
}
}
scope 6 (inlined std::ptr::mut_ptr::<impl *mut [u32]>::len) {
debug self => _9;
let mut _11: *const [u32];
scope 7 (inlined std::ptr::metadata::<[u32]>) {
debug ptr => _11;
scope 8 {
}
}
}
scope 9 (inlined Arguments::<'_>::new_const) {
debug pieces => _10;
}
}
}
}
}

bb0: {
StorageLive(_7);
StorageLive(_4);
StorageLive(_3);
_3 = Len((*_1));
_4 = Lt(_2, move _3);
switchInt(move _4) -> [0: bb1, otherwise: bb2];
_0 = <usize as SliceIndex<[u32]>>::get_mut(move _2, move _1) -> [return: bb1, unwind unreachable];
}

bb1: {
StorageDead(_3);
_0 = const Option::<&mut u32>::None;
goto -> bb3;
}

bb2: {
StorageDead(_3);
StorageLive(_8);
StorageLive(_5);
_5 = &raw mut (*_1);
StorageLive(_9);
StorageLive(_10);
StorageLive(_11);
StorageLive(_6);
_6 = _5 as *mut u32 (PtrToPtr);
_7 = Offset(_6, _2);
StorageDead(_6);
StorageDead(_11);
StorageDead(_10);
StorageDead(_9);
StorageDead(_5);
_8 = &mut (*_7);
_0 = Option::<&mut u32>::Some(move _8);
StorageDead(_8);
goto -> bb3;
}

bb3: {
StorageDead(_4);
StorageDead(_7);
return;
}
}
78 changes: 1 addition & 77 deletions tests/mir-opt/pre-codegen/slice_index.slice_get_mut_usize.PreCodegen.after.panic-unwind.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,89 7,13 @@ fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> {
scope 1 (inlined core::slice::<impl [u32]>::get_mut::<usize>) {
debug self => _1;
debug index => _2;
scope 2 (inlined <usize as SliceIndex<[u32]>>::get_mut) {
debug self => _2;
debug slice => _1;
let mut _3: usize;
let mut _4: bool;
let mut _5: *mut [u32];
let mut _7: *mut u32;
let mut _8: &mut u32;
scope 3 {
scope 4 (inlined <usize as SliceIndex<[u32]>>::get_unchecked_mut) {
debug self => _2;
debug slice => _5;
let mut _6: *mut u32;
let mut _9: *mut [u32];
let mut _10: &[&str];
scope 5 {
scope 10 (inlined std::ptr::mut_ptr::<impl *mut [u32]>::as_mut_ptr) {
debug self => _5;
}
scope 11 (inlined std::ptr::mut_ptr::<impl *mut u32>::add) {
debug self => _6;
debug count => _2;
scope 12 {
}
}
}
scope 6 (inlined std::ptr::mut_ptr::<impl *mut [u32]>::len) {
debug self => _9;
let mut _11: *const [u32];
scope 7 (inlined std::ptr::metadata::<[u32]>) {
debug ptr => _11;
scope 8 {
}
}
}
scope 9 (inlined Arguments::<'_>::new_const) {
debug pieces => _10;
}
}
}
}
}

bb0: {
StorageLive(_7);
StorageLive(_4);
StorageLive(_3);
_3 = Len((*_1));
_4 = Lt(_2, move _3);
switchInt(move _4) -> [0: bb1, otherwise: bb2];
_0 = <usize as SliceIndex<[u32]>>::get_mut(move _2, move _1) -> [return: bb1, unwind continue];
}

bb1: {
StorageDead(_3);
_0 = const Option::<&mut u32>::None;
goto -> bb3;
}

bb2: {
StorageDead(_3);
StorageLive(_8);
StorageLive(_5);
_5 = &raw mut (*_1);
StorageLive(_9);
StorageLive(_10);
StorageLive(_11);
StorageLive(_6);
_6 = _5 as *mut u32 (PtrToPtr);
_7 = Offset(_6, _2);
StorageDead(_6);
StorageDead(_11);
StorageDead(_10);
StorageDead(_9);
StorageDead(_5);
_8 = &mut (*_7);
_0 = Option::<&mut u32>::Some(move _8);
StorageDead(_8);
goto -> bb3;
}

bb3: {
StorageDead(_4);
StorageDead(_7);
return;
}
}
Loading

0 comments on commit 8389ea2

Please sign in to comment.