Auto merge of #121662 - saethlin:precondition-unification, r=<try>

Distinguish between library and lang UB in assert_unsafe_precondition As described in #121583 (comment), `assert_unsafe_precondition` now explicitly distinguishes between language UB (conditions we explicitly optimize on) and library UB (things we document you shouldn't do, and maybe some library internals assume you don't do). `debug_assert_nounwind` was originally added to avoid the "only at runtime" aspect of `assert_unsafe_precondition`. Since then the difference between the macros has gotten muddied. This totally revamps the situation. Now _all_ preconditions shall be checked with `assert_unsafe_precondition`. If you have a precondition that's only checkable at runtime, do a `const_eval_select` hack, as done in this PR. r? RalfJung
rust-lang · Feb 27, 2024 · c492ab1 · c492ab1
2 parents 71ffdf7 + b268228
commit c492ab1
Show file tree

Hide file tree

Showing 46 changed files with 364 additions and 238 deletions.
diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs
@@ -2001,7 +2001,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
@@ -767,7 +767,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
@@ -684,7 +684,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
@@ -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
@@ -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
@@ -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
@@ -123,7 +123,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
@@ -508,7 +509,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
@@ -909,7 +909,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
@@ -1361,8 +1361,13 @@ pub enum NullOp<'tcx> {
     AlignOf,
     /// Returns the offset of a field
     OffsetOf(&'tcx List<(VariantIdx, FieldIdx)>),
-    /// cfg!(debug_assertions), but expanded in codegen
-    DebugAssertions,
+    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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -515,6 +515,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
@@ -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/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
@@ -101,7 +101,11 @@ pub const unsafe fn unreachable_unchecked() -> ! {
     // 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::assert_unsafe_precondition!(
+            check_language_ub,
+            "hint::unreachable_unchecked must never be reached",
+            () => false
+        );
         intrinsics::unreachable()
     }
 }
@@ -147,6 +151,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,
         );