From 60a9d5a5a95c41702bde242705e441863d51014f Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Tue, 19 Oct 2021 07:37:25 +0200 Subject: [PATCH] Re-enable `copy[_nonoverlapping]()` runtime checks 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 #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 get- ting undefined behavior. The CTFE-engine won't allow missuses. The other way round is also fine. --- library/core/src/intrinsics.rs | 60 ++++++++++++++++++++++++++-------- library/core/src/lib.rs | 2 +- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 91230c027c2d2..0f57fb5b14180 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -1951,6 +1951,19 @@ pub(crate) fn is_aligned_and_not_null(ptr: *const T) -> bool { !ptr.is_null() && ptr as usize % mem::align_of::() == 0 } +/// Checks whether the regions of memory starting at `src` and `dst` of size +/// `count * size_of::()` do *not* overlap. +#[cfg(debug_assertions)] +pub(crate) fn is_nonoverlapping(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::().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::()` bytes from `src` to `dst`. The source /// and destination must *not* overlap. /// @@ -2042,15 +2055,24 @@ pub const unsafe fn copy_nonoverlapping(src: *const T, dst: *mut T, count: us pub fn copy_nonoverlapping(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(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(_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. @@ -2127,11 +2149,21 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { fn copy(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(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(_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) } diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 4b16a269f2d73..df9a12695cee9 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -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)]