Skip to content

Commit

Permalink
Put checks that detect UB under their own flag below debug_assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Apr 5, 2024
1 parent c7491b9 commit 6cf853c
Show file tree
Hide file tree
Showing 20 changed files with 92 additions and 15 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/check_alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/instsimplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,10 @@ fn default_configuration(sess: &Session) -> Cfg {
ins_none!(sym::overflow_checks);
}

if sess.ub_checks() {
ins_none!(sym::ub_checks);
}

ins_sym!(sym::panic, sess.panic_strategy().desc_symbol());

// JUSTIFICATION: before wrapper fn is available
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
15 changes: 15 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/ub-checks.md
Original file line number Diff line number Diff line change
@@ -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.
28 changes: 28 additions & 0 deletions tests/codegen/ub-checks.rs
Original file line number Diff line number Diff line change
@@ -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)
}
5 changes: 5 additions & 0 deletions tests/ui/feature-gates/feature-gate-cfg_ub_checks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#![crate_type = "lib"]

pub fn ub_checks_are_enabled() -> bool {
cfg!(ub_checks) //~ ERROR `cfg(ub_checks)` is experimental
}
13 changes: 13 additions & 0 deletions tests/ui/feature-gates/feature-gate-cfg_ub_checks.stderr
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 1 addition & 1 deletion tests/ui/precondition-checks/misaligned-slice.rs
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/precondition-checks/null-slice.rs
Original file line number Diff line number Diff line change
@@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down

0 comments on commit 6cf853c

Please sign in to comment.