diff --git a/library/core/src/ops/index_range.rs b/library/core/src/ops/index_range.rs index 07ea2e930d57a..0c6855900ed98 100644 --- a/library/core/src/ops/index_range.rs +++ b/library/core/src/ops/index_range.rs @@ -19,7 +19,9 @@ impl IndexRange { /// - `start <= end` #[inline] pub const unsafe fn new_unchecked(start: usize, end: usize) -> Self { - crate::panic::debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter + // will miss out on this check. + crate::panic::debug_assert_ubcheck!( start <= end, "IndexRange::new_unchecked requires `start <= end`" ); diff --git a/library/core/src/panic.rs b/library/core/src/panic.rs index 2bd42a4a8cadc..ccdbcaa9bae77 100644 --- a/library/core/src/panic.rs +++ b/library/core/src/panic.rs @@ -143,12 +143,16 @@ pub macro unreachable_2021 { /// checks are enabled when they are monomorphized with debug assertions enabled, and upon failure /// a non-unwinding panic is launched so that failures cannot compromise unwind safety. /// -/// But there are many differences from `assert_unsafe_precondition!`. This macro does not use -/// `const_eval_select` internally, and therefore it is sound to call this with an expression -/// that evaluates to `false`. Also unlike `assert_unsafe_precondition!` the condition being -/// checked here is not put in an outlined function. If the check compiles to a lot of IR, this -/// can cause code bloat if the check is monomorphized many times. But it also means that the checks -/// from this macro can be deduplicated or otherwise optimized out. +/// These checks are not executed in the interpreter (const-eval and Miri), so they should only +/// check requirements that immediately lead to UB if violated (and the interpreter will then show a +/// nice diagnostic for that UB). +/// +/// But there are many differences from `assert_unsafe_precondition!`. Most importantly, the +/// condition being checked here is not put in an outlined function. If the check compiles to a lot +/// of IR, this can cause code bloat if the check is monomorphized many times. But it also means +/// that the checks from this macro can be deduplicated or otherwise optimized out. +/// This also does not use `const_eval_select` so making sure that the code has UB if the condition +/// is violated is not a soundness concern, but as noted above it is still a correctness concern. /// /// In general, this macro should be used to check all public-facing preconditions. But some /// preconditions may be called too often or instantiated too often to make the overhead of the @@ -159,7 +163,7 @@ pub macro unreachable_2021 { #[unstable(feature = "panic_internals", issue = "none")] #[allow_internal_unstable(panic_internals, delayed_debug_assertions)] #[rustc_macro_transparency = "semitransparent"] -pub macro debug_assert_nounwind { +pub macro debug_assert_ubcheck { ($cond:expr $(,)?) => { if $crate::intrinsics::debug_assertions() { if !$cond { diff --git a/library/core/src/ptr/alignment.rs b/library/core/src/ptr/alignment.rs index bd58c167c2e06..ce91f595b394c 100644 --- a/library/core/src/ptr/alignment.rs +++ b/library/core/src/ptr/alignment.rs @@ -77,7 +77,7 @@ impl Alignment { #[inline] pub const unsafe fn new_unchecked(align: usize) -> Self { #[cfg(debug_assertions)] - crate::panic::debug_assert_nounwind!( + crate::panic::debug_assert_ubcheck!( align.is_power_of_two(), "Alignment::new_unchecked requires a power of two" ); diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index 0e2d45c4ada6d..6996d350d0d6e 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -3,7 +3,7 @@ use crate::intrinsics::const_eval_select; use crate::intrinsics::unchecked_sub; use crate::ops; -use crate::panic::debug_assert_nounwind; +use crate::panic::debug_assert_ubcheck; use crate::ptr; #[stable(feature = "rust1", since = "1.0.0")] @@ -225,7 +225,7 @@ unsafe impl SliceIndex<[T]> for usize { #[inline] unsafe fn get_unchecked(self, slice: *const [T]) -> *const T { - debug_assert_nounwind!( + debug_assert_ubcheck!( self < slice.len(), "slice::get_unchecked requires that the index is within the slice" ); @@ -243,7 +243,9 @@ unsafe impl SliceIndex<[T]> for usize { #[inline] unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut T { - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The offset might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( self < slice.len(), "slice::get_unchecked_mut requires that the index is within the slice" ); @@ -292,7 +294,9 @@ unsafe impl SliceIndex<[T]> for ops::IndexRange { #[inline] unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The end might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( self.end() <= slice.len(), "slice::get_unchecked requires that the index is within the slice" ); @@ -305,7 +309,9 @@ unsafe impl SliceIndex<[T]> for ops::IndexRange { #[inline] unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] { - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The end might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( self.end() <= slice.len(), "slice::get_unchecked_mut requires that the index is within the slice" ); @@ -362,7 +368,9 @@ unsafe impl SliceIndex<[T]> for ops::Range { #[inline] unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The end might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( self.end >= self.start && self.end <= slice.len(), "slice::get_unchecked requires that the range is within the slice" ); @@ -379,7 +387,9 @@ unsafe impl SliceIndex<[T]> for ops::Range { #[inline] unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] { - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The end might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( self.end >= self.start && self.end <= slice.len(), "slice::get_unchecked_mut requires that the range is within the slice" ); diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 1ad81fcfcfeed..b8e4d378dd597 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -13,7 +13,7 @@ use crate::intrinsics::exact_div; use crate::mem::{self, SizedTypeProperties}; use crate::num::NonZero; use crate::ops::{Bound, OneSidedRange, Range, RangeBounds}; -use crate::panic::debug_assert_nounwind; +use crate::panic::debug_assert_ubcheck; use crate::ptr; use crate::simd::{self, Simd}; use crate::slice; @@ -945,7 +945,9 @@ impl [T] { #[unstable(feature = "slice_swap_unchecked", issue = "88539")] #[rustc_const_unstable(feature = "const_swap", issue = "83163")] pub const unsafe fn swap_unchecked(&mut self, a: usize, b: usize) { - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The indices might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( a < self.len() && b < self.len(), "slice::swap_unchecked requires that the indices are within the slice" ); @@ -1285,7 +1287,7 @@ impl [T] { #[inline] #[must_use] pub const unsafe fn as_chunks_unchecked(&self) -> &[[T; N]] { - debug_assert_nounwind!( + debug_assert_ubcheck!( N != 0 && self.len() % N == 0, "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks" ); @@ -1439,7 +1441,7 @@ impl [T] { #[inline] #[must_use] pub const unsafe fn as_chunks_unchecked_mut(&mut self) -> &mut [[T; N]] { - debug_assert_nounwind!( + debug_assert_ubcheck!( N != 0 && self.len() % N == 0, "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks" ); @@ -1971,7 +1973,9 @@ impl [T] { let len = self.len(); let ptr = self.as_ptr(); - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (`mid` might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( mid <= len, "slice::split_at_unchecked requires the index to be within the slice" ); @@ -2021,7 +2025,9 @@ impl [T] { let len = self.len(); let ptr = self.as_mut_ptr(); - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (`mid`` might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( mid <= len, "slice::split_at_mut_unchecked requires the index to be within the slice" ); diff --git a/library/core/src/str/traits.rs b/library/core/src/str/traits.rs index 3b9e9c9812719..337d85fc0fe3a 100644 --- a/library/core/src/str/traits.rs +++ b/library/core/src/str/traits.rs @@ -2,7 +2,7 @@ use crate::cmp::Ordering; use crate::ops; -use crate::panic::debug_assert_nounwind; +use crate::panic::debug_assert_ubcheck; use crate::ptr; use crate::slice::SliceIndex; @@ -192,7 +192,9 @@ unsafe impl SliceIndex for ops::Range { unsafe fn get_unchecked(self, slice: *const str) -> *const Self::Output { let slice = slice as *const [u8]; - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The end might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( // We'd like to check that the bounds are on char boundaries, // but there's not really a way to do so without reading // behind the pointer, which has aliasing implications. @@ -213,7 +215,9 @@ unsafe impl SliceIndex for ops::Range { unsafe fn get_unchecked_mut(self, slice: *mut str) -> *mut Self::Output { let slice = slice as *mut [u8]; - debug_assert_nounwind!( + // FIXME: violating this is not immediate language UB, so the interpreter will miss out on + // this check. (The end might be OOB of the slice but still inbounds of the allocation.) + debug_assert_ubcheck!( self.end >= self.start && self.end <= slice.len(), "str::get_unchecked_mut requires that the range is within the string slice" );