-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Put checks that detect UB under their own flag below debug_assertions #123411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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> { | |||||
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()), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this code in rust/compiler/rustc_middle/src/mir/mod.rs Lines 802 to 803 in 9d79cd5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤦 YES |
||||||
}; | ||||||
self.write_scalar(val, &dest)?; | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: rust/compiler/stable_mir/src/mir/body.rs Lines 996 to 997 in 9d79cd5
- /// cfg!(debug_assertions), but at codegen time
+ /// cfg!(ub_checks), but at codegen time
UbChecks, |
||||||
/// 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")] | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 --> | ||
|
||
|
@@ -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` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, but can you also update the date above - 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: |
||
- `unix` | ||
- `windows` | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,17 @@ | ||||||||||
# `ub-checks` | ||||||||||
|
||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
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. |
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) | ||
} |
Uh oh!
There was an error while loading. Please reload this page.