Skip to content
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.

Sign up for GitHub

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

Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods #121571

Merged
merged 2 commits into from
May 25, 2024
Merged

Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods #121571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72b7171
Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr}…
clarfonthey Apr 9, 2024
18cb2fa
Stabilize the runtime of libtest-padding
saethlin May 25, 2024
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
1 change: 1 addition & 0 deletions library/core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@
#![feature(str_split_remainder)]
#![feature(strict_provenance)]
#![feature(ub_checks)]
#![feature(unchecked_neg)]
#![feature(unchecked_shifts)]
#![feature(utf16_extra)]
#![feature(utf16_extra_const)]
Expand Down
99 changes: 81 additions & 18 deletions library/core/src/num/int_macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,19 @@ macro_rules! int_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_add`.
unsafe { intrinsics::unchecked_add(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_add cannot overflow"),
(
lhs: $SelfT = self,
rhs: $SelfT = rhs,
) => !lhs.overflowing_add(rhs).1,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_add(self, rhs)
}
}

/// Checked addition with an unsigned integer. Computes `self + rhs`,
Expand Down Expand Up @@ -630,9 +640,19 @@ macro_rules! int_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_sub(self, rhs: Self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_sub`.
unsafe { intrinsics::unchecked_sub(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_sub cannot overflow"),
(
lhs: $SelfT = self,
rhs: $SelfT = rhs,
) => !lhs.overflowing_sub(rhs).1,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_sub(self, rhs)
}
}

/// Checked subtraction with an unsigned integer. Computes `self - rhs`,
Expand Down Expand Up @@ -772,9 +792,19 @@ macro_rules! int_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_mul(self, rhs: Self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_mul`.
unsafe { intrinsics::unchecked_mul(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_mul cannot overflow"),
(
lhs: $SelfT = self,
rhs: $SelfT = rhs,
) => !lhs.overflowing_mul(rhs).1,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_mul(self, rhs)
}
}

/// Checked integer division. Computes `self / rhs`, returning `None` if `rhs == 0`
Expand Down Expand Up @@ -1111,9 +1141,22 @@ macro_rules! int_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_neg(self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_neg`.
unsafe { intrinsics::unchecked_sub(0, self) }
// ICE resolved by #125184 isn't in bootstrap compiler
#[cfg(not(bootstrap))]
{
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_neg cannot overflow"),
(
lhs: $SelfT = self,
) => !lhs.overflowing_neg().1,
);
}

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_sub(0, self)
}
}

/// Strict negation. Computes `-self`, panicking if `self == MIN`.
Expand Down Expand Up @@ -1234,9 +1277,19 @@ macro_rules! int_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_shl`.
unsafe { intrinsics::unchecked_shl(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_shl cannot overflow"),
(
rhs: u32 = rhs,
bits: u32 = Self::BITS,
) => rhs < bits,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_shl(self, rhs)
}
}

/// Checked shift right. Computes `self >> rhs`, returning `None` if `rhs` is
Expand Down Expand Up @@ -1323,9 +1376,19 @@ macro_rules! int_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_shr`.
unsafe { intrinsics::unchecked_shr(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_shr cannot overflow"),
(
rhs: u32 = rhs,
bits: u32 = Self::BITS,
) => rhs < bits,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_shr(self, rhs)
}
}

/// Checked absolute value. Computes `self.abs()`, returning `None` if
Expand Down
1 change: 1 addition & 0 deletions library/core/src/num/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::hint;
use crate::intrinsics;
use crate::mem;
use crate::str::FromStr;
use crate::ub_checks::assert_unsafe_precondition;

// Used because the `?` operator is not allowed in a const context.
macro_rules! try_opt {
Expand Down
80 changes: 65 additions & 15 deletions library/core/src/num/uint_macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,19 @@ macro_rules! uint_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_add`.
unsafe { intrinsics::unchecked_add(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_add cannot overflow"),
(
lhs: $SelfT = self,
rhs: $SelfT = rhs,
) => !lhs.overflowing_add(rhs).1,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_add(self, rhs)
}
}

/// Checked addition with a signed integer. Computes `self + rhs`,
Expand Down Expand Up @@ -677,9 +687,19 @@ macro_rules! uint_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_sub(self, rhs: Self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_sub`.
unsafe { intrinsics::unchecked_sub(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_sub cannot overflow"),
(
lhs: $SelfT = self,
rhs: $SelfT = rhs,
) => !lhs.overflowing_sub(rhs).1,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_sub(self, rhs)
}
}

/// Checked integer multiplication. Computes `self * rhs`, returning
Expand Down Expand Up @@ -763,9 +783,19 @@ macro_rules! uint_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_mul(self, rhs: Self) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_mul`.
unsafe { intrinsics::unchecked_mul(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_mul cannot overflow"),
(
lhs: $SelfT = self,
rhs: $SelfT = rhs,
) => !lhs.overflowing_mul(rhs).1,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_mul(self, rhs)
}
}

/// Checked integer division. Computes `self / rhs`, returning `None`
Expand Down Expand Up @@ -1334,9 +1364,19 @@ macro_rules! uint_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_shl`.
unsafe { intrinsics::unchecked_shl(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_shl cannot overflow"),
(
rhs: u32 = rhs,
bits: u32 = Self::BITS,
) => rhs < bits,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_shl(self, rhs)
}
}

/// Checked shift right. Computes `self >> rhs`, returning `None`
Expand Down Expand Up @@ -1423,9 +1463,19 @@ macro_rules! uint_impl {
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self {
// SAFETY: the caller must uphold the safety contract for
// `unchecked_shr`.
unsafe { intrinsics::unchecked_shr(self, rhs) }
assert_unsafe_precondition!(
check_language_ub,
concat!(stringify!($SelfT), "::unchecked_shr cannot overflow"),
(
rhs: u32 = rhs,
bits: u32 = Self::BITS,
) => rhs < bits,
);

// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
intrinsics::unchecked_shr(self, rhs)
}
}

/// Checked exponentiation. Computes `self.pow(exp)`, returning `None` if
Expand Down
11 changes: 5 additions & 6 deletions library/core/src/ops/index_range.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::intrinsics::{unchecked_add, unchecked_sub};
use crate::iter::{FusedIterator, TrustedLen};
use crate::num::NonZero;
use crate::ub_checks;
Expand Down Expand Up @@ -46,7 +45,7 @@ impl IndexRange {
#[inline]
pub const fn len(&self) -> usize {
// SAFETY: By invariant, this cannot wrap
unsafe { unchecked_sub(self.end, self.start) }
unsafe { self.end.unchecked_sub(self.start) }
}

/// # Safety
Expand All @@ -57,7 +56,7 @@ impl IndexRange {

let value = self.start;
// SAFETY: The range isn't empty, so this cannot overflow
self.start = unsafe { unchecked_add(value, 1) };
self.start = unsafe { value.unchecked_add(1) };
value
}

Expand All @@ -68,7 +67,7 @@ impl IndexRange {
debug_assert!(self.start < self.end);

// SAFETY: The range isn't empty, so this cannot overflow
let value = unsafe { unchecked_sub(self.end, 1) };
let value = unsafe { self.end.unchecked_sub(1) };
self.end = value;
value
}
Expand All @@ -83,7 +82,7 @@ impl IndexRange {
let mid = if n <= self.len() {
// SAFETY: We just checked that this will be between start and end,
// and thus the addition cannot overflow.
unsafe { unchecked_add(self.start, n) }
unsafe { self.start.unchecked_add(n) }
} else {
self.end
};
Expand All @@ -102,7 +101,7 @@ impl IndexRange {
let mid = if n <= self.len() {
// SAFETY: We just checked that this will be between start and end,
// and thus the addition cannot overflow.
unsafe { unchecked_sub(self.end, n) }
unsafe { self.end.unchecked_sub(n) }
} else {
self.start
};
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/ptr/const_ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,7 @@ impl<T: ?Sized> *const T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[must_use = "returns a new pointer rather than modifying its argument"]
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
#[rustc_allow_const_fn_unstable(unchecked_neg)]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn sub(self, count: usize) -> Self
Expand All @@ -1093,7 +1094,7 @@ impl<T: ?Sized> *const T {
// SAFETY: the caller must uphold the safety contract for `offset`.
// Because the pointee is *not* a ZST, that means that `count` is
// at most `isize::MAX`, and thus the negation cannot overflow.
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
unsafe { self.offset((count as isize).unchecked_neg()) }
}
}

Expand Down
3 changes: 2 additions & 1 deletion library/core/src/ptr/mut_ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,7 @@ impl<T: ?Sized> *mut T {
#[stable(feature = "pointer_methods", since = "1.26.0")]
#[must_use = "returns a new pointer rather than modifying its argument"]
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
#[rustc_allow_const_fn_unstable(unchecked_neg)]
#[inline(always)]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn sub(self, count: usize) -> Self
Expand All @@ -1237,7 +1238,7 @@ impl<T: ?Sized> *mut T {
// SAFETY: the caller must uphold the safety contract for `offset`.
// Because the pointee is *not* a ZST, that means that `count` is
// at most `isize::MAX`, and thus the negation cannot overflow.
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
unsafe { self.offset((count as isize).unchecked_neg()) }
}
}

Expand Down
3 changes: 2 additions & 1 deletion library/core/src/ptr/non_null.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ impl<T: ?Sized> NonNull<T> {
#[must_use = "returns a new pointer rather than modifying its argument"]
#[stable(feature = "non_null_convenience", since = "CURRENT_RUSTC_VERSION")]
#[rustc_const_stable(feature = "non_null_convenience", since = "CURRENT_RUSTC_VERSION")]
#[rustc_allow_const_fn_unstable(unchecked_neg)]
pub const unsafe fn sub(self, count: usize) -> Self
where
T: Sized,
Expand All @@ -712,7 +713,7 @@ impl<T: ?Sized> NonNull<T> {
// SAFETY: the caller must uphold the safety contract for `offset`.
// Because the pointee is *not* a ZST, that means that `count` is
// at most `isize::MAX`, and thus the negation cannot overflow.
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
unsafe { self.offset((count as isize).unchecked_neg()) }
}
}

Expand Down
5 changes: 2 additions & 3 deletions library/core/src/slice/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Indexing implementations for `[T]`.

use crate::intrinsics::const_eval_select;
use crate::intrinsics::unchecked_sub;
use crate::ops;
use crate::ptr;
use crate::ub_checks::assert_unsafe_precondition;
Expand Down Expand Up @@ -374,7 +373,7 @@ unsafe impl<T> SliceIndex<[T]> for ops::Range<usize> {
// `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 {
let new_len = unchecked_sub(self.end, self.start);
let new_len = self.end.unchecked_sub(self.start);
ptr::slice_from_raw_parts(slice.as_ptr().add(self.start), new_len)
}
}
Expand All @@ -392,7 +391,7 @@ unsafe impl<T> SliceIndex<[T]> for ops::Range<usize> {
);
// SAFETY: see comments for `get_unchecked` above.
unsafe {
let new_len = unchecked_sub(self.end, self.start);
let new_len = self.end.unchecked_sub(self.start);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For calls like this that are already under an assert_unsafe_precondition that checks exactly the thing that the unchecked_sub's assert_unsafe_precondition would check, I do wonder if maybe it should stay as the intrinsic so that we don't have to pay the compile-time cost of instantiating a second check.

That could always be a follow-up PR, though, I suppose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, although we're ultimately going to have a bunch of checks duplicated when we have unsafe preconditions floating around anyway.

And besides, in a correct code base, the precondition checks will always be duplicated, since you should be only using them when they've been checked ahead of time anyway. It's just a matter of how many times they're duplicated.

ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start), new_len)
}
}
Expand Down
Loading
Loading