Skip to content

Commit

Permalink
Rollup merge of #123411 - saethlin:ub-checks, r=Urgau,RalfJung
Browse files Browse the repository at this point in the history
Put checks that detect UB under their own flag below debug_assertions

Implementation of rust-lang/compiler-team#725
  • Loading branch information
matthiaskrgr authored Apr 6, 2024
2 parents 0ea4270 + a7912cb commit 84dca15
Show file tree
Hide file tree
Showing 42 changed files with 206 additions and 64 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 @@ -846,6 +846,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
8 changes: 3 additions & 5 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,8 @@ impl<'tcx> Body<'tcx> {
// _1 = const _
// SwitchInt(_1)
//
// And MIR for if intrinsics::debug_assertions() looks like this:
// _1 = cfg!(debug_assertions)
// And MIR for if intrinsics::ub_checks() looks like this:
// _1 = UbChecks()
// SwitchInt(_1)
//
// So we're going to try to recognize this pattern.
Expand All @@ -799,9 +799,7 @@ impl<'tcx> Body<'tcx> {
}

match rvalue {
Rvalue::NullaryOp(NullOp::UbChecks, _) => {
Some((tcx.sess.opts.debug_assertions as u128, targets))
}
Rvalue::NullaryOp(NullOp::UbChecks, _) => Some((tcx.sess.ub_checks() as u128, targets)),
Rvalue::Use(Operand::Constant(constant)) => {
let bits = eval_mono_const(constant);
Some((bits, targets))
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
6 changes: 6 additions & 0 deletions compiler/rustc_session/src/config/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ pub(crate) fn default_configuration(sess: &Session) -> Cfg {
ins_none!(sym::test);
}

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

ret
}

Expand Down Expand Up @@ -367,6 +371,8 @@ impl CheckCfg {

ins!(sym::test, no_values);

ins!(sym::ub_checks, no_values);

ins!(sym::unix, no_values);
ins!(sym::windows, no_values);
}
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 @@ -1992,6 +1992,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
2 changes: 1 addition & 1 deletion compiler/stable_mir/src/mir/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ pub enum NullOp {
AlignOf,
/// Returns the offset of a field.
OffsetOf(Vec<(VariantIdx, FieldIdx)>),
/// cfg!(debug_assertions), but at codegen time
/// cfg!(ub_checks), but at codegen time
UbChecks,
}

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
3 changes: 2 additions & 1 deletion src/doc/unstable-book/src/compiler-flags/check-cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Those well known names and values follows the same stability as what they refer
Well known names and values checking is always enabled as long as at least one
`--check-cfg` argument is present.
As of `2024-02-15T`, the list of known names is as follows:
As of `2024-04-06T`, the list of known names is as follows:
<!--- See CheckCfg::fill_well_known in compiler/rustc_session/src/config.rs -->
Expand Down Expand Up @@ -107,6 +107,7 @@ As of `2024-02-15T`, the list of known names is as follows:
- `target_thread_local`
- `target_vendor`
- `test`
- `ub_checks`
- `unix`
- `windows`
Expand Down
17 changes: 17 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,17 @@
# `ub-checks`

The tracking issue for this feature is: [#123499](https://github.com/rust-lang/rust/issues/123499).

--------------------

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)
}
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/allow-same-level.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ warning: unexpected `cfg` condition name: `FALSE`
LL | #[cfg(FALSE)]
| ^^^^^
|
= help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
= help: to expect this configuration use `--check-cfg=cfg(FALSE)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/cargo-feature.none.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ warning: unexpected `cfg` condition name: `tokio_unstable`
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
|
= help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= help: expected names are: `clippy`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(tokio_unstable)");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/cargo-feature.some.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ warning: unexpected `cfg` condition name: `tokio_unstable`
LL | #[cfg(tokio_unstable)]
| ^^^^^^^^^^^^^^
|
= help: expected names are: `CONFIG_NVME`, `clippy`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= help: expected names are: `CONFIG_NVME`, `clippy`, `debug_assertions`, `doc`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
= help: consider using a Cargo feature instead or adding `println!("cargo:rustc-check-cfg=cfg(tokio_unstable)");` to the top of a `build.rs`
= note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/cfg-value-for-cfg-name-duplicate.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ warning: unexpected `cfg` condition name: `value`
LL | #[cfg(value)]
| ^^^^^
|
= help: expected names are: `bar`, `bee`, `clippy`, `cow`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= help: expected names are: `bar`, `bee`, `clippy`, `cow`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
= help: to expect this configuration use `--check-cfg=cfg(value)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/cfg-value-for-cfg-name-multiple.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ warning: unexpected `cfg` condition name: `my_value`
LL | #[cfg(my_value)]
| ^^^^^^^^
|
= help: expected names are: `bar`, `clippy`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `unix`, `windows`
= help: expected names are: `bar`, `clippy`, `debug_assertions`, `doc`, `doctest`, `foo`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
= help: to expect this configuration use `--check-cfg=cfg(my_value)`
= note: see <https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
Expand Down
Loading

0 comments on commit 84dca15

Please sign in to comment.