From 5a93a59fd557f7d93ab0b6cc192c47580951666a Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Mon, 26 Feb 2024 21:25:27 -0500 Subject: [PATCH 1/5] Distinguish between library and lang UB in assert_unsafe_precondition --- compiler/rustc_borrowck/src/type_check/mod.rs | 2 +- compiler/rustc_codegen_cranelift/src/base.rs | 2 +- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 3 +- .../rustc_const_eval/src/interpret/step.rs | 14 +- .../src/transform/check_consts/check.rs | 2 +- .../src/transform/validate.rs | 2 +- .../rustc_hir_analysis/src/check/intrinsic.rs | 5 +- compiler/rustc_middle/src/mir/pretty.rs | 2 +- compiler/rustc_middle/src/mir/syntax.rs | 12 +- compiler/rustc_middle/src/mir/tcx.rs | 2 +- .../src/move_paths/builder.rs | 2 +- compiler/rustc_mir_transform/src/gvn.rs | 2 +- .../src/known_panics_lint.rs | 2 +- .../src/lower_intrinsics.rs | 21 +- .../rustc_mir_transform/src/promote_consts.rs | 2 +- .../rustc_smir/src/rustc_smir/convert/mir.rs | 8 +- compiler/rustc_span/src/symbol.rs | 2 + compiler/stable_mir/src/mir/body.rs | 10 +- library/core/src/cell.rs | 6 +- library/core/src/char/convert.rs | 1 + library/core/src/hint.rs | 11 +- library/core/src/intrinsics.rs | 194 +++++++++++------- library/core/src/num/nonzero.rs | 10 +- library/core/src/ops/index_range.rs | 9 +- library/core/src/panic.rs | 37 ---- library/core/src/ptr/alignment.rs | 9 +- library/core/src/ptr/const_ptr.rs | 40 ++-- library/core/src/ptr/mod.rs | 40 ++-- library/core/src/ptr/non_null.rs | 1 + library/core/src/slice/index.rs | 52 +++-- library/core/src/slice/mod.rs | 41 ++-- library/core/src/slice/raw.rs | 2 + library/core/src/str/traits.rs | 24 ++- .../clippy_utils/src/qualify_min_const_fn.rs | 2 +- ...n.DataflowConstProp.32bit.panic-abort.diff | 12 +- ....DataflowConstProp.32bit.panic-unwind.diff | 12 +- ...n.DataflowConstProp.64bit.panic-abort.diff | 12 +- ....DataflowConstProp.64bit.panic-unwind.diff | 12 +- ...oxed_slice.main.GVN.32bit.panic-abort.diff | 12 +- ...xed_slice.main.GVN.32bit.panic-unwind.diff | 12 +- ...oxed_slice.main.GVN.64bit.panic-abort.diff | 12 +- ...xed_slice.main.GVN.64bit.panic-unwind.diff | 12 +- ...d.unwrap_unchecked.Inline.panic-abort.diff | 2 +- ....unwrap_unchecked.Inline.panic-unwind.diff | 2 +- ...unchecked.PreCodegen.after.panic-abort.mir | 2 +- ...nchecked.PreCodegen.after.panic-unwind.mir | 2 +- ...witch_targets.ub_if_b.PreCodegen.after.mir | 2 +- 47 files changed, 416 insertions(+), 264 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 8d38b86fa3430..412d50f493eee 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -2000,7 +2000,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ConstraintCategory::SizedBound, ); } - &Rvalue::NullaryOp(NullOp::DebugAssertions, _) => {} + &Rvalue::NullaryOp(NullOp::UbCheck(_), _) => {} Rvalue::ShallowInitBox(operand, ty) => { self.check_operand(operand, location); diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 1ce920f3bdb79..2415c2c90b22c 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -779,7 +779,7 @@ fn codegen_stmt<'tcx>( NullOp::OffsetOf(fields) => { layout.offset_of_subfield(fx, fields.iter()).bytes() } - NullOp::DebugAssertions => { + NullOp::UbCheck(_) => { let val = fx.tcx.sess.opts.debug_assertions; let val = CValue::by_val( fx.bcx.ins().iconst(types::I8, i64::try_from(val).unwrap()), diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 9ae82d4845e16..8c6b9faf39d3e 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -685,7 +685,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let val = layout.offset_of_subfield(bx.cx(), fields.iter()).bytes(); bx.cx().const_usize(val) } - mir::NullOp::DebugAssertions => { + mir::NullOp::UbCheck(_) => { + // In codegen, we want to check for language UB and library UB let val = bx.tcx().sess.opts.debug_assertions; bx.cx().const_bool(val) } diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index d4c96f4573d96..752fdc2c6f847 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -258,10 +258,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let val = layout.offset_of_subfield(self, fields.iter()).bytes(); Scalar::from_target_usize(val, self) } - mir::NullOp::DebugAssertions => { - // The checks hidden behind this are always better done by the interpreter - // itself, because it knows the runtime state better. - Scalar::from_bool(false) + mir::NullOp::UbCheck(kind) => { + // We want to enable checks for library UB, because the interpreter doesn't + // know about those on its own. + // But we want to disable checks for language UB, because the interpreter + // has its own better checks for that. + let should_check = match kind { + mir::UbKind::LibraryUb => true, + mir::UbKind::LanguageUb => false, + }; + Scalar::from_bool(should_check) } }; self.write_scalar(val, &dest)?; diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 7ec78e7b9cfd6..53308cd7f901c 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -558,7 +558,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { Rvalue::Cast(_, _, _) => {} Rvalue::NullaryOp( - NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::DebugAssertions, + NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::UbCheck(_), _, ) => {} Rvalue::ShallowInitBox(_, _) => {} diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 74ba2f6039e86..f26c8f8592dea 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -1157,7 +1157,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { Rvalue::Repeat(_, _) | Rvalue::ThreadLocalRef(_) | Rvalue::AddressOf(_, _) - | Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::DebugAssertions, _) + | Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::UbCheck(_), _) | Rvalue::Discriminant(_) => {} } self.super_rvalue(rvalue, location); diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 95c51cc0486fc..35755a46df3a4 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -127,7 +127,8 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) - | sym::variant_count | sym::is_val_statically_known | sym::ptr_mask - | sym::debug_assertions + | sym::check_language_ub + | sym::check_library_ub | sym::fadd_algebraic | sym::fsub_algebraic | sym::fmul_algebraic @@ -584,7 +585,7 @@ pub fn check_intrinsic_type( (0, 0, vec![Ty::new_imm_ptr(tcx, Ty::new_unit(tcx))], tcx.types.usize) } - sym::debug_assertions => (0, 1, Vec::new(), tcx.types.bool), + sym::check_language_ub | sym::check_library_ub => (0, 1, Vec::new(), tcx.types.bool), sym::simd_eq | sym::simd_ne diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index e058302af312c..18069547a7ed3 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -915,7 +915,7 @@ impl<'tcx> Debug for Rvalue<'tcx> { NullOp::SizeOf => write!(fmt, "SizeOf({t})"), NullOp::AlignOf => write!(fmt, "AlignOf({t})"), NullOp::OffsetOf(fields) => write!(fmt, "OffsetOf({t}, {fields:?})"), - NullOp::DebugAssertions => write!(fmt, "cfg!(debug_assertions)"), + NullOp::UbCheck(kind) => write!(fmt, "UbCheck({kind:?})"), } } ThreadLocalRef(did) => ty::tls::with(|tcx| { diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index f188923f87618..0ab797134c0af 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -1366,8 +1366,16 @@ pub enum NullOp<'tcx> { AlignOf, /// Returns the offset of a field OffsetOf(&'tcx List<(VariantIdx, FieldIdx)>), - /// cfg!(debug_assertions), but expanded in codegen - DebugAssertions, + /// Returns whether we want to check for library UB or language UB at monomorphization time. + /// Both kinds of UB evaluate to `true` in codegen, and only library UB evalutes to `true` in + /// const-eval/Miri, because the interpreter has its own better checks for language UB. + UbCheck(UbKind), +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)] +pub enum UbKind { + LanguageUb, + LibraryUb, } #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index 5bc151de659c3..0c29fe57d4fc9 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -194,7 +194,7 @@ impl<'tcx> Rvalue<'tcx> { Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => { tcx.types.usize } - Rvalue::NullaryOp(NullOp::DebugAssertions, _) => tcx.types.bool, + Rvalue::NullaryOp(NullOp::UbCheck(_), _) => tcx.types.bool, Rvalue::Aggregate(ref ak, ref ops) => match **ak { AggregateKind::Array(ty) => Ty::new_array(tcx, ty, ops.len() as u64), AggregateKind::Tuple => { diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index db48ecd702bdb..0f900e6a5573a 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -433,7 +433,7 @@ impl<'b, 'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> Gatherer<'b, 'a, 'tcx, F> { | Rvalue::Discriminant(..) | Rvalue::Len(..) | Rvalue::NullaryOp( - NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::DebugAssertions, + NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::UbCheck(_), _, ) => {} } diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index a080e2423d47b..a3a2108787a73 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -488,7 +488,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { NullOp::OffsetOf(fields) => { layout.offset_of_subfield(&self.ecx, fields.iter()).bytes() } - NullOp::DebugAssertions => return None, + NullOp::UbCheck(_) => return None, }; let usize_layout = self.ecx.layout_of(self.tcx.types.usize).unwrap(); let imm = ImmTy::try_from_uint(val, usize_layout)?; diff --git a/compiler/rustc_mir_transform/src/known_panics_lint.rs b/compiler/rustc_mir_transform/src/known_panics_lint.rs index 27477769cef85..4bca437ea6f27 100644 --- a/compiler/rustc_mir_transform/src/known_panics_lint.rs +++ b/compiler/rustc_mir_transform/src/known_panics_lint.rs @@ -639,7 +639,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { NullOp::OffsetOf(fields) => { op_layout.offset_of_subfield(self, fields.iter()).bytes() } - NullOp::DebugAssertions => return None, + NullOp::UbCheck(_) => return None, }; ImmTy::from_scalar(Scalar::from_target_usize(val, self), layout).into() } diff --git a/compiler/rustc_mir_transform/src/lower_intrinsics.rs b/compiler/rustc_mir_transform/src/lower_intrinsics.rs index f317c025e966c..1bab240ef50f2 100644 --- a/compiler/rustc_mir_transform/src/lower_intrinsics.rs +++ b/compiler/rustc_mir_transform/src/lower_intrinsics.rs @@ -20,13 +20,30 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { sym::unreachable => { terminator.kind = TerminatorKind::Unreachable; } - sym::debug_assertions => { + sym::check_language_ub => { let target = target.unwrap(); block.statements.push(Statement { source_info: terminator.source_info, kind: StatementKind::Assign(Box::new(( *destination, - Rvalue::NullaryOp(NullOp::DebugAssertions, tcx.types.bool), + Rvalue::NullaryOp( + NullOp::UbCheck(UbKind::LanguageUb), + tcx.types.bool, + ), + ))), + }); + terminator.kind = TerminatorKind::Goto { target }; + } + sym::check_library_ub => { + let target = target.unwrap(); + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::Assign(Box::new(( + *destination, + Rvalue::NullaryOp( + NullOp::UbCheck(UbKind::LibraryUb), + tcx.types.bool, + ), ))), }); terminator.kind = TerminatorKind::Goto { target }; diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 2e11da4d585ef..9fe8c34a8bf5d 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -446,7 +446,7 @@ impl<'tcx> Validator<'_, 'tcx> { NullOp::SizeOf => {} NullOp::AlignOf => {} NullOp::OffsetOf(_) => {} - NullOp::DebugAssertions => {} + NullOp::UbCheck(_) => {} }, Rvalue::ShallowInitBox(_, _) => return Err(Unpromotable), diff --git a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs index 003a9a5920018..c0876adf90509 100644 --- a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs +++ b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs @@ -251,13 +251,19 @@ impl<'tcx> Stable<'tcx> for mir::NullOp<'tcx> { type T = stable_mir::mir::NullOp; fn stable(&self, tables: &mut Tables<'_>) -> Self::T { use rustc_middle::mir::NullOp::*; + use rustc_middle::mir::UbKind; match self { SizeOf => stable_mir::mir::NullOp::SizeOf, AlignOf => stable_mir::mir::NullOp::AlignOf, OffsetOf(indices) => stable_mir::mir::NullOp::OffsetOf( indices.iter().map(|idx| idx.stable(tables)).collect(), ), - DebugAssertions => stable_mir::mir::NullOp::DebugAssertions, + UbCheck(UbKind::LanguageUb) => { + stable_mir::mir::NullOp::UbCheck(stable_mir::mir::UbKind::LanguageUb) + } + UbCheck(UbKind::LibraryUb) => { + stable_mir::mir::NullOp::UbCheck(stable_mir::mir::UbKind::LibraryUb) + } } } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 9dadd47124771..31b2691d9fe87 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -518,6 +518,8 @@ symbols! { cfi, cfi_encoding, char, + check_language_ub, + check_library_ub, client, clippy, clobber_abi, diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index be727f024c609..ae8e71bb950a1 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -639,7 +639,7 @@ impl Rvalue { Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => { Ok(Ty::usize_ty()) } - Rvalue::NullaryOp(NullOp::DebugAssertions, _) => Ok(Ty::bool_ty()), + Rvalue::NullaryOp(NullOp::UbCheck(_), _) => Ok(Ty::bool_ty()), Rvalue::Aggregate(ak, ops) => match *ak { AggregateKind::Array(ty) => Ty::try_new_array(ty, ops.len() as u64), AggregateKind::Tuple => Ok(Ty::new_tuple( @@ -1007,7 +1007,13 @@ pub enum NullOp { /// Returns the offset of a field. OffsetOf(Vec<(VariantIdx, FieldIdx)>), /// cfg!(debug_assertions), but at codegen time - DebugAssertions, + UbCheck(UbKind), +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum UbKind { + LanguageUb, + LibraryUb, } impl Operand { diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 19b05448c8780..e8590ab115860 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -237,7 +237,6 @@ use crate::cmp::Ordering; use crate::fmt::{self, Debug, Display}; -use crate::intrinsics; use crate::marker::{PhantomData, Unsize}; use crate::mem::{self, size_of}; use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn}; @@ -436,7 +435,10 @@ impl Cell { #[stable(feature = "move_cell", since = "1.17.0")] pub fn swap(&self, other: &Self) { fn is_nonoverlapping(src: *const T, dst: *const T) -> bool { - intrinsics::is_nonoverlapping(src.cast(), dst.cast(), size_of::(), 1) + let src_usize = src.addr(); + let dst_usize = dst.addr(); + let diff = src_usize.abs_diff(dst_usize); + diff >= size_of::() } if ptr::eq(self, other) { diff --git a/library/core/src/char/convert.rs b/library/core/src/char/convert.rs index 7bd592492a5c3..70b9e89f9ea93 100644 --- a/library/core/src/char/convert.rs +++ b/library/core/src/char/convert.rs @@ -26,6 +26,7 @@ pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char { // SAFETY: the caller must guarantee that `i` is a valid char value. unsafe { assert_unsafe_precondition!( + check_language_ub, "invalid value for `char`", (i: u32 = i) => char_try_from_u32(i).is_ok() ); diff --git a/library/core/src/hint.rs b/library/core/src/hint.rs index 97c3c9e6fae95..ffe059bf65cad 100644 --- a/library/core/src/hint.rs +++ b/library/core/src/hint.rs @@ -98,12 +98,14 @@ use crate::intrinsics; #[rustc_const_stable(feature = "const_unreachable_unchecked", since = "1.57.0")] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub const unsafe fn unreachable_unchecked() -> ! { + intrinsics::assert_unsafe_precondition!( + check_language_ub, + "hint::unreachable_unchecked must never be reached", + () => false + ); // SAFETY: the safety contract for `intrinsics::unreachable` must // be upheld by the caller. - unsafe { - intrinsics::assert_unsafe_precondition!("hint::unreachable_unchecked must never be reached", () => false); - intrinsics::unreachable() - } + unsafe { intrinsics::unreachable() } } /// Makes a *soundness* promise to the compiler that `cond` holds. @@ -147,6 +149,7 @@ pub const unsafe fn assert_unchecked(cond: bool) { // SAFETY: The caller promised `cond` is true. unsafe { intrinsics::assert_unsafe_precondition!( + check_language_ub, "hint::assert_unchecked must never be called when the condition is false", (cond: bool = cond) => cond, ); diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index aff1c589e628a..eda8b7842ab4a 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2628,24 +2628,38 @@ pub const fn is_val_statically_known(_arg: T) -> bool { false } -/// Returns the value of `cfg!(debug_assertions)`, but after monomorphization instead of in -/// macro expansion. -/// -/// This always returns `false` in const eval and Miri. The interpreter provides better -/// diagnostics than the checks that this is used to implement. However, this means -/// you should only be using this intrinsic to guard requirements that, if violated, -/// immediately lead to UB. Otherwise, const-eval and Miri will miss out on those -/// checks entirely. -/// -/// Since this is evaluated after monomorphization, branching on this value can be used to -/// implement debug assertions that are included in the precompiled standard library, but can -/// be optimized out by builds that monomorphize the standard library code with debug +/// Returns whether we should check for library UB. This evaluate to the value of `cfg!(debug_assertions)` +/// during monomorphization. +/// +/// This intrinsic is evaluated after monomorphization, and therefore branching on this value can +/// be used to implement debug assertions that are included in the precompiled standard library, +/// but can be optimized out by builds that monomorphize the standard library code with debug /// assertions disabled. This intrinsic is primarily used by [`assert_unsafe_precondition`]. -#[rustc_const_unstable(feature = "delayed_debug_assertions", issue = "none")] +/// +/// We have separate intrinsics for library UB and language UB because checkers like the const-eval +/// interpreter and Miri already implement checks for language UB. Since such checkers do not know +/// about library preconditions, checks guarded by this intrinsic let them find more UB. +#[rustc_const_unstable(feature = "ub_checks", issue = "none")] +#[unstable(feature = "core_intrinsics", issue = "none")] +#[inline(always)] +#[cfg_attr(not(bootstrap), rustc_intrinsic)] +pub(crate) const fn check_library_ub() -> bool { + cfg!(debug_assertions) +} + +/// Returns whether we should check for language UB. This evaluate to the value of `cfg!(debug_assertions)` +/// during monomorphization. +/// +/// Since checks implemented at the source level must come strictly before the operation that +/// executes UB, if we enabled language UB checks in const-eval/Miri we would miss out on the +/// interpreter's improved diagnostics for the cases that our source-level checks catch. +/// +/// See `check_library_ub` for more information. +#[rustc_const_unstable(feature = "ub_checks", issue = "none")] #[unstable(feature = "core_intrinsics", issue = "none")] #[inline(always)] #[cfg_attr(not(bootstrap), rustc_intrinsic)] -pub(crate) const fn debug_assertions() -> bool { +pub(crate) const fn check_language_ub() -> bool { cfg!(debug_assertions) } @@ -2700,13 +2714,24 @@ pub unsafe fn vtable_size(_ptr: *const ()) -> usize { // (`transmute` also falls into this category, but it cannot be wrapped due to the // check that `T` and `U` have the same size.) -/// Check that the preconditions of an unsafe function are followed, if debug_assertions are on, -/// and only at runtime. +/// Check that the preconditions of an unsafe function are followed. The check is enabled at +/// runtime if debug assertions are enabled when the caller is monomorphized. In const-eval/Miri +/// checks implemented with this macro for language UB are always ignored. /// /// This macro should be called as -/// `assert_unsafe_precondition!((expr => name: Type, expr => name: Type) => Expression)` -/// where each `expr` will be evaluated and passed in as function argument `name: Type`. Then all -/// those arguments are passed to a function via [`const_eval_select`]. +/// `assert_unsafe_precondition!(check_{library,lang}_ub, "message", (ident: type = expr, ident: type = expr) => check_expr)` +/// where each `expr` will be evaluated and passed in as function argument `ident: type`. Then all +/// those arguments are passed to a function with the body `check_expr`. +/// Pick `check_language_ub` when this is guarding a violation of language UB, i.e., immediate UB +/// according to the Rust Abstract Machine. Pick `check_library_ub` when this is guarding a violation +/// of a documented library precondition that does not *immediately* lead to language UB. +/// +/// If `check_library_ub` is used but the check is actually guarding language UB, the check will +/// slow down const-eval/Miri and we'll get the panic message instead of the interpreter's nice +/// diagnostic, but our ability to detect UB is unchanged. +/// But if `check_language_ub` is used when the check is actually for library UB, the check is +/// omitted in const-eval/Miri and thus if we eventually execute language UB which relies on the +/// library UB, the backtrace Miri reports may be far removed from original cause. /// /// These checks are behind a condition which is evaluated at codegen time, not expansion time like /// [`debug_assert`]. This means that a standard library built with optimizations and debug @@ -2715,31 +2740,25 @@ pub unsafe fn vtable_size(_ptr: *const ()) -> usize { /// this macro, that monomorphization will contain the check. /// /// Since these checks cannot be optimized out in MIR, some care must be taken in both call and -/// implementation to mitigate their compile-time overhead. The runtime function that we -/// [`const_eval_select`] to is monomorphic, `#[inline(never)]`, and `#[rustc_nounwind]`. That -/// combination of properties ensures that the code for the checks is only compiled once, and has a -/// minimal impact on the caller's code size. +/// implementation to mitigate their compile-time overhead. Calls to this macro always expand to +/// this structure: +/// ```ignore (pseudocode) +/// if ::core::intrinsics::check_language_ub() { +/// precondition_check(args) +/// } +/// ``` +/// where `precondition_check` is monomorphic with the attributes `#[rustc_nounwind]`, `#[inline]` and +/// `#[rustc_no_mir_inline]`. This combination of attributes ensures that the actual check logic is +/// compiled only once and generates a minimal amount of IR because the check cannot be inlined in +/// MIR, but *can* be inlined and fully optimized by a codegen backend. /// -/// Callers should also avoid introducing any other `let` bindings or any code outside this macro in +/// Callers should avoid introducing any other `let` bindings or any code outside this macro in /// order to call it. Since the precompiled standard library is built with full debuginfo and these /// variables cannot be optimized out in MIR, an innocent-looking `let` can produce enough /// debuginfo to have a measurable compile-time impact on debug builds. -/// -/// # Safety -/// -/// Invoking this macro is only sound if the following code is already UB when the passed -/// expression evaluates to false. -/// -/// This macro expands to a check at runtime if debug_assertions is set. It has no effect at -/// compile time, but the semantics of the contained `const_eval_select` must be the same at -/// runtime and at compile time. Thus if the expression evaluates to false, this macro produces -/// different behavior at compile time and at runtime, and invoking it is incorrect. -/// -/// So in a sense it is UB if this macro is useful, but we expect callers of `unsafe fn` to make -/// the occasional mistake, and this check should help them figure things out. -#[allow_internal_unstable(const_eval_select, delayed_debug_assertions)] // permit this to be called in stably-const fn +#[allow_internal_unstable(ub_checks)] // permit this to be called in stably-const fn macro_rules! assert_unsafe_precondition { - ($message:expr, ($($name:ident:$ty:ty = $arg:expr),*$(,)?) => $e:expr $(,)?) => { + ($kind:ident, $message:expr, ($($name:ident:$ty:ty = $arg:expr),*$(,)?) => $e:expr $(,)?) => { { // #[cfg(bootstrap)] (this comment) // When the standard library is compiled with debug assertions, we want the check to inline for better performance. @@ -2761,17 +2780,17 @@ macro_rules! assert_unsafe_precondition { #[cfg_attr(not(bootstrap), rustc_no_mir_inline)] #[cfg_attr(not(bootstrap), inline)] #[rustc_nounwind] - fn precondition_check($($name:$ty),*) { + #[rustc_const_unstable(feature = "ub_checks", issue = "none")] + const fn precondition_check($($name:$ty),*) { if !$e { ::core::panicking::panic_nounwind( concat!("unsafe precondition(s) violated: ", $message) ); } } - const fn comptime($(_:$ty),*) {} - if ::core::intrinsics::debug_assertions() { - ::core::intrinsics::const_eval_select(($($arg,)*), comptime, precondition_check); + if ::core::intrinsics::$kind() { + precondition_check($($arg,)*); } } }; @@ -2781,31 +2800,55 @@ pub(crate) use assert_unsafe_precondition; /// Checks whether `ptr` is properly aligned with respect to /// `align_of::()`. #[inline] -pub(crate) fn is_aligned_and_not_null(ptr: *const (), align: usize) -> bool { +pub(crate) const fn is_aligned_and_not_null(ptr: *const (), align: usize) -> bool { !ptr.is_null() && ptr.is_aligned_to(align) } #[inline] -pub(crate) fn is_valid_allocation_size(size: usize, len: usize) -> bool { +pub(crate) const fn is_valid_allocation_size(size: usize, len: usize) -> bool { let max_len = if size == 0 { usize::MAX } else { isize::MAX as usize / size }; len <= max_len } /// Checks whether the regions of memory starting at `src` and `dst` of size /// `count * size` do *not* overlap. +/// +/// # Safety +/// This function must only be called such that if it returns false, we will execute UB. #[inline] -pub(crate) fn is_nonoverlapping(src: *const (), dst: *const (), size: usize, count: usize) -> bool { - let src_usize = src.addr(); - let dst_usize = dst.addr(); - let Some(size) = size.checked_mul(count) else { - crate::panicking::panic_nounwind( - "is_nonoverlapping: `size_of::() * count` overflows a usize", - ) - }; - let diff = src_usize.abs_diff(dst_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 +pub(crate) const unsafe fn is_nonoverlapping( + src: *const (), + dst: *const (), + size: usize, + count: usize, +) -> bool { + #[inline] + fn runtime(src: *const (), dst: *const (), size: usize, count: usize) -> bool { + let src_usize = src.addr(); + let dst_usize = dst.addr(); + let Some(size) = size.checked_mul(count) else { + crate::panicking::panic_nounwind( + "is_nonoverlapping: `size_of::() * count` overflows a usize", + ) + }; + let diff = src_usize.abs_diff(dst_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 + } + + #[inline] + const fn comptime(_: *const (), _: *const (), _: usize, _: usize) -> bool { + true + } + + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] + // SAFETY: This function's precondition is equivalent to that of `const_eval_select`. + // Programs which do not execute UB will only see this function return `true`, which makes the + // const and runtime implementation indistinguishable. + unsafe { + const_eval_select((src, dst, size, count), comptime, runtime) + } } /// Copies `count * size_of::()` bytes from `src` to `dst`. The source @@ -2906,25 +2949,26 @@ 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); } + assert_unsafe_precondition!( + check_language_ub, + "ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \ + and the specified memory ranges do not overlap", + ( + src: *const () = src as *const (), + dst: *mut () = dst as *mut (), + size: usize = size_of::(), + align: usize = align_of::(), + count: usize = count, + ) => + is_aligned_and_not_null(src, align) + && is_aligned_and_not_null(dst, align) + // SAFETY: If this returns false, we're about to execute UB. + && unsafe { is_nonoverlapping(src, dst, size, count) } + ); + // SAFETY: the safety contract for `copy_nonoverlapping` must be // upheld by the caller. - unsafe { - assert_unsafe_precondition!( - "ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \ - and the specified memory ranges do not overlap", - ( - src: *const () = src as *const (), - dst: *mut () = dst as *mut (), - size: usize = size_of::(), - align: usize = align_of::(), - count: usize = count, - ) => - is_aligned_and_not_null(src, align) - && is_aligned_and_not_null(dst, align) - && is_nonoverlapping(src, dst, size, count) - ); - copy_nonoverlapping(src, dst, count) - } + unsafe { copy_nonoverlapping(src, dst, count) } } /// Copies `count * size_of::()` bytes from `src` to `dst`. The source @@ -3011,6 +3055,7 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { // SAFETY: the safety contract for `copy` must be upheld by the caller. unsafe { assert_unsafe_precondition!( + check_language_ub, "ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \ and the specified memory ranges do not overlap", ( @@ -3091,6 +3136,7 @@ pub const unsafe fn write_bytes(dst: *mut T, val: u8, count: usize) { // SAFETY: the safety contract for `write_bytes` must be upheld by the caller. unsafe { assert_unsafe_precondition!( + check_language_ub, "ptr::write_bytes requires that the destination pointer is aligned and non-null", ( addr: *const () = dst as *const (), diff --git a/library/core/src/num/nonzero.rs b/library/core/src/num/nonzero.rs index 9e34c0d240dc2..012f224a4a265 100644 --- a/library/core/src/num/nonzero.rs +++ b/library/core/src/num/nonzero.rs @@ -328,8 +328,9 @@ where // SAFETY: The caller guarantees that `n` is non-zero, so this is unreachable. unsafe { intrinsics::assert_unsafe_precondition!( - "NonZero::new_unchecked requires the argument to be non-zero", - () => false, + check_language_ub, + "NonZero::new_unchecked requires the argument to be non-zero", + () => false, ); intrinsics::unreachable() } @@ -367,8 +368,9 @@ where // SAFETY: The caller guarantees that `n` references a value that is non-zero, so this is unreachable. unsafe { intrinsics::assert_unsafe_precondition!( - "NonZero::from_mut_unchecked requires the argument to dereference as non-zero", - () => false, + check_language_ub, + "NonZero::from_mut_unchecked requires the argument to dereference as non-zero", + () => false, ); intrinsics::unreachable() } diff --git a/library/core/src/ops/index_range.rs b/library/core/src/ops/index_range.rs index 07ea2e930d57a..b28d88fa5bfc2 100644 --- a/library/core/src/ops/index_range.rs +++ b/library/core/src/ops/index_range.rs @@ -1,4 +1,4 @@ -use crate::intrinsics::{unchecked_add, unchecked_sub}; +use crate::intrinsics::{assert_unsafe_precondition, unchecked_add, unchecked_sub}; use crate::iter::{FusedIterator, TrustedLen}; use crate::num::NonZero; @@ -19,9 +19,10 @@ impl IndexRange { /// - `start <= end` #[inline] pub const unsafe fn new_unchecked(start: usize, end: usize) -> Self { - crate::panic::debug_assert_nounwind!( - start <= end, - "IndexRange::new_unchecked requires `start <= end`" + assert_unsafe_precondition!( + check_library_ub, + "IndexRange::new_unchecked requires `start <= end`", + (start: usize = start, end: usize = end) => start <= end, ); IndexRange { start, end } } diff --git a/library/core/src/panic.rs b/library/core/src/panic.rs index 2bd42a4a8cadc..b520efe93f90d 100644 --- a/library/core/src/panic.rs +++ b/library/core/src/panic.rs @@ -139,43 +139,6 @@ pub macro unreachable_2021 { ), } -/// Like `assert_unsafe_precondition!` the defining features of this macro are that its -/// 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. -/// -/// 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 -/// checks tolerable. In such cases, place `#[cfg(debug_assertions)]` on the macro call. That will -/// disable the check in our precompiled standard library, but if a user wishes, they can still -/// enable the check by recompiling the standard library with debug assertions enabled. -#[doc(hidden)] -#[unstable(feature = "panic_internals", issue = "none")] -#[allow_internal_unstable(panic_internals, delayed_debug_assertions)] -#[rustc_macro_transparency = "semitransparent"] -pub macro debug_assert_nounwind { - ($cond:expr $(,)?) => { - if $crate::intrinsics::debug_assertions() { - if !$cond { - $crate::panicking::panic_nounwind($crate::concat!("assertion failed: ", $crate::stringify!($cond))); - } - } - }, - ($cond:expr, $message:expr) => { - if $crate::intrinsics::debug_assertions() { - if !$cond { - $crate::panicking::panic_nounwind($message); - } - } - }, -} - /// An internal trait used by std to pass data from std to `panic_unwind` and /// other panic runtimes. Not intended to be stabilized any time soon, do not /// use. diff --git a/library/core/src/ptr/alignment.rs b/library/core/src/ptr/alignment.rs index 3508b0c7f238d..8f44b7eb7c28d 100644 --- a/library/core/src/ptr/alignment.rs +++ b/library/core/src/ptr/alignment.rs @@ -1,4 +1,6 @@ use crate::convert::{TryFrom, TryInto}; +#[cfg(debug_assertions)] +use crate::intrinsics::assert_unsafe_precondition; use crate::num::NonZero; use crate::{cmp, fmt, hash, mem, num}; @@ -77,9 +79,10 @@ impl Alignment { #[inline] pub const unsafe fn new_unchecked(align: usize) -> Self { #[cfg(debug_assertions)] - crate::panic::debug_assert_nounwind!( - align.is_power_of_two(), - "Alignment::new_unchecked requires a power of two" + assert_unsafe_precondition!( + check_language_ub, + "Alignment::new_unchecked requires a power of two", + (align: usize = align) => align.is_power_of_two() ); // SAFETY: By precondition, this must be a power of two, and diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 0c69bf2aef9c1..764f5d1a76a1e 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -48,7 +48,8 @@ impl *const T { } } - #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block + // on bootstrap bump, remove unsafe block + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // SAFETY: The two versions are equivalent at runtime. unsafe { const_eval_select((self as *const u8,), const_impl, runtime_impl) @@ -809,18 +810,31 @@ impl *const T { where T: Sized, { - #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block - // SAFETY: The comparison has no side-effects, and the intrinsic - // does this check internally in the CTFE implementation. - unsafe { - assert_unsafe_precondition!( - "ptr::sub_ptr requires `self >= origin`", - ( - this: *const () = self as *const (), - origin: *const () = origin as *const (), - ) => this >= origin - ) - }; + const fn runtime_ptr_ge(this: *const (), origin: *const ()) -> bool { + fn runtime(this: *const (), origin: *const ()) -> bool { + this >= origin + } + const fn comptime(_: *const (), _: *const ()) -> bool { + true + } + + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] + // on bootstrap bump, remove unsafe block + // SAFETY: This function is only used to provide the same check that the const eval + // interpreter does at runtime. + unsafe { + intrinsics::const_eval_select((this, origin), comptime, runtime) + } + } + + assert_unsafe_precondition!( + check_language_ub, + "ptr::sub_ptr requires `self >= origin`", + ( + this: *const () = self as *const (), + origin: *const () = origin as *const (), + ) => runtime_ptr_ge(this, origin) + ); let pointee_size = mem::size_of::(); assert!(0 < pointee_size && pointee_size <= isize::MAX as usize); diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index caa8ffb271dd4..80a6d5779e142 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1019,25 +1019,22 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { }; } - #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block - // SAFETY: the caller must guarantee that `x` and `y` are - // valid for writes and properly aligned. - unsafe { - assert_unsafe_precondition!( - "ptr::swap_nonoverlapping requires that both pointer arguments are aligned and non-null \ - and the specified memory ranges do not overlap", - ( - x: *mut () = x as *mut (), - y: *mut () = y as *mut (), - size: usize = size_of::(), - align: usize = align_of::(), - count: usize = count, - ) => - is_aligned_and_not_null(x, align) - && is_aligned_and_not_null(y, align) - && is_nonoverlapping(x, y, size, count) - ); - } + assert_unsafe_precondition!( + check_language_ub, + "ptr::swap_nonoverlapping requires that both pointer arguments are aligned and non-null \ + and the specified memory ranges do not overlap", + ( + x: *mut () = x as *mut (), + y: *mut () = y as *mut (), + size: usize = size_of::(), + align: usize = align_of::(), + count: usize = count, + ) => + is_aligned_and_not_null(x, align) + && is_aligned_and_not_null(y, align) + // SAFETY: If this returns false, we're about to execute UB. + && unsafe { is_nonoverlapping(x, y, size, count) } + ); // Split up the slice into small power-of-two-sized chunks that LLVM is able // to vectorize (unless it's a special type with more-than-pointer alignment, @@ -1125,6 +1122,7 @@ pub const unsafe fn replace(dst: *mut T, mut src: T) -> T { // allocated object. unsafe { assert_unsafe_precondition!( + check_language_ub, "ptr::replace requires that the pointer argument is aligned and non-null", ( addr: *const () = dst as *const (), @@ -1277,6 +1275,7 @@ pub const unsafe fn read(src: *const T) -> T { unsafe { #[cfg(debug_assertions)] // Too expensive to always enable (for now?) assert_unsafe_precondition!( + check_language_ub, "ptr::read requires that the pointer argument is aligned and non-null", ( addr: *const () = src as *const (), @@ -1485,6 +1484,7 @@ pub const unsafe fn write(dst: *mut T, src: T) { unsafe { #[cfg(debug_assertions)] // Too expensive to always enable (for now?) assert_unsafe_precondition!( + check_language_ub, "ptr::write requires that the pointer argument is aligned and non-null", ( addr: *mut () = dst as *mut (), @@ -1656,6 +1656,7 @@ pub unsafe fn read_volatile(src: *const T) -> T { // SAFETY: the caller must uphold the safety contract for `volatile_load`. unsafe { assert_unsafe_precondition!( + check_language_ub, "ptr::read_volatile requires that the pointer argument is aligned and non-null", ( addr: *const () = src as *const (), @@ -1734,6 +1735,7 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { // SAFETY: the caller must uphold the safety contract for `volatile_store`. unsafe { assert_unsafe_precondition!( + check_language_ub, "ptr::write_volatile requires that the pointer argument is aligned and non-null", ( addr: *mut () = dst as *mut (), diff --git a/library/core/src/ptr/non_null.rs b/library/core/src/ptr/non_null.rs index acb8c552a6338..9c0236c172a93 100644 --- a/library/core/src/ptr/non_null.rs +++ b/library/core/src/ptr/non_null.rs @@ -218,6 +218,7 @@ impl NonNull { // SAFETY: the caller must guarantee that `ptr` is non-null. unsafe { assert_unsafe_precondition!( + check_language_ub, "NonNull::new_unchecked requires that the pointer is non-null", (ptr: *mut () = ptr as *mut ()) => !ptr.is_null() ); diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index c771ea7047242..312dccbf10992 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -1,9 +1,9 @@ //! Indexing implementations for `[T]`. +use crate::intrinsics::assert_unsafe_precondition; use crate::intrinsics::const_eval_select; use crate::intrinsics::unchecked_sub; use crate::ops; -use crate::panic::debug_assert_nounwind; use crate::ptr; #[stable(feature = "rust1", since = "1.0.0")] @@ -230,9 +230,10 @@ unsafe impl SliceIndex<[T]> for usize { #[inline] unsafe fn get_unchecked(self, slice: *const [T]) -> *const T { - debug_assert_nounwind!( - self < slice.len(), - "slice::get_unchecked requires that the index is within the slice" + assert_unsafe_precondition!( + check_language_ub, + "slice::get_unchecked requires that the index is within the slice", + (this: usize = self, len: usize = slice.len()) => this < len ); // SAFETY: the caller guarantees that `slice` is not dangling, so it // cannot be longer than `isize::MAX`. They also guarantee that @@ -248,9 +249,10 @@ unsafe impl SliceIndex<[T]> for usize { #[inline] unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut T { - debug_assert_nounwind!( - self < slice.len(), - "slice::get_unchecked_mut requires that the index is within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::get_unchecked_mut requires that the index is within the slice", + (this: usize = self, len: usize = slice.len()) => this < len ); // SAFETY: see comments for `get_unchecked` above. unsafe { slice.as_mut_ptr().add(self) } @@ -297,9 +299,10 @@ unsafe impl SliceIndex<[T]> for ops::IndexRange { #[inline] unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { - debug_assert_nounwind!( - self.end() <= slice.len(), - "slice::get_unchecked requires that the index is within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::get_unchecked requires that the index is within the slice", + (end: usize = self.end(), len: usize = slice.len()) => end <= len ); // SAFETY: the caller guarantees that `slice` is not dangling, so it // cannot be longer than `isize::MAX`. They also guarantee that @@ -310,9 +313,10 @@ unsafe impl SliceIndex<[T]> for ops::IndexRange { #[inline] unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] { - debug_assert_nounwind!( - self.end() <= slice.len(), - "slice::get_unchecked_mut requires that the index is within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::get_unchecked_mut requires that the index is within the slice", + (end: usize = self.end(), len: usize = slice.len()) => end <= len ); // SAFETY: see comments for `get_unchecked` above. @@ -367,9 +371,14 @@ unsafe impl SliceIndex<[T]> for ops::Range { #[inline] unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { - debug_assert_nounwind!( - self.end >= self.start && self.end <= slice.len(), - "slice::get_unchecked requires that the range is within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::get_unchecked requires that the range is within the slice", + ( + start: usize = self.start, + end: usize = self.end, + len: usize = slice.len() + ) => end >= start && end <= len ); // SAFETY: the caller guarantees that `slice` is not dangling, so it @@ -384,9 +393,14 @@ unsafe impl SliceIndex<[T]> for ops::Range { #[inline] unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] { - debug_assert_nounwind!( - self.end >= self.start && self.end <= slice.len(), - "slice::get_unchecked_mut requires that the range is within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::get_unchecked_mut requires that the range is within the slice", + ( + start: usize = self.start, + end: usize = self.end, + len: usize = slice.len() + ) => end >= start && end <= len ); // SAFETY: see comments for `get_unchecked` above. unsafe { diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 2d5e091feaae8..2dd01ba33afc8 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -9,11 +9,11 @@ use crate::cmp::Ordering::{self, Equal, Greater, Less}; use crate::fmt; use crate::hint; +use crate::intrinsics::assert_unsafe_precondition; 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::ptr; use crate::simd::{self, Simd}; use crate::slice; @@ -945,9 +945,14 @@ 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!( - a < self.len() && b < self.len(), - "slice::swap_unchecked requires that the indices are within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::swap_unchecked requires that the indices are within the slice", + ( + len: usize = self.len(), + a: usize = a, + b: usize = b, + ) => a < len && b < len, ); let ptr = self.as_mut_ptr(); @@ -1285,9 +1290,10 @@ impl [T] { #[inline] #[must_use] pub const unsafe fn as_chunks_unchecked(&self) -> &[[T; N]] { - debug_assert_nounwind!( - N != 0 && self.len() % N == 0, - "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks" + assert_unsafe_precondition!( + check_language_ub, + "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks", + (n: usize = N, len: usize = self.len()) => n != 0 && len % n == 0, ); // SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length let new_len = unsafe { exact_div(self.len(), N) }; @@ -1439,9 +1445,10 @@ impl [T] { #[inline] #[must_use] pub const unsafe fn as_chunks_unchecked_mut(&mut self) -> &mut [[T; N]] { - debug_assert_nounwind!( - N != 0 && self.len() % N == 0, - "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks" + assert_unsafe_precondition!( + check_language_ub, + "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks", + (n: usize = N, len: usize = self.len()) => n != 0 && len % n == 0 ); // SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length let new_len = unsafe { exact_div(self.len(), N) }; @@ -1971,9 +1978,10 @@ impl [T] { let len = self.len(); let ptr = self.as_ptr(); - debug_assert_nounwind!( - mid <= len, - "slice::split_at_unchecked requires the index to be within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::split_at_unchecked requires the index to be within the slice", + (mid: usize = mid, len: usize = len) => mid <= len, ); // SAFETY: Caller has to check that `0 <= mid <= self.len()` @@ -2021,9 +2029,10 @@ impl [T] { let len = self.len(); let ptr = self.as_mut_ptr(); - debug_assert_nounwind!( - mid <= len, - "slice::split_at_mut_unchecked requires the index to be within the slice" + assert_unsafe_precondition!( + check_library_ub, + "slice::split_at_mut_unchecked requires the index to be within the slice", + (mid: usize = mid, len: usize = len) => mid <= len, ); // SAFETY: Caller has to check that `0 <= mid <= self.len()`. diff --git a/library/core/src/slice/raw.rs b/library/core/src/slice/raw.rs index 571abc3e99907..2199614ce27e4 100644 --- a/library/core/src/slice/raw.rs +++ b/library/core/src/slice/raw.rs @@ -96,6 +96,7 @@ pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] // SAFETY: the caller must uphold the safety contract for `from_raw_parts`. unsafe { assert_unsafe_precondition!( + check_language_ub, "slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`", ( data: *mut () = data as *mut (), @@ -149,6 +150,7 @@ pub const unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a m // SAFETY: the caller must uphold the safety contract for `from_raw_parts_mut`. unsafe { assert_unsafe_precondition!( + check_language_ub, "slice::from_raw_parts_mut requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`", ( data: *mut () = data as *mut (), diff --git a/library/core/src/str/traits.rs b/library/core/src/str/traits.rs index 3b9e9c9812719..ec81fd095d530 100644 --- a/library/core/src/str/traits.rs +++ b/library/core/src/str/traits.rs @@ -1,8 +1,8 @@ //! Trait implementations for `str`. use crate::cmp::Ordering; +use crate::intrinsics::assert_unsafe_precondition; use crate::ops; -use crate::panic::debug_assert_nounwind; use crate::ptr; use crate::slice::SliceIndex; @@ -192,15 +192,20 @@ 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!( + assert_unsafe_precondition!( // 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. // It's also not possible to move this check up to // `str::get_unchecked` without adding a special function // to `SliceIndex` just for this. - self.end >= self.start && self.end <= slice.len(), - "str::get_unchecked requires that the range is within the string slice" + check_library_ub, + "str::get_unchecked requires that the range is within the string slice", + ( + start: usize = self.start, + end: usize = self.end, + len: usize = slice.len() + ) => end >= start && end <= len, ); // SAFETY: the caller guarantees that `self` is in bounds of `slice` @@ -213,9 +218,14 @@ 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!( - self.end >= self.start && self.end <= slice.len(), - "str::get_unchecked_mut requires that the range is within the string slice" + assert_unsafe_precondition!( + check_library_ub, + "str::get_unchecked_mut requires that the range is within the string slice", + ( + start: usize = self.start, + end: usize = self.end, + len: usize = slice.len() + ) => end >= start && end <= len, ); // SAFETY: see comments for `get_unchecked`. diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index e369cb9d0a40e..dadb0d662ce8f 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -174,7 +174,7 @@ fn check_rvalue<'tcx>( )) } }, - Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::DebugAssertions, _) + Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::UbCheck(_), _) | Rvalue::ShallowInitBox(_, _) => Ok(()), Rvalue::UnaryOp(_, operand) => { let ty = operand.ty(body, tcx); diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-abort.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-abort.diff index e6b8d5e6c21bc..a958e5541fadb 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-abort.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-abort.diff @@ -58,11 +58,10 @@ _7 = const 1_usize; _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb3, otherwise: bb2]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb4, otherwise: bb2]; } bb1: { @@ -71,16 +70,21 @@ } bb2: { + StorageLive(_10); _10 = const {0x1 as *mut ()}; _9 = NonNull::::new_unchecked::precondition_check(const {0x1 as *mut ()}) -> [return: bb3, unwind unreachable]; } bb3: { + StorageDead(_10); + goto -> bb4; + } + + bb4: { StorageDead(_8); _11 = const {0x1 as *const [bool; 0]}; _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; StorageDead(_5); diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-unwind.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-unwind.diff index bd74591018bc7..b073e27729e20 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-unwind.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-unwind.diff @@ -58,11 +58,10 @@ _7 = const 1_usize; _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb4, otherwise: bb3]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb5, otherwise: bb3]; } bb1: { @@ -75,16 +74,21 @@ } bb3: { + StorageLive(_10); _10 = const {0x1 as *mut ()}; _9 = NonNull::::new_unchecked::precondition_check(const {0x1 as *mut ()}) -> [return: bb4, unwind unreachable]; } bb4: { + StorageDead(_10); + goto -> bb5; + } + + bb5: { StorageDead(_8); _11 = const {0x1 as *const [bool; 0]}; _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; StorageDead(_5); diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-abort.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-abort.diff index fdbb0b2df03ae..0a9f339ddbacb 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-abort.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-abort.diff @@ -58,11 +58,10 @@ _7 = const 1_usize; _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb3, otherwise: bb2]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb4, otherwise: bb2]; } bb1: { @@ -71,16 +70,21 @@ } bb2: { + StorageLive(_10); _10 = const {0x1 as *mut ()}; _9 = NonNull::::new_unchecked::precondition_check(const {0x1 as *mut ()}) -> [return: bb3, unwind unreachable]; } bb3: { + StorageDead(_10); + goto -> bb4; + } + + bb4: { StorageDead(_8); _11 = const {0x1 as *const [bool; 0]}; _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; StorageDead(_5); diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-unwind.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-unwind.diff index d6b5984b81dd6..bbc791148af1f 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-unwind.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-unwind.diff @@ -58,11 +58,10 @@ _7 = const 1_usize; _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb4, otherwise: bb3]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb5, otherwise: bb3]; } bb1: { @@ -75,16 +74,21 @@ } bb3: { + StorageLive(_10); _10 = const {0x1 as *mut ()}; _9 = NonNull::::new_unchecked::precondition_check(const {0x1 as *mut ()}) -> [return: bb4, unwind unreachable]; } bb4: { + StorageDead(_10); + goto -> bb5; + } + + bb5: { StorageDead(_8); _11 = const {0x1 as *const [bool; 0]}; _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; StorageDead(_5); diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff index c7445aaee6c55..3a11677f6f036 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff @@ -60,11 +60,10 @@ + _7 = const 1_usize; + _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb3, otherwise: bb2]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb4, otherwise: bb2]; } bb1: { @@ -73,6 +72,7 @@ } bb2: { + StorageLive(_10); - _10 = _6 as *mut () (PtrToPtr); - _9 = NonNull::::new_unchecked::precondition_check(move _10) -> [return: bb3, unwind unreachable]; + _10 = const {0x1 as *mut ()}; @@ -80,13 +80,17 @@ } bb3: { + StorageDead(_10); + goto -> bb4; + } + + bb4: { StorageDead(_8); - _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer)); - _5 = NonNull::<[bool; 0]> { pointer: _11 }; + _11 = const {0x1 as *const [bool; 0]}; + _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); - _4 = Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> }; + _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff index b8e961bc08750..9e7e08866b9d9 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff @@ -60,11 +60,10 @@ + _7 = const 1_usize; + _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb4, otherwise: bb3]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb5, otherwise: bb3]; } bb1: { @@ -77,6 +76,7 @@ } bb3: { + StorageLive(_10); - _10 = _6 as *mut () (PtrToPtr); - _9 = NonNull::::new_unchecked::precondition_check(move _10) -> [return: bb4, unwind unreachable]; + _10 = const {0x1 as *mut ()}; @@ -84,13 +84,17 @@ } bb4: { + StorageDead(_10); + goto -> bb5; + } + + bb5: { StorageDead(_8); - _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer)); - _5 = NonNull::<[bool; 0]> { pointer: _11 }; + _11 = const {0x1 as *const [bool; 0]}; + _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); - _4 = Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> }; + _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff index 9678db90d0590..beadfbc07b6ee 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff @@ -60,11 +60,10 @@ + _7 = const 1_usize; + _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb3, otherwise: bb2]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb4, otherwise: bb2]; } bb1: { @@ -73,6 +72,7 @@ } bb2: { + StorageLive(_10); - _10 = _6 as *mut () (PtrToPtr); - _9 = NonNull::::new_unchecked::precondition_check(move _10) -> [return: bb3, unwind unreachable]; + _10 = const {0x1 as *mut ()}; @@ -80,13 +80,17 @@ } bb3: { + StorageDead(_10); + goto -> bb4; + } + + bb4: { StorageDead(_8); - _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer)); - _5 = NonNull::<[bool; 0]> { pointer: _11 }; + _11 = const {0x1 as *const [bool; 0]}; + _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); - _4 = Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> }; + _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff index 8aa6c9c23e9a4..9ea86956b833c 100644 --- a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff +++ b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff @@ -60,11 +60,10 @@ + _7 = const 1_usize; + _6 = const {0x1 as *mut [bool; 0]}; StorageDead(_7); - StorageLive(_10); StorageLive(_11); StorageLive(_8); - _8 = cfg!(debug_assertions); - switchInt(move _8) -> [0: bb4, otherwise: bb3]; + _8 = UbCheck(LanguageUb); + switchInt(move _8) -> [0: bb5, otherwise: bb3]; } bb1: { @@ -77,6 +76,7 @@ } bb3: { + StorageLive(_10); - _10 = _6 as *mut () (PtrToPtr); - _9 = NonNull::::new_unchecked::precondition_check(move _10) -> [return: bb4, unwind unreachable]; + _10 = const {0x1 as *mut ()}; @@ -84,13 +84,17 @@ } bb4: { + StorageDead(_10); + goto -> bb5; + } + + bb5: { StorageDead(_8); - _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer)); - _5 = NonNull::<[bool; 0]> { pointer: _11 }; + _11 = const {0x1 as *const [bool; 0]}; + _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}; StorageDead(_11); - StorageDead(_10); StorageDead(_6); - _4 = Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> }; + _4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }}; diff --git a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-abort.diff b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-abort.diff index 52688c2e86729..6f7853a3e97f5 100644 --- a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-abort.diff +++ b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-abort.diff @@ -37,7 +37,7 @@ + + bb2: { + StorageLive(_4); -+ _4 = cfg!(debug_assertions); ++ _4 = UbCheck(LanguageUb); + assume(_4); + _5 = unreachable_unchecked::precondition_check() -> [return: bb1, unwind unreachable]; + } diff --git a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-unwind.diff b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-unwind.diff index fd83f1cb33152..cac06d4af088f 100644 --- a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-unwind.diff +++ b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.Inline.panic-unwind.diff @@ -41,7 +41,7 @@ - resume; + bb2: { + StorageLive(_4); -+ _4 = cfg!(debug_assertions); ++ _4 = UbCheck(LanguageUb); + assume(_4); + _5 = unreachable_unchecked::precondition_check() -> [return: bb1, unwind unreachable]; + } diff --git a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir index 8ec65935c6612..5c6116501548d 100644 --- a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir @@ -27,7 +27,7 @@ fn unwrap_unchecked(_1: Option) -> T { bb1: { StorageLive(_3); - _3 = cfg!(debug_assertions); + _3 = UbCheck(LanguageUb); assume(_3); _4 = unreachable_unchecked::precondition_check() -> [return: bb3, unwind unreachable]; } diff --git a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir index 8ec65935c6612..5c6116501548d 100644 --- a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir +++ b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir @@ -27,7 +27,7 @@ fn unwrap_unchecked(_1: Option) -> T { bb1: { StorageLive(_3); - _3 = cfg!(debug_assertions); + _3 = UbCheck(LanguageUb); assume(_3); _4 = unreachable_unchecked::precondition_check() -> [return: bb3, unwind unreachable]; } diff --git a/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir index 7c038b0ee88e7..0597e453e2218 100644 --- a/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir @@ -23,7 +23,7 @@ fn ub_if_b(_1: Thing) -> Thing { bb2: { StorageLive(_3); - _3 = cfg!(debug_assertions); + _3 = UbCheck(LanguageUb); assume(_3); _4 = unreachable_unchecked::precondition_check() -> [return: bb3, unwind unreachable]; } From 50d0bea15384f1116afd6831de3cbdc8c154e677 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 9 Mar 2024 10:49:26 -0500 Subject: [PATCH 2/5] Improve docs --- library/core/src/intrinsics.rs | 15 +++++++++------ library/core/src/ptr/mod.rs | 3 +-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index eda8b7842ab4a..7cda1225672b5 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2799,6 +2799,10 @@ pub(crate) use assert_unsafe_precondition; /// Checks whether `ptr` is properly aligned with respect to /// `align_of::()`. +/// +/// In `const` this is approximate and can fail spuriously. It is primarily intended +/// for `assert_unsafe_precondition!` with `check_language_ub`, in which case the +/// check is anyway not executed in `const`. #[inline] pub(crate) const fn is_aligned_and_not_null(ptr: *const (), align: usize) -> bool { !ptr.is_null() && ptr.is_aligned_to(align) @@ -2813,10 +2817,10 @@ pub(crate) const fn is_valid_allocation_size(size: usize, len: usize) -> bool { /// Checks whether the regions of memory starting at `src` and `dst` of size /// `count * size` do *not* overlap. /// -/// # Safety -/// This function must only be called such that if it returns false, we will execute UB. +/// Note that in const-eval this function just returns `true` and therefore must +/// only be used with `assert_unsafe_precondition!`, similar to `is_aligned_and_not_null`. #[inline] -pub(crate) const unsafe fn is_nonoverlapping( +pub(crate) const fn is_nonoverlapping( src: *const (), dst: *const (), size: usize, @@ -2842,7 +2846,7 @@ pub(crate) const unsafe fn is_nonoverlapping( true } - #[cfg_attr(not(bootstrap), allow(unused_unsafe))] + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: This function's precondition is equivalent to that of `const_eval_select`. // Programs which do not execute UB will only see this function return `true`, which makes the // const and runtime implementation indistinguishable. @@ -2962,8 +2966,7 @@ pub const unsafe fn copy_nonoverlapping(src: *const T, dst: *mut T, count: us ) => is_aligned_and_not_null(src, align) && is_aligned_and_not_null(dst, align) - // SAFETY: If this returns false, we're about to execute UB. - && unsafe { is_nonoverlapping(src, dst, size, count) } + && is_nonoverlapping(src, dst, size, count) ); // SAFETY: the safety contract for `copy_nonoverlapping` must be diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 80a6d5779e142..30af752236846 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1032,8 +1032,7 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { ) => is_aligned_and_not_null(x, align) && is_aligned_and_not_null(y, align) - // SAFETY: If this returns false, we're about to execute UB. - && unsafe { is_nonoverlapping(x, y, size, count) } + && is_nonoverlapping(x, y, size, count) ); // Split up the slice into small power-of-two-sized chunks that LLVM is able From 27cf4bb98516c801c8b24d80cd78cd5eee8edaff Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 9 Mar 2024 11:34:15 -0500 Subject: [PATCH 3/5] Only enable library UB checks in const-eval/Miri when debug_assertions are enabled Co-authored-by: Ralf Jung --- compiler/rustc_const_eval/src/interpret/step.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 752fdc2c6f847..54bac70da3881 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -264,7 +264,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // But we want to disable checks for language UB, because the interpreter // has its own better checks for that. let should_check = match kind { - mir::UbKind::LibraryUb => true, + mir::UbKind::LibraryUb => self.tcx.sess.opts.debug_assertions, mir::UbKind::LanguageUb => false, }; Scalar::from_bool(should_check) From af49c4df0a666f3ed0cefaae63851667dde9647d Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 9 Mar 2024 12:26:48 -0500 Subject: [PATCH 4/5] NonZero::from_mut_unchecked is library UB --- library/core/src/num/nonzero.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/num/nonzero.rs b/library/core/src/num/nonzero.rs index 012f224a4a265..6cbcac20ea1b4 100644 --- a/library/core/src/num/nonzero.rs +++ b/library/core/src/num/nonzero.rs @@ -368,7 +368,7 @@ where // SAFETY: The caller guarantees that `n` references a value that is non-zero, so this is unreachable. unsafe { intrinsics::assert_unsafe_precondition!( - check_language_ub, + check_library_ub, "NonZero::from_mut_unchecked requires the argument to dereference as non-zero", () => false, ); From 2a1f97f77ff2a6f6e27aae983420f4ee57056527 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 9 Mar 2024 13:36:36 -0500 Subject: [PATCH 5/5] Explain why we don't use intrinsics::is_nonoverlapping --- library/core/src/cell.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index e8590ab115860..0e719e169deb6 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -434,6 +434,8 @@ impl Cell { #[inline] #[stable(feature = "move_cell", since = "1.17.0")] pub fn swap(&self, other: &Self) { + // This function documents that it *will* panic, and intrinsics::is_nonoverlapping doesn't + // do the check in const, so trying to use it here would be inviting unnecessary fragility. fn is_nonoverlapping(src: *const T, dst: *const T) -> bool { let src_usize = src.addr(); let dst_usize = dst.addr();