Skip to content

Commit a8a9d85

Browse files
borsbors
committed
Auto merge of rust-lang#117494 - clarfonthey:unchecked-math-preconditions, r=<try>
Add debug_assert_nounwind to unchecked_{add,sub,neg,mul,shl,shr} methods 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 8f359be + 9c4c647 commit a8a9d85

27 files changed

+735
-159
lines changed

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

+66-22
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,15 @@ macro_rules! int_impl {
510510
#[inline(always)]
511511
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
512512
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self {
513-
// SAFETY: the caller must uphold the safety contract for
514-
// `unchecked_add`.
515-
unsafe { intrinsics::unchecked_add(self, rhs) }
513+
debug_assert_nounwind!(
514+
!self.overflowing_add(rhs).1,
515+
concat!(stringify!($SelfT), "::unchecked_add cannot overflow"),
516+
);
517+
// SAFETY: this is guaranteed to be safe by the caller.
518+
unsafe {
519+
let lhs = self;
520+
intrinsics::unchecked_add(lhs, rhs)
521+
}
516522
}
517523

518524
/// Checked addition with an unsigned integer. Computes `self + rhs`,
@@ -648,9 +654,15 @@ macro_rules! int_impl {
648654
#[inline(always)]
649655
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
650656
pub const unsafe fn unchecked_sub(self, rhs: Self) -> Self {
651-
// SAFETY: the caller must uphold the safety contract for
652-
// `unchecked_sub`.
653-
unsafe { intrinsics::unchecked_sub(self, rhs) }
657+
debug_assert_nounwind!(
658+
!self.overflowing_sub(rhs).1,
659+
concat!(stringify!($SelfT), "::unchecked_sub cannot overflow"),
660+
);
661+
// SAFETY: this is guaranteed to be safe by the caller.
662+
unsafe {
663+
let lhs = self;
664+
intrinsics::unchecked_sub(lhs, rhs)
665+
}
654666
}
655667

656668
/// Checked subtraction with an unsigned integer. Computes `self - rhs`,
@@ -786,9 +798,15 @@ macro_rules! int_impl {
786798
#[inline(always)]
787799
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
788800
pub const unsafe fn unchecked_mul(self, rhs: Self) -> Self {
789-
// SAFETY: the caller must uphold the safety contract for
790-
// `unchecked_mul`.
791-
unsafe { intrinsics::unchecked_mul(self, rhs) }
801+
debug_assert_nounwind!(
802+
!self.overflowing_mul(rhs).1,
803+
concat!(stringify!($SelfT), "::unchecked_mul cannot overflow"),
804+
);
805+
// SAFETY: this is guaranteed to be safe by the caller.
806+
unsafe {
807+
let lhs = self;
808+
intrinsics::unchecked_mul(lhs, rhs)
809+
}
792810
}
793811

794812
/// Checked integer division. Computes `self / rhs`, returning `None` if `rhs == 0`
@@ -1125,9 +1143,15 @@ macro_rules! int_impl {
11251143
#[inline(always)]
11261144
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
11271145
pub const unsafe fn unchecked_neg(self) -> Self {
1128-
// SAFETY: the caller must uphold the safety contract for
1129-
// `unchecked_neg`.
1130-
unsafe { intrinsics::unchecked_sub(0, self) }
1146+
debug_assert_nounwind!(
1147+
!self.overflowing_neg().1,
1148+
concat!(stringify!($SelfT), "::unchecked_neg cannot overflow"),
1149+
);
1150+
// SAFETY: this is guaranteed to be safe by the caller.
1151+
unsafe {
1152+
let n = self;
1153+
intrinsics::unchecked_sub(0, n)
1154+
}
11311155
}
11321156

11331157
/// Strict negation. Computes `-self`, panicking if `self == MIN`.
@@ -1179,7 +1203,7 @@ macro_rules! int_impl {
11791203
#[rustc_const_stable(feature = "const_checked_int_methods", since = "1.47.0")]
11801204
#[must_use = "this returns the result of the operation, \
11811205
without modifying the original"]
1182-
#[inline]
1206+
#[inline(always)]
11831207
pub const fn checked_shl(self, rhs: u32) -> Option<Self> {
11841208
let (a, b) = self.overflowing_shl(rhs);
11851209
if unlikely!(b) { None } else { Some(a) }
@@ -1241,10 +1265,17 @@ macro_rules! int_impl {
12411265
#[inline(always)]
12421266
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
12431267
pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self {
1244-
// SAFETY: the caller must uphold the safety contract for
1245-
// `unchecked_shl`.
1268+
debug_assert_nounwind!(
1269+
rhs < Self::BITS,
1270+
concat!(stringify!($SelfT), "::unchecked_shl cannot overflow"),
1271+
);
1272+
// SAFETY: this is guaranteed to be safe by the caller.
12461273
// Any legal shift amount is losslessly representable in the self type.
1247-
unsafe { intrinsics::unchecked_shl(self, conv_rhs_for_unchecked_shift!($SelfT, rhs)) }
1274+
unsafe {
1275+
let lhs = self;
1276+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs);
1277+
intrinsics::unchecked_shl(lhs, rhs)
1278+
}
12481279
}
12491280

12501281
/// Checked shift right. Computes `self >> rhs`, returning `None` if `rhs` is
@@ -1262,7 +1293,7 @@ macro_rules! int_impl {
12621293
#[rustc_const_stable(feature = "const_checked_int_methods", since = "1.47.0")]
12631294
#[must_use = "this returns the result of the operation, \
12641295
without modifying the original"]
1265-
#[inline]
1296+
#[inline(always)]
12661297
pub const fn checked_shr(self, rhs: u32) -> Option<Self> {
12671298
let (a, b) = self.overflowing_shr(rhs);
12681299
if unlikely!(b) { None } else { Some(a) }
@@ -1324,10 +1355,17 @@ macro_rules! int_impl {
13241355
#[inline(always)]
13251356
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
13261357
pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self {
1327-
// SAFETY: the caller must uphold the safety contract for
1328-
// `unchecked_shr`.
1358+
debug_assert_nounwind!(
1359+
rhs < Self::BITS,
1360+
concat!(stringify!($SelfT), "::unchecked_shr cannot overflow"),
1361+
);
1362+
// SAFETY: this is guaranteed to be safe by the caller.
13291363
// Any legal shift amount is losslessly representable in the self type.
1330-
unsafe { intrinsics::unchecked_shr(self, conv_rhs_for_unchecked_shift!($SelfT, rhs)) }
1364+
unsafe {
1365+
let lhs = self;
1366+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs);
1367+
intrinsics::unchecked_shr(lhs, rhs)
1368+
}
13311369
}
13321370

13331371
/// Checked absolute value. Computes `self.abs()`, returning `None` if
@@ -1991,7 +2029,10 @@ macro_rules! int_impl {
19912029
// SAFETY: the masking by the bitsize of the type ensures that we do not shift
19922030
// out of bounds
19932031
unsafe {
1994-
self.unchecked_shl(rhs & (Self::BITS - 1))
2032+
// FIXME: we can't optimize out the extra check here,
2033+
// so, we can't just call the method for now
2034+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs & (Self::BITS - 1));
2035+
intrinsics::unchecked_shl(self, rhs)
19952036
}
19962037
}
19972038

@@ -2021,7 +2062,10 @@ macro_rules! int_impl {
20212062
// SAFETY: the masking by the bitsize of the type ensures that we do not shift
20222063
// out of bounds
20232064
unsafe {
2024-
self.unchecked_shr(rhs & (Self::BITS - 1))
2065+
// FIXME: we can't optimize out the extra check here,
2066+
// so, we can't just call the method for now
2067+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs & (Self::BITS - 1));
2068+
intrinsics::unchecked_shr(self, rhs)
20252069
}
20262070
}
20272071

‎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::ops::{Add, Mul, Sub};
10+
use crate::panic::debug_assert_nounwind;
1011
use crate::str::FromStr;
1112

1213
// Used because the `?` operator is not allowed in a const context.

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

+58-19
Original file line numberDiff line numberDiff line change
@@ -518,9 +518,16 @@ macro_rules! uint_impl {
518518
#[inline(always)]
519519
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
520520
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self {
521-
// SAFETY: the caller must uphold the safety contract for
522-
// `unchecked_add`.
523-
unsafe { intrinsics::unchecked_add(self, rhs) }
521+
debug_assert_nounwind!(
522+
!self.overflowing_add(rhs).1,
523+
concat!(stringify!($SelfT), "::unchecked_add cannot overflow"),
524+
);
525+
526+
// SAFETY: this is guaranteed to be safe by the caller.
527+
unsafe {
528+
let lhs = self;
529+
intrinsics::unchecked_add(lhs, rhs)
530+
}
524531
}
525532

526533
/// Checked addition with a signed integer. Computes `self + rhs`,
@@ -662,9 +669,15 @@ macro_rules! uint_impl {
662669
#[inline(always)]
663670
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
664671
pub const unsafe fn unchecked_sub(self, rhs: Self) -> Self {
665-
// SAFETY: the caller must uphold the safety contract for
666-
// `unchecked_sub`.
667-
unsafe { intrinsics::unchecked_sub(self, rhs) }
672+
debug_assert_nounwind!(
673+
!self.overflowing_sub(rhs).1,
674+
concat!(stringify!($SelfT), "::unchecked_sub cannot overflow"),
675+
);
676+
// SAFETY: this is guaranteed to be safe by the caller.
677+
unsafe {
678+
let lhs = self;
679+
intrinsics::unchecked_sub(lhs, rhs)
680+
}
668681
}
669682

670683
/// Checked integer multiplication. Computes `self * rhs`, returning
@@ -744,9 +757,15 @@ macro_rules! uint_impl {
744757
#[inline(always)]
745758
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
746759
pub const unsafe fn unchecked_mul(self, rhs: Self) -> Self {
747-
// SAFETY: the caller must uphold the safety contract for
748-
// `unchecked_mul`.
749-
unsafe { intrinsics::unchecked_mul(self, rhs) }
760+
debug_assert_nounwind!(
761+
!self.overflowing_mul(rhs).1,
762+
concat!(stringify!($SelfT), "::unchecked_mul cannot overflow"),
763+
);
764+
// SAFETY: this is guaranteed to be safe by the caller.
765+
unsafe {
766+
let lhs = self;
767+
intrinsics::unchecked_mul(lhs, rhs)
768+
}
750769
}
751770

752771
/// Checked integer division. Computes `self / rhs`, returning `None`
@@ -1239,7 +1258,7 @@ macro_rules! uint_impl {
12391258
#[rustc_const_stable(feature = "const_checked_int_methods", since = "1.47.0")]
12401259
#[must_use = "this returns the result of the operation, \
12411260
without modifying the original"]
1242-
#[inline]
1261+
#[inline(always)]
12431262
pub const fn checked_shl(self, rhs: u32) -> Option<Self> {
12441263
let (a, b) = self.overflowing_shl(rhs);
12451264
if unlikely!(b) { None } else { Some(a) }
@@ -1301,10 +1320,17 @@ macro_rules! uint_impl {
13011320
#[inline(always)]
13021321
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
13031322
pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self {
1304-
// SAFETY: the caller must uphold the safety contract for
1305-
// `unchecked_shl`.
1323+
debug_assert_nounwind!(
1324+
rhs < Self::BITS,
1325+
concat!(stringify!($SelfT), "::unchecked_shl cannot overflow"),
1326+
);
1327+
// SAFETY: this is guaranteed to be safe by the caller.
13061328
// Any legal shift amount is losslessly representable in the self type.
1307-
unsafe { intrinsics::unchecked_shl(self, conv_rhs_for_unchecked_shift!($SelfT, rhs)) }
1329+
unsafe {
1330+
let lhs = self;
1331+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs);
1332+
intrinsics::unchecked_shl(lhs, rhs)
1333+
}
13081334
}
13091335

13101336
/// Checked shift right. Computes `self >> rhs`, returning `None`
@@ -1322,7 +1348,7 @@ macro_rules! uint_impl {
13221348
#[rustc_const_stable(feature = "const_checked_int_methods", since = "1.47.0")]
13231349
#[must_use = "this returns the result of the operation, \
13241350
without modifying the original"]
1325-
#[inline]
1351+
#[inline(always)]
13261352
pub const fn checked_shr(self, rhs: u32) -> Option<Self> {
13271353
let (a, b) = self.overflowing_shr(rhs);
13281354
if unlikely!(b) { None } else { Some(a) }
@@ -1384,10 +1410,17 @@ macro_rules! uint_impl {
13841410
#[inline(always)]
13851411
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
13861412
pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self {
1387-
// SAFETY: the caller must uphold the safety contract for
1388-
// `unchecked_shr`.
1413+
debug_assert_nounwind!(
1414+
rhs < Self::BITS,
1415+
concat!(stringify!($SelfT), "::unchecked_shr cannot overflow"),
1416+
);
1417+
// SAFETY: this is guaranteed to be safe by the caller.
13891418
// Any legal shift amount is losslessly representable in the self type.
1390-
unsafe { intrinsics::unchecked_shr(self, conv_rhs_for_unchecked_shift!($SelfT, rhs)) }
1419+
unsafe {
1420+
let lhs = self;
1421+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs);
1422+
intrinsics::unchecked_shr(lhs, rhs)
1423+
}
13911424
}
13921425

13931426
/// Checked exponentiation. Computes `self.pow(exp)`, returning `None` if
@@ -1878,7 +1911,10 @@ macro_rules! uint_impl {
18781911
// SAFETY: the masking by the bitsize of the type ensures that we do not shift
18791912
// out of bounds
18801913
unsafe {
1881-
self.unchecked_shl(rhs & (Self::BITS - 1))
1914+
// FIXME: we can't optimize out the extra check here,
1915+
// so, we can't just call the method for now
1916+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs & (Self::BITS - 1));
1917+
intrinsics::unchecked_shl(self, rhs)
18821918
}
18831919
}
18841920

@@ -1911,7 +1947,10 @@ macro_rules! uint_impl {
19111947
// SAFETY: the masking by the bitsize of the type ensures that we do not shift
19121948
// out of bounds
19131949
unsafe {
1914-
self.unchecked_shr(rhs & (Self::BITS - 1))
1950+
// FIXME: we can't optimize out the extra check here,
1951+
// so, we can't just call the method for now
1952+
let rhs = conv_rhs_for_unchecked_shift!($SelfT, rhs & (Self::BITS - 1));
1953+
intrinsics::unchecked_shr(self, rhs)
19151954
}
19161955
}
19171956

‎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

@@ -44,7 +43,7 @@ impl IndexRange {
4443
#[inline]
4544
pub const fn len(&self) -> usize {
4645
// SAFETY: By invariant, this cannot wrap
47-
unsafe { unchecked_sub(self.end, self.start) }
46+
unsafe { self.end.unchecked_sub(self.start) }
4847
}
4948

5049
/// # Safety
@@ -55,7 +54,7 @@ impl IndexRange {
5554

5655
let value = self.start;
5756
// SAFETY: The range isn't empty, so this cannot overflow
58-
self.start = unsafe { unchecked_add(value, 1) };
57+
self.start = unsafe { value.unchecked_add(1) };
5958
value
6059
}
6160

@@ -66,7 +65,7 @@ impl IndexRange {
6665
debug_assert!(self.start < self.end);
6766

6867
// SAFETY: The range isn't empty, so this cannot overflow
69-
let value = unsafe { unchecked_sub(self.end, 1) };
68+
let value = unsafe { self.end.unchecked_sub(1) };
7069
self.end = value;
7170
value
7271
}
@@ -81,7 +80,7 @@ impl IndexRange {
8180
let mid = if n <= self.len() {
8281
// SAFETY: We just checked that this will be between start and end,
8382
// and thus the addition cannot overflow.
84-
unsafe { unchecked_add(self.start, n) }
83+
unsafe { self.start.unchecked_add(n) }
8584
} else {
8685
self.end
8786
};
@@ -100,7 +99,7 @@ impl IndexRange {
10099
let mid = if n <= self.len() {
101100
// SAFETY: We just checked that this will be between start and end,
102101
// and thus the addition cannot overflow.
103-
unsafe { unchecked_sub(self.end, n) }
102+
unsafe { self.end.unchecked_sub(n) }
104103
} else {
105104
self.start
106105
};

‎library/core/src/panic.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ pub macro debug_assert_nounwind {
167167
}
168168
}
169169
},
170-
($cond:expr, $message:expr) => {
170+
($cond:expr, $message:expr $(,)?) => {
171171
if $crate::intrinsics::debug_assertions() {
172172
if !$cond {
173173
$crate::panicking::panic_nounwind($message);

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -1021,8 +1021,8 @@ impl<T: ?Sized> *const T {
10211021
#[must_use = "returns a new pointer rather than modifying its argument"]
10221022
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
10231023
// We could always go back to wrapping if unchecked becomes unacceptable
1024-
#[rustc_allow_const_fn_unstable(const_int_unchecked_arith)]
10251024
#[inline(always)]
1025+
#[rustc_allow_const_fn_unstable(unchecked_math)]
10261026
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
10271027
pub const unsafe fn sub(self, count: usize) -> Self
10281028
where
@@ -1035,7 +1035,10 @@ impl<T: ?Sized> *const T {
10351035
// SAFETY: the caller must uphold the safety contract for `offset`.
10361036
// Because the pointee is *not* a ZST, that means that `count` is
10371037
// at most `isize::MAX`, and thus the negation cannot overflow.
1038-
unsafe { self.offset(intrinsics::unchecked_sub(0, count as isize)) }
1038+
// FIXME: replacing unchecked_sub with unchecked_neg and replacing the
1039+
// unchecked_math flag with unchecked_neg will anger the UnstableInStable lint
1040+
// and I cannot for the life of me understand why
1041+
unsafe { self.offset(0isize.unchecked_sub(count as isize)) }
10391042
}
10401043
}
10411044

0 commit comments

Comments
 (0)