Skip to content

Commit

Permalink
Re-enable copy[_nonoverlapping]() runtime checks
Browse files Browse the repository at this point in the history
This commit re-enables the debug checks for valid usages of the two
functions `copy()` and `copy_nonoverlapping()`. Those checks were com-
mented out in rust-lang#79684 in order to make the functions const. All that's
been left was a FIXME, that could not be resolved until there is was way
to only do the checks at runtime.
Since rust-lang#89247 there is such a way: `const_eval_select()`. This commit
uses that new intrinsic in order to either do nothing (at compile time)
or to do the old checks (at runtime).

The change itself is rather small: in order to make the checks usable
with `const_eval_select`, they are moved into a local function (one for
`copy` and one for `copy_nonoverlapping` to keep symmetry).

The change does not break referential transparency, as there is nothing
you can do at compile time, which you cannot do on runtime without get-
ting undefined behavior. The CTFE-engine won't allow missuses. The other
way round is also fine.
  • Loading branch information
jfrimmel committed Nov 9, 2021
1 parent 60952bc commit 60a9d5a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 15 deletions.
60 changes: 46 additions & 14 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1951,6 +1951,19 @@ pub(crate) fn is_aligned_and_not_null<T>(ptr: *const T) -> bool {
!ptr.is_null() && ptr as usize % mem::align_of::<T>() == 0
}

/// Checks whether the regions of memory starting at `src` and `dst` of size
/// `count * size_of::<T>()` do *not* overlap.
#[cfg(debug_assertions)]
pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -> bool {
let src_usize = src as usize;
let dst_usize = dst as usize;
let size = mem::size_of::<T>().checked_mul(count).unwrap();
let diff = if src_usize > dst_usize { src_usize - dst_usize } else { dst_usize - src_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
}

/// Copies `count * size_of::<T>()` bytes from `src` to `dst`. The source
/// and destination must *not* overlap.
///
Expand Down Expand Up @@ -2042,15 +2055,24 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
pub fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);
}

// FIXME: Perform these checks only at run time
/*if cfg!(debug_assertions)
&& !(is_aligned_and_not_null(src)
&& is_aligned_and_not_null(dst)
&& is_nonoverlapping(src, dst, count))
{
// Not panicking to keep codegen impact smaller.
abort();
}*/
#[cfg(debug_assertions)]
fn runtime_check<T>(src: *const T, dst: *mut T, count: usize) {
if !is_aligned_and_not_null(src)
|| !is_aligned_and_not_null(dst)
|| !is_nonoverlapping(src, dst, count)
{
// Not panicking to keep codegen impact smaller.
abort();
}
}
#[cfg(debug_assertions)]
const fn compiletime_check<T>(_src: *const T, _dst: *mut T, _count: usize) {}
#[cfg(debug_assertions)]
// SAFETY: runtime debug-assertions are a best-effort basis; it's fine to
// not do them during compile time
unsafe {
const_eval_select((src, dst, count), compiletime_check, runtime_check);
}

// SAFETY: the safety contract for `copy_nonoverlapping` must be
// upheld by the caller.
Expand Down Expand Up @@ -2127,11 +2149,21 @@ pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
fn copy<T>(src: *const T, dst: *mut T, count: usize);
}

// FIXME: Perform these checks only at run time
/*if cfg!(debug_assertions) && !(is_aligned_and_not_null(src) && is_aligned_and_not_null(dst)) {
// Not panicking to keep codegen impact smaller.
abort();
}*/
#[cfg(debug_assertions)]
fn runtime_check<T>(src: *const T, dst: *mut T) {
if !is_aligned_and_not_null(src) || !is_aligned_and_not_null(dst) {
// Not panicking to keep codegen impact smaller.
abort();
}
}
#[cfg(debug_assertions)]
const fn compiletime_check<T>(_src: *const T, _dst: *mut T) {}
#[cfg(debug_assertions)]
// SAFETY: runtime debug-assertions are a best-effort basis; it's fine to
// not do them during compile time
unsafe {
const_eval_select((src, dst), compiletime_check, runtime_check);
}

// SAFETY: the safety contract for `copy` must be upheld by the caller.
unsafe { copy(src, dst, count) }
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
#![feature(const_caller_location)]
#![feature(const_cell_into_inner)]
#![feature(const_discriminant)]
#![cfg_attr(not(bootstrap), feature(const_eval_select))]
#![feature(const_eval_select)]
#![feature(const_float_bits_conv)]
#![feature(const_float_classify)]
#![feature(const_fmt_arguments_new)]
Expand Down

0 comments on commit 60a9d5a

Please sign in to comment.