Put checks that detect UB under their own flag below debug_assertions

saethlin · saethlin · commit 6edde243dcd8 · 2024-04-05T14:25:10.000-04:00
diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs
@@ -789,7 +789,7 @@ fn codegen_stmt<'tcx>(
                             layout.offset_of_subfield(fx, fields.iter()).bytes()
                         }
                         NullOp::UbChecks => {
-                            let val = fx.tcx.sess.opts.debug_assertions;
+                            let val = fx.tcx.sess.ub_checks();
                             let val = CValue::by_val(
                                 fx.bcx.ins().iconst(types::I8, i64::try_from(val).unwrap()),
                                 fx.layout_of(fx.tcx.types.bool),
diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs
@@ -682,7 +682,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         bx.cx().const_usize(val)
                     }
                     mir::NullOp::UbChecks => {
-                        let val = bx.tcx().sess.opts.debug_assertions;
+                        let val = bx.tcx().sess.ub_checks();
                         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,7 +258,7 @@ 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::UbChecks => Scalar::from_bool(self.tcx.sess.opts.debug_assertions),
+                    mir::NullOp::UbChecks => Scalar::from_bool(self.tcx.sess.ub_checks()),
                 };
                 self.write_scalar(val, &dest)?;
             }
diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs
@@ -25,6 +25,7 @@ pub type GatedCfg = (Symbol, Symbol, GateFn);
 const GATED_CFGS: &[GatedCfg] = &[
     // (name in cfg, feature, function to check if the feature is enabled)
     (sym::overflow_checks, sym::cfg_overflow_checks, cfg_fn!(cfg_overflow_checks)),
+    (sym::ub_checks, sym::cfg_ub_checks, cfg_fn!(cfg_ub_checks)),
     (sym::target_thread_local, sym::cfg_target_thread_local, cfg_fn!(cfg_target_thread_local)),
     (
         sym::target_has_atomic_equal_alignment,
diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs
@@ -381,6 +381,8 @@ declare_features! (
     (unstable, cfg_target_has_atomic_equal_alignment, "1.60.0", Some(93822)),
     /// Allows `cfg(target_thread_local)`.
     (unstable, cfg_target_thread_local, "1.7.0", Some(29594)),
+    /// Allows the use of `#[cfg(ub_checks)` to check if UB checks are enabled.
+    (unstable, cfg_ub_checks, "CURRENT_RUSTC_VERSION", Some(123499)),
     /// Allow conditional compilation depending on rust version
     (unstable, cfg_version, "1.45.0", Some(64796)),
     /// Allows to use the `#[cfi_encoding = ""]` attribute.
diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs
@@ -847,6 +847,7 @@ fn test_unstable_options_tracking_hash() {
     tracked!(trap_unreachable, Some(false));
     tracked!(treat_err_as_bug, NonZero::new(1));
     tracked!(tune_cpu, Some(String::from("abc")));
+    tracked!(ub_checks, Some(false));
     tracked!(uninit_const_chunk_threshold, 123);
     tracked!(unleash_the_miri_inside_of_you, true);
     tracked!(use_ctors_section, Some(true));
diff --git a/compiler/rustc_mir_transform/src/check_alignment.rs b/compiler/rustc_mir_transform/src/check_alignment.rs
@@ -16,7 +16,7 @@ impl<'tcx> MirPass<'tcx> for CheckAlignment {
         if sess.target.llvm_target == "i686-pc-windows-msvc" {
             return false;
         }
-        sess.opts.debug_assertions
+        sess.ub_checks()
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs
@@ -149,7 +149,7 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
 
     fn simplify_ub_check(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) {
         if let Rvalue::NullaryOp(NullOp::UbChecks, _) = *rvalue {
-            let const_ = Const::from_bool(self.tcx, self.tcx.sess.opts.debug_assertions);
+            let const_ = Const::from_bool(self.tcx, self.tcx.sess.ub_checks());
             let constant = ConstOperand { span: source_info.span, const_, user_ty: None };
             *rvalue = Rvalue::Use(Operand::Constant(Box::new(constant)));
         }
diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs
@@ -1329,6 +1329,10 @@ fn default_configuration(sess: &Session) -> Cfg {
         ins_none!(sym::test);
     }
 
+    if sess.ub_checks() {
+        ins_none!(sym::ub_checks);
+    }
+
     ret
 }
 
@@ -1532,6 +1536,8 @@ impl CheckCfg {
 
         ins!(sym::test, no_values);
 
+        ins!(sym::ub_checks, no_values);
+
         ins!(sym::unix, no_values);
         ins!(sym::windows, no_values);
     }
diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs
@@ -1994,6 +1994,9 @@ written to standard error output)"),
         "in diagnostics, use heuristics to shorten paths referring to items"),
     tune_cpu: Option<String> = (None, parse_opt_string, [TRACKED],
         "select processor to schedule for (`rustc --print target-cpus` for details)"),
+    #[rustc_lint_opt_deny_field_access("use `Session::ub_checks` instead of this field")]
+    ub_checks: Option<bool> = (None, parse_opt_bool, [TRACKED],
+        "emit runtime checks for Undefined Behavior (default: -Cdebug-assertions)"),
     ui_testing: bool = (false, parse_bool, [UNTRACKED],
         "emit compiler diagnostics in a form suitable for UI testing (default: no)"),
     uninit_const_chunk_threshold: usize = (16, parse_number, [TRACKED],
diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs
@@ -735,6 +735,10 @@ impl Session {
         self.opts.cg.overflow_checks.unwrap_or(self.opts.debug_assertions)
     }
 
+    pub fn ub_checks(&self) -> bool {
+        self.opts.unstable_opts.ub_checks.unwrap_or(self.opts.debug_assertions)
+    }
+
     pub fn relocation_model(&self) -> RelocModel {
         self.opts.cg.relocation_model.unwrap_or(self.target.relocation_model)
     }
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -514,6 +514,7 @@ symbols! {
         cfg_target_has_atomic_equal_alignment,
         cfg_target_thread_local,
         cfg_target_vendor,
+        cfg_ub_checks,
         cfg_version,
         cfi,
         cfi_encoding,
diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs
@@ -2704,17 +2704,17 @@ pub const unsafe fn typed_swap<T>(x: *mut T, y: *mut T) {
 }
 
 /// Returns whether we should perform some UB-checking at runtime. This eventually evaluates to
-/// `cfg!(debug_assertions)`, but behaves different from `cfg!` when mixing crates built with different
-/// flags: if the crate has debug assertions enabled or carries the `#[rustc_preserve_ub_checks]`
+/// `cfg!(ub_checks)`, but behaves different from `cfg!` when mixing crates built with different
+/// flags: if the crate has UB checks enabled or carries the `#[rustc_preserve_ub_checks]`
 /// attribute, evaluation is delayed until monomorphization (or until the call gets inlined into
 /// a crate that does not delay evaluation further); otherwise it can happen any time.
 ///
-/// The common case here is a user program built with debug_assertions linked against the distributed
-/// sysroot which is built without debug_assertions but with `#[rustc_preserve_ub_checks]`.
+/// The common case here is a user program built with ub_checks linked against the distributed
+/// sysroot which is built without ub_checks but with `#[rustc_preserve_ub_checks]`.
 /// For code that gets monomorphized in the user crate (i.e., generic functions and functions with
-/// `#[inline]`), gating assertions on `ub_checks()` rather than `cfg!(debug_assertions)` means that
-/// assertions are enabled whenever the *user crate* has debug assertions enabled. However if the
-/// user has debug assertions disabled, the checks will still get optimized out. This intrinsic is
+/// `#[inline]`), gating assertions on `ub_checks()` rather than `cfg!(ub_checks)` means that
+/// assertions are enabled whenever the *user crate* has UB checks enabled. However if the
+/// user has UB checks disabled, the checks will still get optimized out. This intrinsic is
 /// primarily used by [`ub_checks::assert_unsafe_precondition`].
 #[rustc_const_unstable(feature = "const_ub_checks", issue = "none")]
 #[unstable(feature = "core_intrinsics", issue = "none")]
diff --git a/src/doc/unstable-book/src/compiler-flags/ub-checks.md b/src/doc/unstable-book/src/compiler-flags/ub-checks.md
@@ -0,0 +1,15 @@
+# `ub-checks`
+
+--------------------
+
+The `-Zub-checks` compiler flag enables additional runtime checks that detect some causes of Undefined Behavior at runtime.
+By default, `-Zub-checks` flag inherits the value of `-Cdebug-assertions`.
+
+All checks are generated on a best-effort basis; even if we have a check implemented for some cause of Undefined Behavior, it may be possible for the check to not fire.
+If a dependency is compiled with `-Zub-checks=no` but the final binary or library is compiled with `-Zub-checks=yes`, UB checks reached by the dependency are likely to be optimized out.
+
+When `-Zub-checks` detects UB, a non-unwinding panic is produced.
+That means that we will not unwind the stack and will not call any `Drop` impls, but we will execute the configured panic hook.
+We expect that unsafe code has been written which relies on code not unwinding which may have UB checks inserted.
+Ergo, an unwinding panic could easily turn works-as-intended UB into a much bigger problem.
+Calling the panic hook theoretically has the same implications, but we expect that the standard library panic hook will be stateless enough to be always called, and that if a user has configured a panic hook that the hook may be very helpful to debugging the detected UB.
diff --git a/tests/codegen/ub-checks.rs b/tests/codegen/ub-checks.rs
@@ -0,0 +1,28 @@
+// With -Zub-checks=yes (enabled by default by -Cdebug-assertions=yes) we will produce a runtime
+// check that the index to slice::get_unchecked is in-bounds of the slice. That is tested for by
+// tests/ui/precondition-checks/out-of-bounds-get-unchecked.rs
+//
+// This test ensures that such a runtime check is *not* emitted when debug-assertions are enabled,
+// but ub-checks are explicitly disabled.
+
+//@ revisions: DEBUG NOCHECKS
+//@ [DEBUG] compile-flags:
+//@ [NOCHECKS] compile-flags: -Zub-checks=no
+//@ compile-flags: -O -Cdebug-assertions=yes
+
+#![crate_type = "lib"]
+
+use std::ops::Range;
+
+// CHECK-LABEL: @slice_get_unchecked(
+#[no_mangle]
+pub unsafe fn slice_get_unchecked(x: &[i32], i: usize) -> &i32 {
+    //    CHECK: icmp ult
+    // NOCHECKS: tail call void @llvm.assume
+    //    DEBUG: br i1
+    //    DEBUG: call core::panicking::panic_nounwind
+    //    DEBUG: unreachable
+    //    CHECK: getelementptr inbounds
+    //    CHECK: ret ptr
+    x.get_unchecked(i)
+}
diff --git a/tests/ui/feature-gates/feature-gate-cfg_ub_checks.rs b/tests/ui/feature-gates/feature-gate-cfg_ub_checks.rs
@@ -0,0 +1,5 @@
+#![crate_type = "lib"]
+
+pub fn ub_checks_are_enabled() -> bool {
+    cfg!(ub_checks) //~ ERROR `cfg(ub_checks)` is experimental
+}
diff --git a/tests/ui/feature-gates/feature-gate-cfg_ub_checks.stderr b/tests/ui/feature-gates/feature-gate-cfg_ub_checks.stderr
@@ -0,0 +1,13 @@
+error[E0658]: `cfg(ub_checks)` is experimental and subject to change
+  --> $DIR/feature-gate-cfg_ub_checks.rs:4:10
+   |
+LL |     cfg!(ub_checks)
+   |          ^^^^^^^^^
+   |
+   = note: see issue #123499 <https://github.com/rust-lang/rust/issues/123499> for more information
+   = help: add `#![feature(cfg_ub_checks)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0658`.
diff --git a/tests/ui/precondition-checks/cfg-ub-checks-default.rs b/tests/ui/precondition-checks/cfg-ub-checks-default.rs
@@ -0,0 +1,10 @@
+//@ run-pass
+//@ revisions YES NO
+//@ [YES] compile-flags: -Cdebug-assertions=yes
+//@ [NO] compile-flags: -Cdebug-assertions=no
+
+#![feature(cfg_ub_checks)]
+
+fn main() {
+    assert_eq!(cfg!(ub_checks), cfg!(debug_assertions));
+}
diff --git a/tests/ui/precondition-checks/cfg-ub-checks-no.rs b/tests/ui/precondition-checks/cfg-ub-checks-no.rs
@@ -0,0 +1,19 @@
+//@ run-pass
+//@ compile-flags: -Cdebug-assertions=yes -Zub-checks=no
+
+#![feature(cfg_ub_checks)]
+
+fn main() {
+    assert!(!cfg!(ub_checks));
+    assert!(compiles_differently());
+}
+
+#[cfg(ub_checks)]
+fn compiles_differently() -> bool {
+    false
+}
+
+#[cfg(not(ub_checks))]
+fn compiles_differently() -> bool {
+    true
+}
diff --git a/tests/ui/precondition-checks/cfg-ub-checks-yes.rs b/tests/ui/precondition-checks/cfg-ub-checks-yes.rs
@@ -0,0 +1,19 @@
+//@ run-pass
+//@ compile-flags: -Cdebug-assertions=no -Zub-checks=yes
+
+#![feature(cfg_ub_checks)]
+
+fn main() {
+    assert!(cfg!(ub_checks));
+    assert!(compiles_differently());
+}
+
+#[cfg(ub_checks)]
+fn compiles_differently() -> bool {
+    true
+}
+
+#[cfg(not(ub_checks))]
+fn compiles_differently() -> bool {
+    false
+}
diff --git a/tests/ui/precondition-checks/misaligned-slice.rs b/tests/ui/precondition-checks/misaligned-slice.rs
@@ -1,5 +1,5 @@
 //@ run-fail
-//@ compile-flags: -Copt-level=3 -Cdebug-assertions=yes
+//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
 //@ error-pattern: unsafe precondition(s) violated: slice::from_raw_parts
 //@ ignore-debug
 
diff --git a/tests/ui/precondition-checks/null-slice.rs b/tests/ui/precondition-checks/null-slice.rs
@@ -1,5 +1,5 @@
 //@ run-fail
-//@ compile-flags: -Copt-level=3 -Cdebug-assertions=yes
+//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
 //@ error-pattern: unsafe precondition(s) violated: slice::from_raw_parts
 //@ ignore-debug
 
diff --git a/tests/ui/precondition-checks/out-of-bounds-get-unchecked.rs b/tests/ui/precondition-checks/out-of-bounds-get-unchecked.rs
@@ -1,5 +1,5 @@
 //@ run-fail
-//@ compile-flags: -Copt-level=3 -Cdebug-assertions=yes
+//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
 //@ error-pattern: slice::get_unchecked requires
 //@ ignore-debug
 

Original file line number	Diff line number	Diff line change
`@@ -789,7 +789,7 @@ fn codegen_stmt<'tcx>(`
`789`	`789`	`layout.offset_of_subfield(fx, fields.iter()).bytes()`
`790`	`790`	`}`
`791`	`791`	`NullOp::UbChecks => {`
`792`		`- let val = fx.tcx.sess.opts.debug_assertions;`
	`792`	`+ let val = fx.tcx.sess.ub_checks();`
`793`	`793`	`let val = CValue::by_val(`
`794`	`794`	`fx.bcx.ins().iconst(types::I8, i64::try_from(val).unwrap()),`
`795`	`795`	`fx.layout_of(fx.tcx.types.bool),`
Original file line number	Diff line number	Diff line change
`@@ -682,7 +682,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {`
`682`	`682`	`bx.cx().const_usize(val)`
`683`	`683`	`}`
`684`	`684`	`mir::NullOp::UbChecks => {`
`685`		`- let val = bx.tcx().sess.opts.debug_assertions;`
	`685`	`+ let val = bx.tcx().sess.ub_checks();`
`686`	`686`	`bx.cx().const_bool(val)`
`687`	`687`	`}`
`688`	`688`	`};`
Original file line number	Diff line number	Diff line change
`@@ -258,7 +258,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {`
`258`	`258`	`let val = layout.offset_of_subfield(self, fields.iter()).bytes();`
`259`	`259`	`Scalar::from_target_usize(val, self)`
`260`	`260`	`}`
`261`		`- mir::NullOp::UbChecks => Scalar::from_bool(self.tcx.sess.opts.debug_assertions),`
	`261`	`+ mir::NullOp::UbChecks => Scalar::from_bool(self.tcx.sess.ub_checks()),`
`262`	`262`	`};`
`263`	`263`	`self.write_scalar(val, &dest)?;`
`264`	`264`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,7 @@ pub type GatedCfg = (Symbol, Symbol, GateFn);`
`25`	`25`	`const GATED_CFGS: &[GatedCfg] = &[`
`26`	`26`	`// (name in cfg, feature, function to check if the feature is enabled)`
`27`	`27`	`(sym::overflow_checks, sym::cfg_overflow_checks, cfg_fn!(cfg_overflow_checks)),`
	`28`	`+ (sym::ub_checks, sym::cfg_ub_checks, cfg_fn!(cfg_ub_checks)),`
`28`	`29`	`(sym::target_thread_local, sym::cfg_target_thread_local, cfg_fn!(cfg_target_thread_local)),`
`29`	`30`	`(`
`30`	`31`	`sym::target_has_atomic_equal_alignment,`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ impl<'tcx> MirPass<'tcx> for CheckAlignment {`
`16`	`16`	`if sess.target.llvm_target == "i686-pc-windows-msvc" {`
`17`	`17`	`return false;`
`18`	`18`	`}`
`19`		`- sess.opts.debug_assertions`
	`19`	`+ sess.ub_checks()`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {`
Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,7 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {`
`149`	`149`
`150`	`150`	`fn simplify_ub_check(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) {`
`151`	`151`	`if let Rvalue::NullaryOp(NullOp::UbChecks, _) = *rvalue {`
`152`		`- let const_ = Const::from_bool(self.tcx, self.tcx.sess.opts.debug_assertions);`
	`152`	`+ let const_ = Const::from_bool(self.tcx, self.tcx.sess.ub_checks());`
`153`	`153`	`let constant = ConstOperand { span: source_info.span, const_, user_ty: None };`
`154`	`154`	`*rvalue = Rvalue::Use(Operand::Constant(Box::new(constant)));`
`155`	`155`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1329,6 +1329,10 @@ fn default_configuration(sess: &Session) -> Cfg {`
`1329`	`1329`	`ins_none!(sym::test);`
`1330`	`1330`	`}`
`1331`	`1331`
	`1332`	`+ if sess.ub_checks() {`
	`1333`	`+ ins_none!(sym::ub_checks);`
	`1334`	`+ }`
	`1335`	`+`
`1332`	`1336`	`ret`
`1333`	`1337`	`}`
`1334`	`1338`
`@@ -1532,6 +1536,8 @@ impl CheckCfg {`
`1532`	`1536`
`1533`	`1537`	`ins!(sym::test, no_values);`
`1534`	`1538`
	`1539`	`+ ins!(sym::ub_checks, no_values);`
	`1540`	`+`
`1535`	`1541`	`ins!(sym::unix, no_values);`
`1536`	`1542`	`ins!(sym::windows, no_values);`
`1537`	`1543`	`}`