From 73dc4d4917c3103408f8ef6c9701de35ea203d8e Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 23 Aug 2024 23:50:07 -0400 Subject: [PATCH] Try enabling precondition checks on ptr::{read,write} --- compiler/rustc_feature/src/builtin_attrs.rs | 4 ++++ compiler/rustc_mir_transform/src/instsimplify.rs | 16 ++++++++++++---- compiler/rustc_span/src/symbol.rs | 1 + library/alloc/src/boxed.rs | 1 + library/core/src/mem/manually_drop.rs | 1 + library/core/src/mem/mod.rs | 1 + library/core/src/ptr/mod.rs | 3 +-- tests/codegen/mem-replace-big-type.rs | 2 -- tests/codegen/mem-replace-simple-type.rs | 2 -- tests/mir-opt/pre-codegen/mem_replace.rs | 2 -- tests/mir-opt/pre-codegen/ptr_offset.rs | 2 +- 11 files changed, 22 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 1e33e2e9393f7..19fb7d6a76f84 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -1036,6 +1036,10 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_no_mir_inline, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::Yes, "#[rustc_no_mir_inline] prevents the MIR inliner from inlining a function while not affecting codegen" ), + rustc_attr!( + rustc_no_ubchecks, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::Yes, + "#[rustc_no_ubchecks] asks the compiler to delete UB checks from a function" + ), rustc_attr!( rustc_force_inline, Normal, template!(Word, NameValueStr: "reason"), WarnFollowing, EncodeCrossCrate::Yes, "#[rustc_force_inline] forces a free function to be inlined" diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs index a8d6aaa50a294..e8b0b6303b90a 100644 --- a/compiler/rustc_mir_transform/src/instsimplify.rs +++ b/compiler/rustc_mir_transform/src/instsimplify.rs @@ -30,6 +30,7 @@ impl<'tcx> crate::MirPass<'tcx> for InstSimplify { } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + let def_id = body.source.def_id(); let ctx = InstSimplifyContext { tcx, local_decls: &body.local_decls, @@ -37,14 +38,21 @@ impl<'tcx> crate::MirPass<'tcx> for InstSimplify { }; let preserve_ub_checks = attr::contains_name(tcx.hir_krate_attrs(), sym::rustc_preserve_ub_checks); + let remove_ub_checks = if tcx.is_coroutine(def_id) { + false + } else { + tcx.has_attr(def_id, sym::rustc_no_ubchecks) + }; for block in body.basic_blocks.as_mut() { for statement in block.statements.iter_mut() { let StatementKind::Assign(box (.., rvalue)) = &mut statement.kind else { continue; }; - if !preserve_ub_checks { - ctx.simplify_ub_check(rvalue); + if remove_ub_checks { + ctx.simplify_ub_check(rvalue, false); + } else if !preserve_ub_checks { + ctx.simplify_ub_check(rvalue, tcx.sess.ub_checks()); } ctx.simplify_bool_cmp(rvalue); ctx.simplify_ref_deref(rvalue); @@ -168,10 +176,10 @@ impl<'tcx> InstSimplifyContext<'_, 'tcx> { } } - fn simplify_ub_check(&self, rvalue: &mut Rvalue<'tcx>) { + fn simplify_ub_check(&self, rvalue: &mut Rvalue<'tcx>, ub_checks: bool) { let Rvalue::NullaryOp(NullOp::UbChecks, _) = *rvalue else { return }; - let const_ = Const::from_bool(self.tcx, self.tcx.sess.ub_checks()); + let const_ = Const::from_bool(self.tcx, ub_checks); let constant = ConstOperand { span: DUMMY_SP, const_, user_ty: None }; *rvalue = Rvalue::Use(Operand::Constant(Box::new(constant))); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 986370f501918..3489508e6b96b 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1829,6 +1829,7 @@ symbols! { rustc_never_returns_null_ptr, rustc_never_type_options, rustc_no_mir_inline, + rustc_no_ubchecks, rustc_nonnull_optimization_guaranteed, rustc_nounwind, rustc_object_lifetime_default, diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 4536f55544354..9c9225d678f45 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1372,6 +1372,7 @@ impl Box { #[must_use = "losing the pointer will leak memory"] #[unstable(feature = "allocator_api", issue = "32838")] #[inline] + #[cfg_attr(not(bootstrap), rustc_no_ubchecks)] pub fn into_raw_with_allocator(b: Self) -> (*mut T, A) { let mut b = mem::ManuallyDrop::new(b); // We carefully get the raw pointer out in a way that Miri's aliasing model understands what diff --git a/library/core/src/mem/manually_drop.rs b/library/core/src/mem/manually_drop.rs index 7d519384e373c..d5da724afc0ce 100644 --- a/library/core/src/mem/manually_drop.rs +++ b/library/core/src/mem/manually_drop.rs @@ -217,6 +217,7 @@ impl ManuallyDrop { #[must_use = "if you don't need the value, you can use `ManuallyDrop::drop` instead"] #[stable(feature = "manually_drop_take", since = "1.42.0")] #[inline] + #[cfg_attr(not(bootstrap), rustc_no_ubchecks)] pub unsafe fn take(slot: &mut ManuallyDrop) -> T { // SAFETY: we are reading from a reference, which is guaranteed // to be valid for reads. diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index 27387754633d1..231200fc63793 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -847,6 +847,7 @@ pub fn take(dest: &mut T) -> T { #[must_use = "if you don't need the old value, you can just assign the new value directly"] #[rustc_const_stable(feature = "const_replace", since = "1.83.0")] #[rustc_diagnostic_item = "mem_replace"] +#[cfg_attr(not(bootstrap), rustc_no_ubchecks)] pub const fn replace(dest: &mut T, src: T) -> T { // It may be tempting to use `swap` to avoid `unsafe` here. Don't! // The compiler optimizes the implementation below to two `memcpy`s diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 2357ba23aa0d2..d0676190e8b03 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1113,6 +1113,7 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { /// Same behavior and safety conditions as [`swap_nonoverlapping`] #[inline] +#[cfg_attr(not(bootstrap), rustc_no_ubchecks)] const unsafe fn swap_nonoverlapping_const(x: *mut T, y: *mut T, count: usize) { let mut i = 0; while i < count { @@ -1407,7 +1408,6 @@ pub const unsafe fn read(src: *const T) -> T { // SAFETY: the caller must guarantee that `src` is valid for reads. unsafe { - #[cfg(debug_assertions)] // Too expensive to always enable (for now?) ub_checks::assert_unsafe_precondition!( check_language_ub, "ptr::read requires that the pointer argument is aligned and non-null", @@ -1607,7 +1607,6 @@ pub const unsafe fn write(dst: *mut T, src: T) { // `dst` cannot overlap `src` because the caller has mutable access // to `dst` while `src` is owned by this function. unsafe { - #[cfg(debug_assertions)] // Too expensive to always enable (for now?) ub_checks::assert_unsafe_precondition!( check_language_ub, "ptr::write requires that the pointer argument is aligned and non-null", diff --git a/tests/codegen/mem-replace-big-type.rs b/tests/codegen/mem-replace-big-type.rs index 0b2229ba7d104..34a8bea0919d7 100644 --- a/tests/codegen/mem-replace-big-type.rs +++ b/tests/codegen/mem-replace-big-type.rs @@ -4,8 +4,6 @@ // known to be `1` after inlining). //@ compile-flags: -C no-prepopulate-passes -Zinline-mir=no -//@ ignore-std-debug-assertions -// Reason: precondition checks in ptr::read make them a bad candidate for MIR inlining //@ needs-deterministic-layouts #![crate_type = "lib"] diff --git a/tests/codegen/mem-replace-simple-type.rs b/tests/codegen/mem-replace-simple-type.rs index 9f3c6bacb712c..cc8830a700c03 100644 --- a/tests/codegen/mem-replace-simple-type.rs +++ b/tests/codegen/mem-replace-simple-type.rs @@ -1,7 +1,5 @@ //@ compile-flags: -Copt-level=3 -C no-prepopulate-passes //@ only-x86_64 (to not worry about usize differing) -//@ ignore-std-debug-assertions -// Reason: precondition checks make mem::replace not a candidate for MIR inlining #![crate_type = "lib"] diff --git a/tests/mir-opt/pre-codegen/mem_replace.rs b/tests/mir-opt/pre-codegen/mem_replace.rs index be23dcdb22ae5..3fa5674d73285 100644 --- a/tests/mir-opt/pre-codegen/mem_replace.rs +++ b/tests/mir-opt/pre-codegen/mem_replace.rs @@ -1,7 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir -//@ ignore-std-debug-assertions -// Reason: precondition checks on ptr::read/write are under cfg(debug_assertions) // EMIT_MIR_FOR_EACH_PANIC_STRATEGY #![crate_type = "lib"] diff --git a/tests/mir-opt/pre-codegen/ptr_offset.rs b/tests/mir-opt/pre-codegen/ptr_offset.rs index 120be99fc94af..138ff74483038 100644 --- a/tests/mir-opt/pre-codegen/ptr_offset.rs +++ b/tests/mir-opt/pre-codegen/ptr_offset.rs @@ -1,6 +1,6 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir -//@ ignore-std-debug-assertions (precondition checks are under cfg(debug_assertions)) +//@ ignore-std-debug-assertions (precondition checks on ptr::add are under cfg(debug_assertions)) // EMIT_MIR_FOR_EACH_PANIC_STRATEGY #![crate_type = "lib"]