Put "checks that detect UB" under their own flag below `debug_assertions` #725

RalfJung · 2024-02-26T07:20:09Z

Proposal

@saethlin has been working on adding more and more checks to rustc and the standard library that can detect widespread uses of UB. Currently, they are all controlled by -Cdebug-assertions/cfg(debug_assertions). I would argue that similar to the existing -Coverflow-checks/cfg(overflow_checks), it is worth having a knob that can separately control them. While they generally have fairly little overhead, in some cases they can hurt a lot. And vice versa, maybe someone wants to turn on just these checks but not all the checks for more "harmless" bugs.

So I suggest we add a new flag, -Cub-checks/cfg(ub_checks), that behaves like overflow_checks: it defaults to match debug_assertions but can be overwritten separately. Initially it would be unstable as -Zub-checks, and cfg(ub_checks) would not be stabilized before cfg(overflow_checks).

When this was discussed previously, a lot was talked about whether this should be grouped with the sanitizers. However, currently this is controlled entirely by -Cdebug-assertions, so just splitting up that flag in the established way seems to make most sense to me. In the end, sanitizers are not the only way we can detect UB -- we also have Miri, and now these checks. It would probably be confusing to people familiar with sanitizers if these kinds of debug assertions were mixed up with ubsan/msan and friends.

Mentors or Reviewers

Myself, and possibly @saethlin ?

Process

The main points of the Major Change Process are as follows:

File an issue describing the proposal.
A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
- Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

The text was updated successfully, but these errors were encountered:

rustbot · 2024-02-26T07:20:11Z

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

@rustbot concern reason-for-concern 
<description of the concern>

Concerns can be lifted with:

@rustbot resolve reason-for-concern

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler @rust-lang/compiler-contributors

Noratrieb · 2024-02-26T17:14:15Z

@rustbot second for adding it unstably with the plan to stabilize it

RalfJung · 2024-03-14T09:00:47Z

This has been seconded more than 10 days ago, does that mean it got accepted?

Noratrieb · 2024-03-14T09:09:00Z

the mark-as-accepted process is still manual, but seems ok

… r=<try> refactor check_{lang,library}_ub: use a single intrinsics This enacts the plan I laid out [here](rust-lang#122282 (comment)): use a single intrinsic, called `ub_checks` (in aniticpation of rust-lang/compiler-team#725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library. This makes it easier to do something like rust-lang#122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time. The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics. r? `@saethlin`

… r=<try> refactor check_{lang,library}_ub: use a single intrinsic This enacts the plan I laid out [here](rust-lang#122282 (comment)): use a single intrinsic, called `ub_checks` (in aniticpation of rust-lang/compiler-team#725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library. This makes it easier to do something like rust-lang#122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time. The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics. r? `@saethlin`

… r=saethlin refactor check_{lang,library}_ub: use a single intrinsic This enacts the plan I laid out [here](rust-lang#122282 (comment)): use a single intrinsic, called `ub_checks` (in aniticpation of rust-lang/compiler-team#725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library. This makes it easier to do something like rust-lang#122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time. The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics. r? `@saethlin`

refactor check_{lang,library}_ub: use a single intrinsic This enacts the plan I laid out [here](rust-lang/rust#122282 (comment)): use a single intrinsic, called `ub_checks` (in aniticpation of rust-lang/compiler-team#725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library. This makes it easier to do something like rust-lang/rust#122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time. The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics. r? `@saethlin`

… r=saethlin refactor check_{lang,library}_ub: use a single intrinsic This enacts the plan I laid out [here](rust-lang#122282 (comment)): use a single intrinsic, called `ub_checks` (in aniticpation of rust-lang/compiler-team#725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library. This makes it easier to do something like rust-lang#122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time. The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics. r? `@saethlin`

refactor check_{lang,library}_ub: use a single intrinsic This enacts the plan I laid out [here](rust-lang/rust#122282 (comment)): use a single intrinsic, called `ub_checks` (in aniticpation of rust-lang/compiler-team#725), that just exposes the value of `debug_assertions` (consistently implemented in both codegen and the interpreter). Put the language vs library UB logic into the library. This makes it easier to do something like rust-lang/rust#122282 in the future: that just slightly alters the semantics of `ub_checks` (making it more approximating when crates built with different flags are mixed), but it no longer affects whether these checks can happen in Miri or compile-time. The first commit just moves things around; I don't think these macros and functions belong into `intrinsics.rs` as they are not intrinsics. r? `@saethlin`

Put checks that detect UB under their own flag below debug_assertions Implementation of rust-lang/compiler-team#725

Rollup merge of rust-lang#123411 - saethlin:ub-checks, r=Urgau,RalfJung Put checks that detect UB under their own flag below debug_assertions Implementation of rust-lang/compiler-team#725

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Link: https://lore.kernel.org/r/20240701183625.665574-13-ojeda@kernel.org

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Tested-by: Benno Lossin <benno.lossin@proton.me>

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Link: https://lore.kernel.org/r/20240701183625.665574-13-ojeda@kernel.org

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Reviewed-by: Finn Behrens <me@kloenk.dev> Tested-by: Benno Lossin <benno.lossin@proton.me> Tested-by: Andreas Hindborg <a.hindborg@samsung.com> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Reviewed-by: Finn Behrens <me@kloenk.dev> Tested-by: Benno Lossin <benno.lossin@proton.me> Tested-by: Andreas Hindborg <a.hindborg@samsung.com> Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Link: https://lore.kernel.org/r/20240709160615.998336-13-ojeda@kernel.org

Rust 1.79.0 has introduced a new codegen flag, `-Zub-checks` [1], to allow to independently configure (from `-Cdebug-assertions`) whether the extra runtime checks for UB are emitted, in a similar fashion to `-Coverflow-checks`. This allows to configure the kernel with only the UB checks enabled, but not the `debug_assert!`s; or vice versa, e.g. [2]. It also showcases how `RUSTC_VERSION` and the Kbuild macros, introduced in the previous commit, can be used. Link: rust-lang/compiler-team#725 [1] Link: https://godbolt.org/z/jY69ezx5K [2] Reviewed-by: Finn Behrens <me@kloenk.dev> Tested-by: Benno Lossin <benno.lossin@proton.me> Tested-by: Andreas Hindborg <a.hindborg@samsung.com> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

RalfJung added T-compiler Add this label so rfcbot knows to poll the compiler team major-change A proposal to make a major change to rustc labels Feb 26, 2024

rustbot added the to-announce Announce this issue on triage meeting label Feb 26, 2024

RalfJung mentioned this issue Feb 26, 2024

Add a stable flag to enable/disable codegen UB checks #625

Closed

3 tasks

rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Feb 26, 2024

apiraino removed the to-announce Announce this issue on triage meeting label Feb 29, 2024

RalfJung mentioned this issue Mar 14, 2024

Eliminate UbCheck for non-standard libraries rust-lang/rust#122282

Closed

Noratrieb added major-change-accepted A major change proposal that was accepted and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Mar 14, 2024

Noratrieb closed this as completed Mar 14, 2024

rustbot added the to-announce Announce this issue on triage meeting label Mar 14, 2024

apiraino removed the to-announce Announce this issue on triage meeting label Mar 14, 2024

RalfJung mentioned this issue Mar 17, 2024

refactor check_{lang,library}_ub: use a single intrinsic rust-lang/rust#122629

Merged

RalfJung mentioned this issue Mar 18, 2024

Put "checks that detect UB" under their own flag below debug_assertions rust-lang/rust#122678

Closed

saethlin mentioned this issue Apr 3, 2024

Put checks that detect UB under their own flag below debug_assertions rust-lang/rust#123411

Merged

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 6, 2024

Rollup merge of rust-lang#123411 - saethlin:ub-checks, r=Urgau,RalfJung

7a79a83

Put checks that detect UB under their own flag below debug_assertions Implementation of rust-lang/compiler-team#725

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 6, 2024

Rollup merge of rust-lang#123411 - saethlin:ub-checks, r=Urgau,RalfJung

84dca15

Put checks that detect UB under their own flag below debug_assertions Implementation of rust-lang/compiler-team#725

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Put "checks that detect UB" under their own flag below `debug_assertions` #725

Put "checks that detect UB" under their own flag below `debug_assertions` #725

RalfJung commented Feb 26, 2024 •

edited

Loading

rustbot commented Feb 26, 2024

Noratrieb commented Feb 26, 2024

RalfJung commented Mar 14, 2024

Noratrieb commented Mar 14, 2024

Put "checks that detect UB" under their own flag below debug_assertions #725

Put "checks that detect UB" under their own flag below debug_assertions #725

Comments

RalfJung commented Feb 26, 2024 • edited Loading

Proposal

Mentors or Reviewers

Process

Comments

rustbot commented Feb 26, 2024

Noratrieb commented Feb 26, 2024

RalfJung commented Mar 14, 2024

Noratrieb commented Mar 14, 2024

Put "checks that detect UB" under their own flag below `debug_assertions` #725

Put "checks that detect UB" under their own flag below `debug_assertions` #725

RalfJung commented Feb 26, 2024 •

edited

Loading