From 39f2b11d545ebdf37ac857c52e46fd2b5a0abeb8 Mon Sep 17 00:00:00 2001 From: DianQK Date: Sun, 10 Mar 2024 16:29:39 +0800 Subject: [PATCH] Eliminate `UbCheck` for non-standard libraries --- compiler/rustc_feature/src/builtin_attrs.rs | 4 ++ compiler/rustc_middle/src/mir/syntax.rs | 8 ++- .../rustc_mir_transform/src/instsimplify.rs | 17 ++++++ compiler/rustc_span/src/symbol.rs | 1 + library/alloc/src/lib.rs | 1 + library/core/src/intrinsics.rs | 14 +++-- library/core/src/lib.rs | 1 + library/std/src/lib.rs | 1 + ...unchecked.PreCodegen.after.panic-abort.mir | 13 +--- ...nchecked.PreCodegen.after.panic-unwind.mir | 13 +--- tests/mir-opt/instsimplify/ub_check.rs | 16 +++++ ...b_check.unwrap_unchecked.InstSimplify.diff | 59 +++++++++++++++++++ ...witch_targets.ub_if_b.PreCodegen.after.mir | 11 +--- 13 files changed, 119 insertions(+), 40 deletions(-) create mode 100644 tests/mir-opt/instsimplify/ub_check.rs create mode 100644 tests/mir-opt/instsimplify/ub_check.unwrap_unchecked.InstSimplify.diff diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 22cf50fce7f49..6a8a1722bcb23 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -821,6 +821,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_allow_incoherent_impl, AttributeType::Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::No, "#[rustc_allow_incoherent_impl] has to be added to all impl items of an incoherent inherent impl." ), + rustc_attr!( + rustc_preserve_ub_checks, AttributeType::CrateLevel, template!(Word), ErrorFollowing, EncodeCrossCrate::No, + "`#![rustc_preserve_ub_checks]` prevents the designated crate from evaluating whether UB checks are enabled when optimizing MIR", + ), rustc_attr!( rustc_deny_explicit_impl, AttributeType::Normal, diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 36b7a48b2a2b8..d75171520dc53 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -1367,8 +1367,12 @@ pub enum NullOp<'tcx> { AlignOf, /// Returns the offset of a field OffsetOf(&'tcx List<(VariantIdx, FieldIdx)>), - /// Returns whether we want to check for UB. - /// This returns the value of `cfg!(debug_assertions)` at monomorphization time. + /// 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]` + /// 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. + /// Refer to the comments in the `ub_checks` function. UbChecks, } diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs index 6b33d81c1c412..edb5de05cc4e0 100644 --- a/compiler/rustc_mir_transform/src/instsimplify.rs +++ b/compiler/rustc_mir_transform/src/instsimplify.rs @@ -1,10 +1,12 @@ //! Performs various peephole optimizations. use crate::simplify::simplify_duplicate_switch_targets; +use rustc_ast::attr; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::layout::ValidityRequirement; use rustc_middle::ty::{self, GenericArgsRef, ParamEnv, Ty, TyCtxt}; +use rustc_span::sym; use rustc_span::symbol::Symbol; use rustc_target::abi::FieldIdx; use rustc_target::spec::abi::Abi; @@ -22,10 +24,17 @@ impl<'tcx> MirPass<'tcx> for InstSimplify { local_decls: &body.local_decls, param_env: tcx.param_env_reveal_all_normalized(body.source.def_id()), }; + // FIXME(#116171) Coverage related, also see `unreachable_prop.rs`. + let preserve_ub_checks = + attr::contains_name(tcx.hir().krate_attrs(), sym::rustc_preserve_ub_checks) + || tcx.sess.instrument_coverage(); for block in body.basic_blocks.as_mut() { for statement in block.statements.iter_mut() { match statement.kind { StatementKind::Assign(box (_place, ref mut rvalue)) => { + if !preserve_ub_checks { + ctx.simplify_ub_check(&statement.source_info, rvalue); + } ctx.simplify_bool_cmp(&statement.source_info, rvalue); ctx.simplify_ref_deref(&statement.source_info, rvalue); ctx.simplify_len(&statement.source_info, rvalue); @@ -140,6 +149,14 @@ 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 constant = ConstOperand { span: source_info.span, const_, user_ty: None }; + *rvalue = Rvalue::Use(Operand::Constant(Box::new(constant))); + } + } + fn simplify_cast(&self, rvalue: &mut Rvalue<'tcx>) { if let Rvalue::Cast(kind, operand, cast_ty) = rvalue { let operand_ty = operand.ty(self.local_decls, self.tcx); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 73fcd2a76dfc0..72b720feefd03 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1571,6 +1571,7 @@ symbols! { rustc_peek_maybe_init, rustc_peek_maybe_uninit, rustc_polymorphize_error, + rustc_preserve_ub_checks, rustc_private, rustc_proc_macro_decls, rustc_promotable, diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 02d155aaf12f8..afcd7aea38b14 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -174,6 +174,7 @@ // Language features: // tidy-alphabetical-start #![cfg_attr(bootstrap, feature(associated_type_bounds))] +#![cfg_attr(not(bootstrap), rustc_preserve_ub_checks)] #![cfg_attr(not(test), feature(coroutine_trait))] #![cfg_attr(test, feature(panic_update_hook))] #![cfg_attr(test, feature(test))] diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 76e387d54d8a7..908eeb177923a 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2661,12 +2661,14 @@ pub const unsafe fn typed_swap(x: *mut T, y: *mut T) { unsafe { ptr::swap_nonoverlapping(x, y, 1) }; } -/// Returns whether we should perform some UB-checking at runtime. This evaluate to the value of -/// `cfg!(debug_assertions)` during monomorphization. -/// -/// This intrinsic is evaluated after monomorphization, which is relevant when mixing crates -/// compiled with and without debug_assertions. The common case here is a user program built with -/// debug_assertions linked against the distributed sysroot which is built without debug_assertions. +/// 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]` +/// 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]`. /// 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 diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index f0448a98981fb..c8fa12ccb1958 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -94,6 +94,7 @@ ))] #![no_core] #![rustc_coherence_is_core] +#![cfg_attr(not(bootstrap), rustc_preserve_ub_checks)] // // Lints: #![deny(rust_2021_incompatible_or_patterns)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index c457c39e0c11c..56bd21ec67511 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -221,6 +221,7 @@ // #![cfg_attr(not(feature = "restricted-std"), stable(feature = "rust1", since = "1.0.0"))] #![cfg_attr(feature = "restricted-std", unstable(feature = "restricted_std", issue = "none"))] +#![cfg_attr(not(bootstrap), rustc_preserve_ub_checks)] #![doc( html_playground_url = "https://play.rust-lang.org/", issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/", diff --git a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir index 9cd7053871e56..d629336d3859d 100644 --- a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-abort.mir @@ -11,8 +11,6 @@ fn unwrap_unchecked(_1: Option) -> T { } scope 3 { scope 4 (inlined unreachable_unchecked) { - let mut _3: bool; - let _4: (); scope 5 { } scope 6 (inlined core::ub_checks::check_language_ub) { @@ -26,23 +24,16 @@ fn unwrap_unchecked(_1: Option) -> T { bb0: { StorageLive(_2); _2 = discriminant(_1); - switchInt(move _2) -> [0: bb1, 1: bb2, otherwise: bb3]; + switchInt(move _2) -> [0: bb2, 1: bb1, otherwise: bb2]; } bb1: { - StorageLive(_3); - _3 = UbChecks(); - assume(_3); - _4 = unreachable_unchecked::precondition_check() -> [return: bb3, unwind unreachable]; - } - - bb2: { _0 = ((_1 as Some).0: T); StorageDead(_2); return; } - bb3: { + bb2: { unreachable; } } diff --git a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir index 9cd7053871e56..d629336d3859d 100644 --- a/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir +++ b/tests/mir-opt/inline/unwrap_unchecked.unwrap_unchecked.PreCodegen.after.panic-unwind.mir @@ -11,8 +11,6 @@ fn unwrap_unchecked(_1: Option) -> T { } scope 3 { scope 4 (inlined unreachable_unchecked) { - let mut _3: bool; - let _4: (); scope 5 { } scope 6 (inlined core::ub_checks::check_language_ub) { @@ -26,23 +24,16 @@ fn unwrap_unchecked(_1: Option) -> T { bb0: { StorageLive(_2); _2 = discriminant(_1); - switchInt(move _2) -> [0: bb1, 1: bb2, otherwise: bb3]; + switchInt(move _2) -> [0: bb2, 1: bb1, otherwise: bb2]; } bb1: { - StorageLive(_3); - _3 = UbChecks(); - assume(_3); - _4 = unreachable_unchecked::precondition_check() -> [return: bb3, unwind unreachable]; - } - - bb2: { _0 = ((_1 as Some).0: T); StorageDead(_2); return; } - bb3: { + bb2: { unreachable; } } diff --git a/tests/mir-opt/instsimplify/ub_check.rs b/tests/mir-opt/instsimplify/ub_check.rs new file mode 100644 index 0000000000000..fc568abcd601c --- /dev/null +++ b/tests/mir-opt/instsimplify/ub_check.rs @@ -0,0 +1,16 @@ +//@ unit-test: InstSimplify +//@ compile-flags: -Cdebug-assertions=no -Zinline-mir + +// EMIT_MIR ub_check.unwrap_unchecked.InstSimplify.diff +pub fn unwrap_unchecked(x: Option) -> i32 { + // CHECK-LABEL: fn unwrap_unchecked( + // CHECK-NOT: UbChecks() + // CHECK: [[assume:_.*]] = const false; + // CHECK-NEXT: assume([[assume]]); + // CHECK-NEXT: unreachable_unchecked::precondition_check + unsafe { x.unwrap_unchecked() } +} + +fn main() { + unwrap_unchecked(None); +} diff --git a/tests/mir-opt/instsimplify/ub_check.unwrap_unchecked.InstSimplify.diff b/tests/mir-opt/instsimplify/ub_check.unwrap_unchecked.InstSimplify.diff new file mode 100644 index 0000000000000..f3402fde05b90 --- /dev/null +++ b/tests/mir-opt/instsimplify/ub_check.unwrap_unchecked.InstSimplify.diff @@ -0,0 +1,59 @@ +- // MIR for `unwrap_unchecked` before InstSimplify ++ // MIR for `unwrap_unchecked` after InstSimplify + + fn unwrap_unchecked(_1: Option) -> i32 { + debug x => _1; + let mut _0: i32; + let mut _2: std::option::Option; + scope 1 { + scope 2 (inlined #[track_caller] Option::::unwrap_unchecked) { + debug self => _2; + let mut _3: isize; + scope 3 { + debug val => _0; + } + scope 4 { + scope 5 (inlined unreachable_unchecked) { + let mut _4: bool; + let _5: (); + scope 6 { + } + scope 7 (inlined core::ub_checks::check_language_ub) { + scope 8 (inlined core::ub_checks::check_language_ub::runtime) { + } + } + } + } + } + } + + bb0: { + StorageLive(_2); + _2 = _1; + StorageLive(_3); + StorageLive(_5); + _3 = discriminant(_2); + switchInt(move _3) -> [0: bb2, 1: bb3, otherwise: bb1]; + } + + bb1: { + unreachable; + } + + bb2: { + StorageLive(_4); +- _4 = UbChecks(); ++ _4 = const false; + assume(_4); + _5 = unreachable_unchecked::precondition_check() -> [return: bb1, unwind unreachable]; + } + + bb3: { + _0 = move ((_2 as Some).0: i32); + StorageDead(_5); + StorageDead(_3); + StorageDead(_2); + return; + } + } + diff --git a/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir index 455e4ba724411..ebe846e8a5104 100644 --- a/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/duplicate_switch_targets.ub_if_b.PreCodegen.after.mir @@ -5,8 +5,6 @@ fn ub_if_b(_1: Thing) -> Thing { let mut _0: Thing; let mut _2: isize; scope 1 (inlined unreachable_unchecked) { - let mut _3: bool; - let _4: (); scope 2 { } scope 3 (inlined core::ub_checks::check_language_ub) { @@ -17,7 +15,7 @@ fn ub_if_b(_1: Thing) -> Thing { bb0: { _2 = discriminant(_1); - switchInt(move _2) -> [0: bb1, 1: bb2, otherwise: bb3]; + switchInt(move _2) -> [0: bb1, 1: bb2, otherwise: bb2]; } bb1: { @@ -26,13 +24,6 @@ fn ub_if_b(_1: Thing) -> Thing { } bb2: { - StorageLive(_3); - _3 = UbChecks(); - assume(_3); - _4 = unreachable_unchecked::precondition_check() -> [return: bb3, unwind unreachable]; - } - - bb3: { unreachable; } }