From 7e411bce92add1cdb1877ba0908fc465f76b95f5 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 25 Feb 2024 10:51:29 +0100
Subject: [PATCH] =?UTF-8?q?rename=20debug=5Fassert=5Fnounwind=20=E2=86=92?=
=?UTF-8?q?=20debug=5Fassert=5Fubcheck?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
library/core/src/ops/index_range.rs | 4 +++-
library/core/src/panic.rs | 18 +++++++++++-------
library/core/src/ptr/alignment.rs | 2 +-
library/core/src/slice/index.rs | 24 +++++++++++++++++-------
library/core/src/slice/mod.rs | 18 ++++++++++++------
library/core/src/str/traits.rs | 10 +++++++---
6 files changed, 51 insertions(+), 25 deletions(-)
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<T> 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<T> 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<T> 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<T> 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<T> SliceIndex<[T]> for ops::Range<usize> {
#[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<T> SliceIndex<[T]> for ops::Range<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 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> [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> [T] {
#[inline]
#[must_use]
pub const unsafe fn as_chunks_unchecked<const N: usize>(&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> [T] {
#[inline]
#[must_use]
pub const unsafe fn as_chunks_unchecked_mut<const N: usize>(&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> [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> [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<str> for ops::Range<usize> {
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<str> for ops::Range<usize> {
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"
);