Skip to content

Commit 1cdaadd

Browse files
borsbors
committed
Auto merge of rust-lang#121571 - clarfonthey:unchecked-math-preconditions, r=saethlin
Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods (Old PR is haunted, opening a new one. See rust-lang#117494 for previous discussion.) This ensures that these preconditions are actually checked in debug mode, and hopefully should let people know if they messed up. I've also replaced the calls (I could find) in the code that use these intrinsics directly with those that use these methods, so that the asserts actually apply. More discussions on people misusing these methods in the tracking issue: rust-lang#85122.
2 parents 9cdfe28 + ced045e commit 1cdaadd

16 files changed

+225
-119
lines changed

‎library/core/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@
191191
#![feature(str_split_remainder)]
192192
#![feature(strict_provenance)]
193193
#![feature(ub_checks)]
194+
#![feature(unchecked_neg)]
194195
#![feature(unchecked_shifts)]
195196
#![feature(utf16_extra)]
196197
#![feature(utf16_extra_const)]

‎library/core/src/num/int_macros.rs

+81-18
Original file line numberDiff line numberDiff line change
@@ -488,9 +488,19 @@ macro_rules! int_impl {
488488
#[inline(always)]
489489
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
490490
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self {
491-
// SAFETY: the caller must uphold the safety contract for
492-
// `unchecked_add`.
493-
unsafe { intrinsics::unchecked_add(self, rhs) }
491+
assert_unsafe_precondition!(
492+
check_language_ub,
493+
concat!(stringify!($SelfT), "::unchecked_add cannot overflow"),
494+
(
495+
lhs: $SelfT = self,
496+
rhs: $SelfT = rhs,
497+
) => !lhs.overflowing_add(rhs).1,
498+
);
499+
500+
// SAFETY: this is guaranteed to be safe by the caller.
501+
unsafe {
502+
intrinsics::unchecked_add(self, rhs)
503+
}
494504
}
495505

496506
/// Checked addition with an unsigned integer. Computes `self + rhs`,
@@ -630,9 +640,19 @@ macro_rules! int_impl {
630640
#[inline(always)]
631641
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
632642
pub const unsafe fn unchecked_sub(self, rhs: Self) -> Self {
633-
// SAFETY: the caller must uphold the safety contract for
634-
// `unchecked_sub`.
635-
unsafe { intrinsics::unchecked_sub(self, rhs) }
643+
assert_unsafe_precondition!(
644+
check_language_ub,
645+
concat!(stringify!($SelfT), "::unchecked_sub cannot overflow"),
646+
(
647+
lhs: $SelfT = self,
648+
rhs: $SelfT = rhs,
649+
) => !lhs.overflowing_sub(rhs).1,
650+
);
651+
652+
// SAFETY: this is guaranteed to be safe by the caller.
653+
unsafe {
654+
intrinsics::unchecked_sub(self, rhs)
655+
}
636656
}
637657

638658
/// Checked subtraction with an unsigned integer. Computes `self - rhs`,
@@ -772,9 +792,19 @@ macro_rules! int_impl {
772792
#[inline(always)]
773793
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
774794
pub const unsafe fn unchecked_mul(self, rhs: Self) -> Self {
775-
// SAFETY: the caller must uphold the safety contract for
776-
// `unchecked_mul`.
777-
unsafe { intrinsics::unchecked_mul(self, rhs) }
795+
assert_unsafe_precondition!(
796+
check_language_ub,
797+
concat!(stringify!($SelfT), "::unchecked_mul cannot overflow"),
798+
(
799+
lhs: $SelfT = self,
800+
rhs: $SelfT = rhs,
801+
) => !lhs.overflowing_mul(rhs).1,
802+
);
803+
804+
// SAFETY: this is guaranteed to be safe by the caller.
805+
unsafe {
806+
intrinsics::unchecked_mul(self, rhs)
807+
}
778808
}
779809

780810
/// Checked integer division. Computes `self / rhs`, returning `None` if `rhs == 0`
@@ -1111,9 +1141,22 @@ macro_rules! int_impl {
11111141
#[inline(always)]
11121142
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
11131143
pub const unsafe fn unchecked_neg(self) -> Self {
1114-
// SAFETY: the caller must uphold the safety contract for
1115-
// `unchecked_neg`.
1116-
unsafe { intrinsics::unchecked_sub(0, self) }
1144+
// ICE resolved by #125184 isn't in bootstrap compiler
1145+
#[cfg(not(bootstrap))]
1146+
{
1147+
assert_unsafe_precondition!(
1148+
check_language_ub,
1149+
concat!(stringify!($SelfT), "::unchecked_neg cannot overflow"),
1150+
(
1151+
lhs: $SelfT = self,
1152+
) => !lhs.overflowing_neg().1,
1153+
);
1154+
}
1155+
1156+
// SAFETY: this is guaranteed to be safe by the caller.
1157+
unsafe {
1158+
intrinsics::unchecked_sub(0, self)
1159+
}
11171160
}
11181161

11191162
/// Strict negation. Computes `-self`, panicking if `self == MIN`.
@@ -1234,9 +1277,19 @@ macro_rules! int_impl {
12341277
#[inline(always)]
12351278
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
12361279
pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self {
1237-
// SAFETY: the caller must uphold the safety contract for
1238-
// `unchecked_shl`.
1239-
unsafe { intrinsics::unchecked_shl(self, rhs) }
1280+
assert_unsafe_precondition!(
1281+
check_language_ub,
1282+
concat!(stringify!($SelfT), "::unchecked_shl cannot overflow"),
1283+
(
1284+
rhs: u32 = rhs,
1285+
bits: u32 = Self::BITS,
1286+
) => rhs < bits,
1287+
);
1288+
1289+
// SAFETY: this is guaranteed to be safe by the caller.
1290+
unsafe {
1291+
intrinsics::unchecked_shl(self, rhs)
1292+
}
12401293
}
12411294

12421295
/// Checked shift right. Computes `self >> rhs`, returning `None` if `rhs` is
@@ -1323,9 +1376,19 @@ macro_rules! int_impl {
13231376
#[inline(always)]
13241377
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
13251378
pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self {
1326-
// SAFETY: the caller must uphold the safety contract for
1327-
// `unchecked_shr`.
1328-
unsafe { intrinsics::unchecked_shr(self, rhs) }
1379+
assert_unsafe_precondition!(
1380+
check_language_ub,
1381+
concat!(stringify!($SelfT), "::unchecked_shr cannot overflow"),
1382+
(
1383+
rhs: u32 = rhs,
1384+
bits: u32 = Self::BITS,
1385+
) => rhs < bits,
1386+
);
1387+
1388+
// SAFETY: this is guaranteed to be safe by the caller.
1389+
unsafe {
1390+
intrinsics::unchecked_shr(self, rhs)
1391+
}
13291392
}
13301393

13311394
/// Checked absolute value. Computes `self.abs()`, returning `None` if

‎library/core/src/num/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::hint;
77
use crate::intrinsics;
88
use crate::mem;
99
use crate::str::FromStr;
10+
use crate::ub_checks::assert_unsafe_precondition;
1011

1112
// Used because the `?` operator is not allowed in a const context.
1213
macro_rules! try_opt {

‎library/core/src/num/uint_macros.rs

+65-15
Original file line numberDiff line numberDiff line change
@@ -495,9 +495,19 @@ macro_rules! uint_impl {
495495
#[inline(always)]
496496
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
497497
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self {
498-
// SAFETY: the caller must uphold the safety contract for
499-
// `unchecked_add`.
500-
unsafe { intrinsics::unchecked_add(self, rhs) }
498+
assert_unsafe_precondition!(
499+
check_language_ub,
500+
concat!(stringify!($SelfT), "::unchecked_add cannot overflow"),
501+
(
502+
lhs: $SelfT = self,
503+
rhs: $SelfT = rhs,
504+
) => !lhs.overflowing_add(rhs).1,
505+
);
506+
507+
// SAFETY: this is guaranteed to be safe by the caller.
508+
unsafe {
509+
intrinsics::unchecked_add(self, rhs)
510+
}
501511
}
502512

503513
/// Checked addition with a signed integer. Computes `self + rhs`,
@@ -677,9 +687,19 @@ macro_rules! uint_impl {
677687
#[inline(always)]
678688
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
679689
pub const unsafe fn unchecked_sub(self, rhs: Self) -> Self {
680-
// SAFETY: the caller must uphold the safety contract for
681-
// `unchecked_sub`.
682-
unsafe { intrinsics::unchecked_sub(self, rhs) }
690+
assert_unsafe_precondition!(
691+
check_language_ub,
692+
concat!(stringify!($SelfT), "::unchecked_sub cannot overflow"),
693+
(
694+
lhs: $SelfT = self,
695+
rhs: $SelfT = rhs,
696+
) => !lhs.overflowing_sub(rhs).1,
697+
);
698+
699+
// SAFETY: this is guaranteed to be safe by the caller.
700+
unsafe {
701+
intrinsics::unchecked_sub(self, rhs)
702+
}
683703
}
684704

685705
/// Checked integer multiplication. Computes `self * rhs`, returning
@@ -763,9 +783,19 @@ macro_rules! uint_impl {
763783
#[inline(always)]
764784
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
765785
pub const unsafe fn unchecked_mul(self, rhs: Self) -> Self {
766-
// SAFETY: the caller must uphold the safety contract for
767-
// `unchecked_mul`.
768-
unsafe { intrinsics::unchecked_mul(self, rhs) }
786+
assert_unsafe_precondition!(
787+
check_language_ub,
788+
concat!(stringify!($SelfT), "::unchecked_mul cannot overflow"),
789+
(
790+
lhs: $SelfT = self,
791+
rhs: $SelfT = rhs,
792+
) => !lhs.overflowing_mul(rhs).1,
793+
);
794+
795+
// SAFETY: this is guaranteed to be safe by the caller.
796+
unsafe {
797+
intrinsics::unchecked_mul(self, rhs)
798+
}
769799
}
770800

771801
/// Checked integer division. Computes `self / rhs`, returning `None`
@@ -1334,9 +1364,19 @@ macro_rules! uint_impl {
13341364
#[inline(always)]
13351365
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
13361366
pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self {
1337-
// SAFETY: the caller must uphold the safety contract for
1338-
// `unchecked_shl`.
1339-
unsafe { intrinsics::unchecked_shl(self, rhs) }
1367+
assert_unsafe_precondition!(
1368+
check_language_ub,
1369+
concat!(stringify!($SelfT), "::unchecked_shl cannot overflow"),
1370+
(
1371+
rhs: u32 = rhs,
1372+
bits: u32 = Self::BITS,
1373+
) => rhs < bits,
1374+
);
1375+
1376+
// SAFETY: this is guaranteed to be safe by the caller.
1377+
unsafe {
1378+
intrinsics::unchecked_shl(self, rhs)
1379+
}
13401380
}
13411381

13421382
/// Checked shift right. Computes `self >> rhs`, returning `None`
@@ -1423,9 +1463,19 @@ macro_rules! uint_impl {
14231463
#[inline(always)]
14241464
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
14251465
pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self {
1426-
// SAFETY: the caller must uphold the safety contract for
1427-
// `unchecked_shr`.
1428-
unsafe { intrinsics::unchecked_shr(self, rhs) }
1466+
assert_unsafe_precondition!(
1467+
check_language_ub,
1468+
concat!(stringify!($SelfT), "::unchecked_shr cannot overflow"),
1469+
(
1470+
rhs: u32 = rhs,
1471+
bits: u32 = Self::BITS,
1472+
) => rhs < bits,
1473+
);
1474+
1475+
// SAFETY: this is guaranteed to be safe by the caller.
1476+
unsafe {
1477+
intrinsics::unchecked_shr(self, rhs)
1478+
}
14291479
}
14301480

14311481
/// Checked exponentiation. Computes `self.pow(exp)`, returning `None` if

‎library/core/src/ops/index_range.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::intrinsics::{unchecked_add, unchecked_sub};
21
use crate::iter::{FusedIterator, TrustedLen};
32
use crate::num::NonZero;
43
use crate::ub_checks;
@@ -46,7 +45,7 @@ impl IndexRange {
4645
#[inline]
4746
pub const fn len(&self) -> usize {
4847
// SAFETY: By invariant, this cannot wrap
49-
unsafe { unchecked_sub(self.end, self.start) }
48+
unsafe { self.end.unchecked_sub(self.start) }
5049
}
5150

5251
/// # Safety
@@ -57,7 +56,7 @@ impl IndexRange {
5756

5857
let value = self.start;
5958
// SAFETY: The range isn't empty, so this cannot overflow
60-
self.start = unsafe { unchecked_add(value, 1) };
59+
self.start = unsafe { value.unchecked_add(1) };
6160
value
6261
}
6362

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

7069
// SAFETY: The range isn't empty, so this cannot overflow
71-
let value = unsafe { unchecked_sub(self.end, 1) };
70+
let value = unsafe { self.end.unchecked_sub(1) };
7271
self.end = value;
7372
value
7473
}
@@ -83,7 +82,7 @@ impl IndexRange {
8382
let mid = if n <= self.len() {
8483
// SAFETY: We just checked that this will be between start and end,
8584
// and thus the addition cannot overflow.
86-
unsafe { unchecked_add(self.start, n) }
85+
unsafe { self.start.unchecked_add(n) }
8786
} else {
8887
self.end
8988
};
@@ -102,7 +101,7 @@ impl IndexRange {
102101
let mid = if n <= self.len() {
103102
// SAFETY: We just checked that this will be between start and end,
104103
// and thus the addition cannot overflow.
105-
unsafe { unchecked_sub(self.end, n) }
104+
unsafe { self.end.unchecked_sub(n) }
106105
} else {
107106
self.start
108107
};

‎library/core/src/ptr/const_ptr.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,7 @@ impl<T: ?Sized> *const T {
10801080
#[stable(feature = "pointer_methods", since = "1.26.0")]
10811081
#[must_use = "returns a new pointer rather than modifying its argument"]
10821082
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
1083+
#[rustc_allow_const_fn_unstable(unchecked_neg)]
10831084
#[inline(always)]
10841085
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
10851086
pub const unsafe fn sub(self, count: usize) -> Self
@@ -1093,7 +1094,7 @@ impl<T: ?Sized> *const T {
10931094
// SAFETY: the caller must uphold the safety contract for `offset`.
10941095
// Because the pointee is *not* a ZST, that means that `count` is
10951096
// at most `isize::MAX`, and thus the negation cannot overflow.
1096-
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
1097+
unsafe { self.offset((count as isize).unchecked_neg()) }
10971098
}
10981099
}
10991100

‎library/core/src/ptr/mut_ptr.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,7 @@ impl<T: ?Sized> *mut T {
12241224
#[stable(feature = "pointer_methods", since = "1.26.0")]
12251225
#[must_use = "returns a new pointer rather than modifying its argument"]
12261226
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
1227+
#[rustc_allow_const_fn_unstable(unchecked_neg)]
12271228
#[inline(always)]
12281229
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
12291230
pub const unsafe fn sub(self, count: usize) -> Self
@@ -1237,7 +1238,7 @@ impl<T: ?Sized> *mut T {
12371238
// SAFETY: the caller must uphold the safety contract for `offset`.
12381239
// Because the pointee is *not* a ZST, that means that `count` is
12391240
// at most `isize::MAX`, and thus the negation cannot overflow.
1240-
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
1241+
unsafe { self.offset((count as isize).unchecked_neg()) }
12411242
}
12421243
}
12431244

‎library/core/src/ptr/non_null.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,7 @@ impl<T: ?Sized> NonNull<T> {
701701
#[must_use = "returns a new pointer rather than modifying its argument"]
702702
#[stable(feature = "non_null_convenience", since = "CURRENT_RUSTC_VERSION")]
703703
#[rustc_const_stable(feature = "non_null_convenience", since = "CURRENT_RUSTC_VERSION")]
704+
#[rustc_allow_const_fn_unstable(unchecked_neg)]
704705
pub const unsafe fn sub(self, count: usize) -> Self
705706
where
706707
T: Sized,
@@ -712,7 +713,7 @@ impl<T: ?Sized> NonNull<T> {
712713
// SAFETY: the caller must uphold the safety contract for `offset`.
713714
// Because the pointee is *not* a ZST, that means that `count` is
714715
// at most `isize::MAX`, and thus the negation cannot overflow.
715-
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
716+
unsafe { self.offset((count as isize).unchecked_neg()) }
716717
}
717718
}
718719

‎library/core/src/slice/index.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Indexing implementations for `[T]`.
22
33
use crate::intrinsics::const_eval_select;
4-
use crate::intrinsics::unchecked_sub;
54
use crate::ops;
65
use crate::ptr;
76
use crate::ub_checks::assert_unsafe_precondition;
@@ -374,7 +373,7 @@ unsafe impl<T> SliceIndex<[T]> for ops::Range<usize> {
374373
// `self` is in bounds of `slice` so `self` cannot overflow an `isize`,
375374
// so the call to `add` is safe and the length calculation cannot overflow.
376375
unsafe {
377-
let new_len = unchecked_sub(self.end, self.start);
376+
let new_len = self.end.unchecked_sub(self.start);
378377
ptr::slice_from_raw_parts(slice.as_ptr().add(self.start), new_len)
379378
}
380379
}
@@ -392,7 +391,7 @@ unsafe impl<T> SliceIndex<[T]> for ops::Range<usize> {
392391
);
393392
// SAFETY: see comments for `get_unchecked` above.
394393
unsafe {
395-
let new_len = unchecked_sub(self.end, self.start);
394+
let new_len = self.end.unchecked_sub(self.start);
396395
ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start), new_len)
397396
}
398397
}

0 commit comments

Comments
 (0)