Skip to content

Commit

Permalink
Auto merge of #90041 - jfrimmel:rt_copy_checks, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Re-enable `copy[_nonoverlapping]()` debug-checks

This commit re-enables the debug checks for valid usages of the two functions `copy()` and `copy_nonoverlapping()`. Those checks were commented out in #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 #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 getting undefined behavior. The CTFE-engine won't allow missuses. The other way round is also fine.

I've refactored the code to use `#[cfg(debug_assertions)]` on the new items. If that is not desired, the second commit can be dropped.
I haven't added any checks, as I currently don't know, how to test this properly.

Closes #90012.

cc `@rust-lang/lang,` `@rust-lang/libs` and `@rust-lang/wg-const-eval` (as those teams are linked in the issue above).
  • Loading branch information
bors committed Nov 13, 2021
2 parents 032dfe4 + 60a9d5a commit 7594067
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 7594067

Please sign in to comment.