Skip to content

Commit

Permalink
Rewrite assert_unsafe_precondition around the new intrinsic
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Feb 8, 2024
1 parent 8836ac5 commit 61118ff
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 59 deletions.
2 changes: 1 addition & 1 deletion library/core/src/char/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
unsafe {
assert_unsafe_precondition!(
"invalid value for `char`",
(i: u32) => char_try_from_u32(i).is_ok()
(i: u32 = i) => char_try_from_u32(i).is_ok()
);
transmute(i)
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub const unsafe fn assert_unchecked(cond: bool) {
unsafe {
intrinsics::assert_unsafe_precondition!(
"hint::assert_unchecked must never be called when the condition is false",
(cond: bool) => cond,
(cond: bool = cond) => cond,
);
crate::intrinsics::assume(cond);
}
Expand Down
123 changes: 85 additions & 38 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@

use crate::marker::DiscriminantKind;
use crate::marker::Tuple;
use crate::mem;
use crate::mem::{self, align_of};

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

::core::intrinsics::const_eval_select(($($i,)*), comptime, runtime);
if ::core::intrinsics::debug_assertions() {
::core::intrinsics::const_eval_select(($($arg,)*), comptime, precondition_check);
}
}
};
}
Expand All @@ -2643,30 +2658,47 @@ pub(crate) use assert_unsafe_precondition;
/// Checks whether `ptr` is properly aligned with respect to
/// `align_of::<T>()`.
#[inline]
pub(crate) fn is_aligned_and_not_null<T>(ptr: *const T) -> bool {
!ptr.is_null() && ptr.is_aligned()
pub(crate) fn is_aligned_and_not_null(ptr: *const (), align: usize) -> bool {
!ptr.is_null() && ptr.is_aligned_to(align)
}

/// Checks whether an allocation of `len` instances of `T` exceeds
/// the maximum allowed allocation size.
#[inline]
pub(crate) fn is_valid_allocation_size<T>(len: usize) -> bool {
let max_len = const {
let size = crate::mem::size_of::<T>();
if size == 0 { usize::MAX } else { isize::MAX as usize / size }
};
pub(crate) fn is_valid_allocation_size(size: usize, len: usize) -> bool {
let max_len = if size == 0 { usize::MAX } else { isize::MAX as usize / size };
len <= max_len
}

pub(crate) fn is_nonoverlapping_mono(
src: *const (),
dst: *const (),
size: usize,
count: usize,
) -> bool {
let src_usize = src.addr();
let dst_usize = dst.addr();
let Some(size) = size.checked_mul(count) else {
crate::panicking::panic_nounwind(
"is_nonoverlapping: `size_of::<T>() * count` overflows a usize",
)
};
let diff = src_usize.abs_diff(dst_usize);
// If the absolute distance between the ptrs is at least as big as the size of the buffer,
// they do not overlap.
diff >= size
}

/// Checks whether the regions of memory starting at `src` and `dst` of size
/// `count * size_of::<T>()` do *not* overlap.
#[inline]
pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -> bool {
let src_usize = src.addr();
let dst_usize = dst.addr();
let size = mem::size_of::<T>()
.checked_mul(count)
.expect("is_nonoverlapping: `size_of::<T>() * count` overflows a usize");
let Some(size) = mem::size_of::<T>().checked_mul(count) else {
// Use panic_nounwind instead of Option::expect, so that this function is nounwind.
crate::panicking::panic_nounwind(
"is_nonoverlapping: `size_of::<T>() * count` overflows a usize",
)
};
let diff = src_usize.abs_diff(dst_usize);
// If the absolute distance between the ptrs is at least as big as the size of the buffer,
// they do not overlap.
Expand Down Expand Up @@ -2777,10 +2809,16 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
assert_unsafe_precondition!(
"ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \
and the specified memory ranges do not overlap",
[T](src: *const T, dst: *mut T, count: usize) =>
is_aligned_and_not_null(src)
&& is_aligned_and_not_null(dst)
&& is_nonoverlapping(src, dst, count)
(
src: *const () = src as *const (),
dst: *mut () = dst as *mut (),
size: usize = size_of::<T>(),
align: usize = align_of::<T>(),
count: usize = count,
) =>
is_aligned_and_not_null(src, align)
&& is_aligned_and_not_null(dst, align)
&& is_nonoverlapping_mono(src, dst, size, count)
);
copy_nonoverlapping(src, dst, count)
}
Expand Down Expand Up @@ -2870,9 +2908,15 @@ pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
// SAFETY: the safety contract for `copy` must be upheld by the caller.
unsafe {
assert_unsafe_precondition!(
"ptr::copy requires that both pointer arguments are aligned and non-null",
[T](src: *const T, dst: *mut T) =>
is_aligned_and_not_null(src) && is_aligned_and_not_null(dst)
"ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \
and the specified memory ranges do not overlap",
(
src: *const () = src as *const (),
dst: *mut () = dst as *mut (),
align: usize = align_of::<T>(),
) =>
is_aligned_and_not_null(src, align)
&& is_aligned_and_not_null(dst, align)
);
copy(src, dst, count)
}
Expand Down Expand Up @@ -2945,7 +2989,10 @@ pub const unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
unsafe {
assert_unsafe_precondition!(
"ptr::write_bytes requires that the destination pointer is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
(
addr: *const () = dst as *const (),
align: usize = align_of::<T>(),
) => is_aligned_and_not_null(addr, align)
);
write_bytes(dst, val, count)
}
Expand Down
8 changes: 5 additions & 3 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,13 +806,15 @@ impl<T: ?Sized> *const T {
where
T: Sized,
{
let this = self;
// SAFETY: The comparison has no side-effects, and the intrinsic
// does this check internally in the CTFE implementation.
unsafe {
assert_unsafe_precondition!(
"ptr::sub_ptr requires `this >= origin`",
[T](this: *const T, origin: *const T) => this >= origin
"ptr::sub_ptr requires `self >= origin`",
(
this: *const () = self as *const (),
origin: *const () = origin as *const (),
) => this >= origin
)
};

Expand Down
45 changes: 34 additions & 11 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,11 @@ use crate::cmp::Ordering;
use crate::fmt;
use crate::hash;
use crate::intrinsics::{
self, assert_unsafe_precondition, is_aligned_and_not_null, is_nonoverlapping,
self, assert_unsafe_precondition, is_aligned_and_not_null, is_nonoverlapping_mono,
};
use crate::marker::FnPtr;

use crate::mem::{self, MaybeUninit};
use crate::mem::{self, align_of, size_of, MaybeUninit};

mod alignment;
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
Expand Down Expand Up @@ -967,10 +967,16 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
assert_unsafe_precondition!(
"ptr::swap_nonoverlapping requires that both pointer arguments are aligned and non-null \
and the specified memory ranges do not overlap",
[T](x: *mut T, y: *mut T, count: usize) =>
is_aligned_and_not_null(x)
&& is_aligned_and_not_null(y)
&& is_nonoverlapping(x, y, count)
(
x: *mut () = x as *mut (),
y: *mut () = y as *mut (),
size: usize = size_of::<T>(),
align: usize = align_of::<T>(),
count: usize = count,
) =>
is_aligned_and_not_null(x, align)
&& is_aligned_and_not_null(y, align)
&& is_nonoverlapping_mono(x, y, size, count)
);
}

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

// SAFETY: the caller must guarantee that `src` is valid for reads.
unsafe {
#[cfg(debug_assertions)] // Too expensive to always enable (for now?)
assert_unsafe_precondition!(
"ptr::read requires that the pointer argument is aligned and non-null",
[T](src: *const T) => is_aligned_and_not_null(src)
(
addr: *const () = src as *const (),
align: usize = align_of::<T>(),
) => is_aligned_and_not_null(addr, align)
);
crate::intrinsics::read_via_copy(src)
}
Expand Down Expand Up @@ -1411,9 +1424,13 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
// `dst` cannot overlap `src` because the caller has mutable access
// to `dst` while `src` is owned by this function.
unsafe {
#[cfg(debug_assertions)] // Too expensive to always enable (for now?)
assert_unsafe_precondition!(
"ptr::write requires that the pointer argument is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
(
addr: *mut () = dst as *mut (),
align: usize = align_of::<T>(),
) => is_aligned_and_not_null(addr, align)
);
intrinsics::write_via_move(dst, src)
}
Expand Down Expand Up @@ -1581,7 +1598,10 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
unsafe {
assert_unsafe_precondition!(
"ptr::read_volatile requires that the pointer argument is aligned and non-null",
[T](src: *const T) => is_aligned_and_not_null(src)
(
addr: *const () = src as *const (),
align: usize = align_of::<T>(),
) => is_aligned_and_not_null(addr, align)
);
intrinsics::volatile_load(src)
}
Expand Down Expand Up @@ -1656,7 +1676,10 @@ pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
unsafe {
assert_unsafe_precondition!(
"ptr::write_volatile requires that the pointer argument is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
(
addr: *mut () = dst as *mut (),
align: usize = align_of::<T>(),
) => is_aligned_and_not_null(addr, align)
);
intrinsics::volatile_store(dst, src);
}
Expand Down
5 changes: 4 additions & 1 deletion library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ impl<T: ?Sized> NonNull<T> {
pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
// SAFETY: the caller must guarantee that `ptr` is non-null.
unsafe {
assert_unsafe_precondition!("NonNull::new_unchecked requires that the pointer is non-null", [T: ?Sized](ptr: *mut T) => !ptr.is_null());
assert_unsafe_precondition!(
"NonNull::new_unchecked requires that the pointer is non-null",
(ptr: *mut () = ptr as *mut ()) => !ptr.is_null()
);
NonNull { pointer: ptr as _ }
}
}
Expand Down
21 changes: 17 additions & 4 deletions library/core/src/slice/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::array;
use crate::intrinsics::{
assert_unsafe_precondition, is_aligned_and_not_null, is_valid_allocation_size,
};
use crate::mem::{align_of, size_of};
use crate::ops::Range;
use crate::ptr;

Expand Down Expand Up @@ -96,8 +97,14 @@ pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T]
unsafe {
assert_unsafe_precondition!(
"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`",
[T](data: *const T, len: usize) => is_aligned_and_not_null(data)
&& is_valid_allocation_size::<T>(len)
(
data: *mut () = data as *mut (),
size: usize = size_of::<T>(),
align: usize = align_of::<T>(),
len: usize = len,
) =>
is_aligned_and_not_null(data, align)
&& is_valid_allocation_size(size, len)
);
&*ptr::slice_from_raw_parts(data, len)
}
Expand Down Expand Up @@ -143,8 +150,14 @@ pub const unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a m
unsafe {
assert_unsafe_precondition!(
"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`",
[T](data: *mut T, len: usize) => is_aligned_and_not_null(data)
&& is_valid_allocation_size::<T>(len)
(
data: *mut () = data as *mut (),
size: usize = size_of::<T>(),
align: usize = align_of::<T>(),
len: usize = len,
) =>
is_aligned_and_not_null(data, align)
&& is_valid_allocation_size(size, len)
);
&mut *ptr::slice_from_raw_parts_mut(data, len)
}
Expand Down

0 comments on commit 61118ff

Please sign in to comment.