Skip to content

Commit 61118ff

Browse files
committed
Rewrite assert_unsafe_precondition around the new intrinsic
1 parent 8836ac5 commit 61118ff

File tree

7 files changed

+147
-59
lines changed

7 files changed

+147
-59
lines changed

Diff for: library/core/src/char/convert.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
2727
unsafe {
2828
assert_unsafe_precondition!(
2929
"invalid value for `char`",
30-
(i: u32) => char_try_from_u32(i).is_ok()
30+
(i: u32 = i) => char_try_from_u32(i).is_ok()
3131
);
3232
transmute(i)
3333
}

Diff for: library/core/src/hint.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ pub const unsafe fn assert_unchecked(cond: bool) {
148148
unsafe {
149149
intrinsics::assert_unsafe_precondition!(
150150
"hint::assert_unchecked must never be called when the condition is false",
151-
(cond: bool) => cond,
151+
(cond: bool = cond) => cond,
152152
);
153153
crate::intrinsics::assume(cond);
154154
}

Diff for: library/core/src/intrinsics.rs

+85-38
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656

5757
use crate::marker::DiscriminantKind;
5858
use crate::marker::Tuple;
59-
use crate::mem;
59+
use crate::mem::{self, align_of};
6060

6161
pub mod mir;
6262
pub mod simd;
@@ -2598,10 +2598,27 @@ pub const unsafe fn is_val_statically_known<T: Copy>(_arg: T) -> bool {
25982598
/// Check that the preconditions of an unsafe function are followed, if debug_assertions are on,
25992599
/// and only at runtime.
26002600
///
2601-
/// This macro should be called as `assert_unsafe_precondition!([Generics](name: Type) => Expression)`
2602-
/// where the names specified will be moved into the macro as captured variables, and defines an item
2603-
/// to call `const_eval_select` on. The tokens inside the square brackets are used to denote generics
2604-
/// for the function declarations and can be omitted if there is no generics.
2601+
/// This macro should be called as
2602+
/// `assert_unsafe_precondition!((expr => name: Type, expr => name: Type) => Expression)`
2603+
/// where each `expr` will be evaluated and passed in as function argument `name: Type`. Then all
2604+
/// those arguments are passed to a function via [`const_eval_select`].
2605+
///
2606+
/// These checks are behind a condition which is evaluated at codegen time, not expansion time like
2607+
/// [`debug_assert`]. This means that a standard library built with optimizations and debug
2608+
/// assertions disabled will have these checks optimized out of its monomorphizations, but if a
2609+
/// a caller of the standard library has debug assertions enabled and monomorphizes an expansion of
2610+
/// this macro, that monomorphization will contain the check.
2611+
///
2612+
/// Since these checks cannot be optimized out in MIR, some care must be taken in both call and
2613+
/// implementation to mitigate their compile-time overhead. The runtime function that we
2614+
/// [`const_eval_select`] to is monomorphic, `#[inline(never)]`, and `#[rustc_nounwind]`. That
2615+
/// combination of properties ensures that the code for the checks is only compiled once, and has a
2616+
/// minimal impact on the caller's code size.
2617+
///
2618+
/// Caller should also introducing any other `let` bindings or any code outside this macro in order
2619+
/// to call it. Since the precompiled standard library is built with full debuginfo and these
2620+
/// variables cannot be optimized out in MIR, an innocent-looking `let` can produce enough
2621+
/// debuginfo to have a measurable compile-time impact on debug builds.
26052622
///
26062623
/// # Safety
26072624
///
@@ -2615,26 +2632,24 @@ pub const unsafe fn is_val_statically_known<T: Copy>(_arg: T) -> bool {
26152632
///
26162633
/// So in a sense it is UB if this macro is useful, but we expect callers of `unsafe fn` to make
26172634
/// the occasional mistake, and this check should help them figure things out.
2618-
#[allow_internal_unstable(const_eval_select)] // permit this to be called in stably-const fn
2635+
#[allow_internal_unstable(const_eval_select, delayed_debug_assertions)] // permit this to be called in stably-const fn
26192636
macro_rules! assert_unsafe_precondition {
2620-
($name:expr, $([$($tt:tt)*])?($($i:ident:$ty:ty),*$(,)?) => $e:expr $(,)?) => {
2621-
if cfg!(debug_assertions) {
2622-
// allow non_snake_case to allow capturing const generics
2623-
#[allow(non_snake_case)]
2624-
#[inline(always)]
2625-
fn runtime$(<$($tt)*>)?($($i:$ty),*) {
2637+
($message:expr, ($($name:ident:$ty:ty = $arg:expr),*$(,)?) => $e:expr $(,)?) => {
2638+
{
2639+
#[inline(never)]
2640+
#[rustc_nounwind]
2641+
fn precondition_check($($name:$ty),*) {
26262642
if !$e {
2627-
// don't unwind to reduce impact on code size
26282643
::core::panicking::panic_nounwind(
2629-
concat!("unsafe precondition(s) violated: ", $name)
2644+
concat!("unsafe precondition(s) violated: ", $message)
26302645
);
26312646
}
26322647
}
2633-
#[allow(non_snake_case)]
2634-
#[inline]
2635-
const fn comptime$(<$($tt)*>)?($(_:$ty),*) {}
2648+
const fn comptime($(_:$ty),*) {}
26362649

2637-
::core::intrinsics::const_eval_select(($($i,)*), comptime, runtime);
2650+
if ::core::intrinsics::debug_assertions() {
2651+
::core::intrinsics::const_eval_select(($($arg,)*), comptime, precondition_check);
2652+
}
26382653
}
26392654
};
26402655
}
@@ -2643,30 +2658,47 @@ pub(crate) use assert_unsafe_precondition;
26432658
/// Checks whether `ptr` is properly aligned with respect to
26442659
/// `align_of::<T>()`.
26452660
#[inline]
2646-
pub(crate) fn is_aligned_and_not_null<T>(ptr: *const T) -> bool {
2647-
!ptr.is_null() && ptr.is_aligned()
2661+
pub(crate) fn is_aligned_and_not_null(ptr: *const (), align: usize) -> bool {
2662+
!ptr.is_null() && ptr.is_aligned_to(align)
26482663
}
26492664

2650-
/// Checks whether an allocation of `len` instances of `T` exceeds
2651-
/// the maximum allowed allocation size.
26522665
#[inline]
2653-
pub(crate) fn is_valid_allocation_size<T>(len: usize) -> bool {
2654-
let max_len = const {
2655-
let size = crate::mem::size_of::<T>();
2656-
if size == 0 { usize::MAX } else { isize::MAX as usize / size }
2657-
};
2666+
pub(crate) fn is_valid_allocation_size(size: usize, len: usize) -> bool {
2667+
let max_len = if size == 0 { usize::MAX } else { isize::MAX as usize / size };
26582668
len <= max_len
26592669
}
26602670

2671+
pub(crate) fn is_nonoverlapping_mono(
2672+
src: *const (),
2673+
dst: *const (),
2674+
size: usize,
2675+
count: usize,
2676+
) -> bool {
2677+
let src_usize = src.addr();
2678+
let dst_usize = dst.addr();
2679+
let Some(size) = size.checked_mul(count) else {
2680+
crate::panicking::panic_nounwind(
2681+
"is_nonoverlapping: `size_of::<T>() * count` overflows a usize",
2682+
)
2683+
};
2684+
let diff = src_usize.abs_diff(dst_usize);
2685+
// If the absolute distance between the ptrs is at least as big as the size of the buffer,
2686+
// they do not overlap.
2687+
diff >= size
2688+
}
2689+
26612690
/// Checks whether the regions of memory starting at `src` and `dst` of size
26622691
/// `count * size_of::<T>()` do *not* overlap.
26632692
#[inline]
26642693
pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -> bool {
26652694
let src_usize = src.addr();
26662695
let dst_usize = dst.addr();
2667-
let size = mem::size_of::<T>()
2668-
.checked_mul(count)
2669-
.expect("is_nonoverlapping: `size_of::<T>() * count` overflows a usize");
2696+
let Some(size) = mem::size_of::<T>().checked_mul(count) else {
2697+
// Use panic_nounwind instead of Option::expect, so that this function is nounwind.
2698+
crate::panicking::panic_nounwind(
2699+
"is_nonoverlapping: `size_of::<T>() * count` overflows a usize",
2700+
)
2701+
};
26702702
let diff = src_usize.abs_diff(dst_usize);
26712703
// If the absolute distance between the ptrs is at least as big as the size of the buffer,
26722704
// they do not overlap.
@@ -2777,10 +2809,16 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
27772809
assert_unsafe_precondition!(
27782810
"ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \
27792811
and the specified memory ranges do not overlap",
2780-
[T](src: *const T, dst: *mut T, count: usize) =>
2781-
is_aligned_and_not_null(src)
2782-
&& is_aligned_and_not_null(dst)
2783-
&& is_nonoverlapping(src, dst, count)
2812+
(
2813+
src: *const () = src as *const (),
2814+
dst: *mut () = dst as *mut (),
2815+
size: usize = size_of::<T>(),
2816+
align: usize = align_of::<T>(),
2817+
count: usize = count,
2818+
) =>
2819+
is_aligned_and_not_null(src, align)
2820+
&& is_aligned_and_not_null(dst, align)
2821+
&& is_nonoverlapping_mono(src, dst, size, count)
27842822
);
27852823
copy_nonoverlapping(src, dst, count)
27862824
}
@@ -2870,9 +2908,15 @@ pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
28702908
// SAFETY: the safety contract for `copy` must be upheld by the caller.
28712909
unsafe {
28722910
assert_unsafe_precondition!(
2873-
"ptr::copy requires that both pointer arguments are aligned and non-null",
2874-
[T](src: *const T, dst: *mut T) =>
2875-
is_aligned_and_not_null(src) && is_aligned_and_not_null(dst)
2911+
"ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \
2912+
and the specified memory ranges do not overlap",
2913+
(
2914+
src: *const () = src as *const (),
2915+
dst: *mut () = dst as *mut (),
2916+
align: usize = align_of::<T>(),
2917+
) =>
2918+
is_aligned_and_not_null(src, align)
2919+
&& is_aligned_and_not_null(dst, align)
28762920
);
28772921
copy(src, dst, count)
28782922
}
@@ -2945,7 +2989,10 @@ pub const unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
29452989
unsafe {
29462990
assert_unsafe_precondition!(
29472991
"ptr::write_bytes requires that the destination pointer is aligned and non-null",
2948-
[T](dst: *mut T) => is_aligned_and_not_null(dst)
2992+
(
2993+
addr: *const () = dst as *const (),
2994+
align: usize = align_of::<T>(),
2995+
) => is_aligned_and_not_null(addr, align)
29492996
);
29502997
write_bytes(dst, val, count)
29512998
}

Diff for: library/core/src/ptr/const_ptr.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -806,13 +806,15 @@ impl<T: ?Sized> *const T {
806806
where
807807
T: Sized,
808808
{
809-
let this = self;
810809
// SAFETY: The comparison has no side-effects, and the intrinsic
811810
// does this check internally in the CTFE implementation.
812811
unsafe {
813812
assert_unsafe_precondition!(
814-
"ptr::sub_ptr requires `this >= origin`",
815-
[T](this: *const T, origin: *const T) => this >= origin
813+
"ptr::sub_ptr requires `self >= origin`",
814+
(
815+
this: *const () = self as *const (),
816+
origin: *const () = origin as *const (),
817+
) => this >= origin
816818
)
817819
};
818820

Diff for: library/core/src/ptr/mod.rs

+34-11
Original file line numberDiff line numberDiff line change
@@ -381,11 +381,11 @@ use crate::cmp::Ordering;
381381
use crate::fmt;
382382
use crate::hash;
383383
use crate::intrinsics::{
384-
self, assert_unsafe_precondition, is_aligned_and_not_null, is_nonoverlapping,
384+
self, assert_unsafe_precondition, is_aligned_and_not_null, is_nonoverlapping_mono,
385385
};
386386
use crate::marker::FnPtr;
387387

388-
use crate::mem::{self, MaybeUninit};
388+
use crate::mem::{self, align_of, size_of, MaybeUninit};
389389

390390
mod alignment;
391391
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
@@ -967,10 +967,16 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
967967
assert_unsafe_precondition!(
968968
"ptr::swap_nonoverlapping requires that both pointer arguments are aligned and non-null \
969969
and the specified memory ranges do not overlap",
970-
[T](x: *mut T, y: *mut T, count: usize) =>
971-
is_aligned_and_not_null(x)
972-
&& is_aligned_and_not_null(y)
973-
&& is_nonoverlapping(x, y, count)
970+
(
971+
x: *mut () = x as *mut (),
972+
y: *mut () = y as *mut (),
973+
size: usize = size_of::<T>(),
974+
align: usize = align_of::<T>(),
975+
count: usize = count,
976+
) =>
977+
is_aligned_and_not_null(x, align)
978+
&& is_aligned_and_not_null(y, align)
979+
&& is_nonoverlapping_mono(x, y, size, count)
974980
);
975981
}
976982

@@ -1061,7 +1067,10 @@ pub const unsafe fn replace<T>(dst: *mut T, mut src: T) -> T {
10611067
unsafe {
10621068
assert_unsafe_precondition!(
10631069
"ptr::replace requires that the pointer argument is aligned and non-null",
1064-
[T](dst: *mut T) => is_aligned_and_not_null(dst)
1070+
(
1071+
addr: *const () = dst as *const (),
1072+
align: usize = align_of::<T>(),
1073+
) => is_aligned_and_not_null(addr, align)
10651074
);
10661075
mem::swap(&mut *dst, &mut src); // cannot overlap
10671076
}
@@ -1207,9 +1216,13 @@ pub const unsafe fn read<T>(src: *const T) -> T {
12071216

12081217
// SAFETY: the caller must guarantee that `src` is valid for reads.
12091218
unsafe {
1219+
#[cfg(debug_assertions)] // Too expensive to always enable (for now?)
12101220
assert_unsafe_precondition!(
12111221
"ptr::read requires that the pointer argument is aligned and non-null",
1212-
[T](src: *const T) => is_aligned_and_not_null(src)
1222+
(
1223+
addr: *const () = src as *const (),
1224+
align: usize = align_of::<T>(),
1225+
) => is_aligned_and_not_null(addr, align)
12131226
);
12141227
crate::intrinsics::read_via_copy(src)
12151228
}
@@ -1411,9 +1424,13 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
14111424
// `dst` cannot overlap `src` because the caller has mutable access
14121425
// to `dst` while `src` is owned by this function.
14131426
unsafe {
1427+
#[cfg(debug_assertions)] // Too expensive to always enable (for now?)
14141428
assert_unsafe_precondition!(
14151429
"ptr::write requires that the pointer argument is aligned and non-null",
1416-
[T](dst: *mut T) => is_aligned_and_not_null(dst)
1430+
(
1431+
addr: *mut () = dst as *mut (),
1432+
align: usize = align_of::<T>(),
1433+
) => is_aligned_and_not_null(addr, align)
14171434
);
14181435
intrinsics::write_via_move(dst, src)
14191436
}
@@ -1581,7 +1598,10 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
15811598
unsafe {
15821599
assert_unsafe_precondition!(
15831600
"ptr::read_volatile requires that the pointer argument is aligned and non-null",
1584-
[T](src: *const T) => is_aligned_and_not_null(src)
1601+
(
1602+
addr: *const () = src as *const (),
1603+
align: usize = align_of::<T>(),
1604+
) => is_aligned_and_not_null(addr, align)
15851605
);
15861606
intrinsics::volatile_load(src)
15871607
}
@@ -1656,7 +1676,10 @@ pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
16561676
unsafe {
16571677
assert_unsafe_precondition!(
16581678
"ptr::write_volatile requires that the pointer argument is aligned and non-null",
1659-
[T](dst: *mut T) => is_aligned_and_not_null(dst)
1679+
(
1680+
addr: *mut () = dst as *mut (),
1681+
align: usize = align_of::<T>(),
1682+
) => is_aligned_and_not_null(addr, align)
16601683
);
16611684
intrinsics::volatile_store(dst, src);
16621685
}

Diff for: library/core/src/ptr/non_null.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,10 @@ impl<T: ?Sized> NonNull<T> {
218218
pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
219219
// SAFETY: the caller must guarantee that `ptr` is non-null.
220220
unsafe {
221-
assert_unsafe_precondition!("NonNull::new_unchecked requires that the pointer is non-null", [T: ?Sized](ptr: *mut T) => !ptr.is_null());
221+
assert_unsafe_precondition!(
222+
"NonNull::new_unchecked requires that the pointer is non-null",
223+
(ptr: *mut () = ptr as *mut ()) => !ptr.is_null()
224+
);
222225
NonNull { pointer: ptr as _ }
223226
}
224227
}

Diff for: library/core/src/slice/raw.rs

+17-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::array;
44
use crate::intrinsics::{
55
assert_unsafe_precondition, is_aligned_and_not_null, is_valid_allocation_size,
66
};
7+
use crate::mem::{align_of, size_of};
78
use crate::ops::Range;
89
use crate::ptr;
910

@@ -96,8 +97,14 @@ pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T]
9697
unsafe {
9798
assert_unsafe_precondition!(
9899
"slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`",
99-
[T](data: *const T, len: usize) => is_aligned_and_not_null(data)
100-
&& is_valid_allocation_size::<T>(len)
100+
(
101+
data: *mut () = data as *mut (),
102+
size: usize = size_of::<T>(),
103+
align: usize = align_of::<T>(),
104+
len: usize = len,
105+
) =>
106+
is_aligned_and_not_null(data, align)
107+
&& is_valid_allocation_size(size, len)
101108
);
102109
&*ptr::slice_from_raw_parts(data, len)
103110
}
@@ -143,8 +150,14 @@ pub const unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a m
143150
unsafe {
144151
assert_unsafe_precondition!(
145152
"slice::from_raw_parts_mut requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`",
146-
[T](data: *mut T, len: usize) => is_aligned_and_not_null(data)
147-
&& is_valid_allocation_size::<T>(len)
153+
(
154+
data: *mut () = data as *mut (),
155+
size: usize = size_of::<T>(),
156+
align: usize = align_of::<T>(),
157+
len: usize = len,
158+
) =>
159+
is_aligned_and_not_null(data, align)
160+
&& is_valid_allocation_size(size, len)
148161
);
149162
&mut *ptr::slice_from_raw_parts_mut(data, len)
150163
}

0 commit comments

Comments
 (0)