Skip to content

Add more debug assertions to unsafe functions #105117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,11 @@ unsafe impl<T> const SliceIndex<[T]> for ops::Range<usize> {

#[inline]
unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] {
let this = ops::Range { start: self.start, end: self.end };
let this = ops::Range { ..self };
// 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 {
assert_unsafe_precondition!(
"slice::get_unchecked requires that the range is within the slice",
Expand All @@ -389,7 +388,7 @@ unsafe impl<T> const SliceIndex<[T]> for ops::Range<usize> {

#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] {
let this = ops::Range { start: self.start, end: self.end };
let this = ops::Range { ..self };
// SAFETY: see comments for `get_unchecked` above.
unsafe {
assert_unsafe_precondition!(
Expand Down
8 changes: 7 additions & 1 deletion library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,13 @@ impl<T> [T] {
let ptr = self.as_ptr();

// SAFETY: Caller has to check that `0 <= mid <= self.len()`
unsafe { (from_raw_parts(ptr, mid), from_raw_parts(ptr.add(mid), len - mid)) }
unsafe {
assert_unsafe_precondition!(
"slice::split_at_unchecked requires the index to be within the slice",
(mid: usize, len: usize) => mid <= len
);
(from_raw_parts(ptr, mid), from_raw_parts(ptr.add(mid), len - mid))
}
}

/// Divides one mutable slice into two at an index, without doing bounds checking.
Expand Down
72 changes: 41 additions & 31 deletions library/core/src/str/traits.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Trait implementations for `str`.

use crate::cmp::Ordering;
use crate::intrinsics::assert_unsafe_precondition;
use crate::ops;
use crate::ptr;
use crate::slice::SliceIndex;
Expand Down Expand Up @@ -198,15 +199,37 @@ unsafe impl const SliceIndex<str> for ops::Range<usize> {
let slice = slice as *const [u8];
// SAFETY: the caller guarantees that `self` is in bounds of `slice`
// which satisfies all the conditions for `add`.
let ptr = unsafe { slice.as_ptr().add(self.start) };
let ptr = unsafe {
let this = ops::Range { ..self };
assert_unsafe_precondition!(
"str::get_unchecked requires that the range is within the string slice",
(this: ops::Range<usize>, slice: *const [u8]) =>
// We'd like to check that the bounds are on char boundaries,
// but there's not really a way to do so without reading
// behind the pointer, which has aliasing implications.
// It's also not possible to move this check up to
// `str::get_unchecked` without adding a special function
// to `SliceIndex` just for this.
this.end >= this.start && this.end <= slice.len()
);
slice.as_ptr().add(self.start)
};
let len = self.end - self.start;
ptr::slice_from_raw_parts(ptr, len) as *const str
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut str) -> *mut Self::Output {
let slice = slice as *mut [u8];
// SAFETY: see comments for `get_unchecked`.
let ptr = unsafe { slice.as_mut_ptr().add(self.start) };
let ptr = unsafe {
let this = ops::Range { ..self };
assert_unsafe_precondition!(
"str::get_unchecked_mut requires that the range is within the string slice",
(this: ops::Range<usize>, slice: *mut [u8]) =>
this.end >= this.start && this.end <= slice.len()
);
slice.as_mut_ptr().add(self.start)
};
let len = self.end - self.start;
ptr::slice_from_raw_parts_mut(ptr, len) as *mut str
}
Expand Down Expand Up @@ -276,15 +299,13 @@ unsafe impl const SliceIndex<str> for ops::RangeTo<usize> {
}
#[inline]
unsafe fn get_unchecked(self, slice: *const str) -> *const Self::Output {
let slice = slice as *const [u8];
let ptr = slice.as_ptr();
ptr::slice_from_raw_parts(ptr, self.end) as *const str
// SAFETY: the caller has to uphold the safety contract for `get_unchecked`.
unsafe { (0..self.end).get_unchecked(slice) }
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut str) -> *mut Self::Output {
let slice = slice as *mut [u8];
let ptr = slice.as_mut_ptr();
ptr::slice_from_raw_parts_mut(ptr, self.end) as *mut str
// SAFETY: the caller has to uphold the safety contract for `get_unchecked_mut`.
unsafe { (0..self.end).get_unchecked_mut(slice) }
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
Expand Down Expand Up @@ -347,20 +368,15 @@ unsafe impl const SliceIndex<str> for ops::RangeFrom<usize> {
}
#[inline]
unsafe fn get_unchecked(self, slice: *const str) -> *const Self::Output {
let slice = slice as *const [u8];
// SAFETY: the caller guarantees that `self` is in bounds of `slice`
// which satisfies all the conditions for `add`.
let ptr = unsafe { slice.as_ptr().add(self.start) };
let len = slice.len() - self.start;
ptr::slice_from_raw_parts(ptr, len) as *const str
let len = (slice as *const [u8]).len();
// SAFETY: the caller has to uphold the safety contract for `get_unchecked`.
unsafe { (self.start..len).get_unchecked(slice) }
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut str) -> *mut Self::Output {
let slice = slice as *mut [u8];
// SAFETY: identical to `get_unchecked`.
let ptr = unsafe { slice.as_mut_ptr().add(self.start) };
let len = slice.len() - self.start;
ptr::slice_from_raw_parts_mut(ptr, len) as *mut str
let len = (slice as *mut [u8]).len();
// SAFETY: the caller has to uphold the safety contract for `get_unchecked_mut`.
unsafe { (self.start..len).get_unchecked_mut(slice) }
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
Expand Down Expand Up @@ -456,35 +472,29 @@ unsafe impl const SliceIndex<str> for ops::RangeToInclusive<usize> {
type Output = str;
#[inline]
fn get(self, slice: &str) -> Option<&Self::Output> {
if self.end == usize::MAX { None } else { (..self.end + 1).get(slice) }
(0..=self.end).get(slice)
}
#[inline]
fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> {
if self.end == usize::MAX { None } else { (..self.end + 1).get_mut(slice) }
(0..=self.end).get_mut(slice)
}
#[inline]
unsafe fn get_unchecked(self, slice: *const str) -> *const Self::Output {
// SAFETY: the caller must uphold the safety contract for `get_unchecked`.
unsafe { (..self.end + 1).get_unchecked(slice) }
unsafe { (0..=self.end).get_unchecked(slice) }
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut str) -> *mut Self::Output {
// SAFETY: the caller must uphold the safety contract for `get_unchecked_mut`.
unsafe { (..self.end + 1).get_unchecked_mut(slice) }
unsafe { (0..=self.end).get_unchecked_mut(slice) }
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
if self.end == usize::MAX {
str_index_overflow_fail();
}
(..self.end + 1).index(slice)
(0..=self.end).index(slice)
}
#[inline]
fn index_mut(self, slice: &mut str) -> &mut Self::Output {
if self.end == usize::MAX {
str_index_overflow_fail();
}
(..self.end + 1).index_mut(slice)
(0..=self.end).index_mut(slice)
}
}

Expand Down