From 18bf9ce8528a5c26ecfd92d43467d9b3a39aaf3c Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 23 Aug 2024 23:50:07 -0400 Subject: [PATCH 1/4] Try enabling precondition checks on ptr::{read,write} --- library/core/src/ptr/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index d7ed4edcc0041..8d7eb8ea20d9f 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1425,7 +1425,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", @@ -1634,7 +1633,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", From d527afc0b1e32f8fec95afbae21e221ab2b8f1dd Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 27 Aug 2024 20:20:21 -0400 Subject: [PATCH 2/4] Add #[rustc_no_ubchecks] --- compiler/rustc_feature/src/builtin_attrs.rs | 4 ++++ .../rustc_mir_transform/src/instsimplify.rs | 23 +++++++++++++++---- compiler/rustc_span/src/symbol.rs | 1 + library/alloc/src/boxed.rs | 1 + library/core/src/mem/mod.rs | 1 + library/core/src/ptr/mod.rs | 1 + tests/codegen/mem-replace-big-type.rs | 1 - tests/codegen/mem-replace-simple-type.rs | 1 - ...m_replace.PreCodegen.after.panic-abort.mir | 14 ++++++++++- ..._replace.PreCodegen.after.panic-unwind.mir | 14 ++++++++++- tests/mir-opt/pre-codegen/mem_replace.rs | 1 - tests/mir-opt/pre-codegen/ptr_offset.rs | 1 - 12 files changed, 52 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 2747a14d60a5c..38a9b039b0ea9 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -997,6 +997,10 @@ pub const 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_intrinsic_must_be_overridden, Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::Yes, "the `#[rustc_intrinsic_must_be_overridden]` attribute is used to declare intrinsics without real bodies", diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs index 3ec553d0ba0c5..5ede3e7b1a64c 100644 --- a/compiler/rustc_mir_transform/src/instsimplify.rs +++ b/compiler/rustc_mir_transform/src/instsimplify.rs @@ -37,19 +37,27 @@ impl<'tcx> 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, - param_env: tcx.param_env_reveal_all_normalized(body.source.def_id()), + param_env: tcx.param_env_reveal_all_normalized(def_id), }; let preserve_ub_checks = attr::contains_name(tcx.hir().krate_attrs(), sym::rustc_preserve_ub_checks); + let remove_ub_checks = tcx.has_attr(def_id, sym::rustc_no_ubchecks); 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); + if remove_ub_checks { + ctx.simplify_ub_check(&statement.source_info, rvalue, false); + } else if !preserve_ub_checks { + ctx.simplify_ub_check( + &statement.source_info, + rvalue, + tcx.sess.ub_checks(), + ); } ctx.simplify_bool_cmp(&statement.source_info, rvalue); ctx.simplify_ref_deref(&statement.source_info, rvalue); @@ -199,9 +207,14 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> { } } - fn simplify_ub_check(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { + fn simplify_ub_check( + &self, + source_info: &SourceInfo, + rvalue: &mut Rvalue<'tcx>, + ub_checks: bool, + ) { if let Rvalue::NullaryOp(NullOp::UbChecks, _) = *rvalue { - 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: source_info.span, 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 aba5673272568..14889dc4251d5 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1656,6 +1656,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 38b1766c17440..1a0a6f357955d 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1222,6 +1222,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/mod.rs b/library/core/src/mem/mod.rs index e602b497d171a..49831f3cae347 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -859,6 +859,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_unstable(feature = "const_replace", issue = "83164")] #[cfg_attr(not(test), 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 8d7eb8ea20d9f..ab2c8a482dd07 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1195,6 +1195,7 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { /// `swap_nonoverlapping` tries to use) so no need to manually SIMD it. #[inline] #[rustc_const_unstable(feature = "const_swap", issue = "83163")] +#[cfg_attr(not(bootstrap), rustc_no_ubchecks)] const unsafe fn swap_nonoverlapping_simple_untyped(x: *mut T, y: *mut T, count: usize) { let x = x.cast::>(); let y = y.cast::>(); diff --git a/tests/codegen/mem-replace-big-type.rs b/tests/codegen/mem-replace-big-type.rs index d5eadda4469f6..be67fd9847a6b 100644 --- a/tests/codegen/mem-replace-big-type.rs +++ b/tests/codegen/mem-replace-big-type.rs @@ -4,7 +4,6 @@ // known to be `1` after inlining). //@ compile-flags: -C no-prepopulate-passes -Zinline-mir=no -//@ ignore-debug: precondition checks in ptr::read make them a bad candidate for MIR inlining #![crate_type = "lib"] diff --git a/tests/codegen/mem-replace-simple-type.rs b/tests/codegen/mem-replace-simple-type.rs index 7209fa21925ad..1cc45c096772c 100644 --- a/tests/codegen/mem-replace-simple-type.rs +++ b/tests/codegen/mem-replace-simple-type.rs @@ -1,6 +1,5 @@ //@ compile-flags: -O -C no-prepopulate-passes //@ only-x86_64 (to not worry about usize differing) -//@ ignore-debug: precondition checks make mem::replace not a candidate for MIR inlining #![crate_type = "lib"] diff --git a/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-abort.mir index ed494f6e74cf3..1949dbd244e1e 100644 --- a/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-abort.mir @@ -6,10 +6,22 @@ fn mem_replace(_1: &mut u32, _2: u32) -> u32 { let mut _0: u32; scope 1 (inlined std::mem::replace::) { scope 2 { - scope 4 (inlined std::ptr::write::) { + scope 7 (inlined std::ptr::write::) { + scope 8 (inlined core::ub_checks::check_language_ub) { + scope 9 (inlined core::ub_checks::check_language_ub::runtime) { + } + } + scope 10 (inlined align_of::) { + } } } scope 3 (inlined std::ptr::read::) { + scope 4 (inlined core::ub_checks::check_language_ub) { + scope 5 (inlined core::ub_checks::check_language_ub::runtime) { + } + } + scope 6 (inlined align_of::) { + } } } diff --git a/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-unwind.mir index ed494f6e74cf3..1949dbd244e1e 100644 --- a/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-unwind.mir +++ b/tests/mir-opt/pre-codegen/mem_replace.mem_replace.PreCodegen.after.panic-unwind.mir @@ -6,10 +6,22 @@ fn mem_replace(_1: &mut u32, _2: u32) -> u32 { let mut _0: u32; scope 1 (inlined std::mem::replace::) { scope 2 { - scope 4 (inlined std::ptr::write::) { + scope 7 (inlined std::ptr::write::) { + scope 8 (inlined core::ub_checks::check_language_ub) { + scope 9 (inlined core::ub_checks::check_language_ub::runtime) { + } + } + scope 10 (inlined align_of::) { + } } } scope 3 (inlined std::ptr::read::) { + scope 4 (inlined core::ub_checks::check_language_ub) { + scope 5 (inlined core::ub_checks::check_language_ub::runtime) { + } + } + scope 6 (inlined align_of::) { + } } } diff --git a/tests/mir-opt/pre-codegen/mem_replace.rs b/tests/mir-opt/pre-codegen/mem_replace.rs index a68fe31f6094d..3fa5674d73285 100644 --- a/tests/mir-opt/pre-codegen/mem_replace.rs +++ b/tests/mir-opt/pre-codegen/mem_replace.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir -//@ ignore-debug: 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 88ee00296a078..f06b9d3c7bbdf 100644 --- a/tests/mir-opt/pre-codegen/ptr_offset.rs +++ b/tests/mir-opt/pre-codegen/ptr_offset.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir -//@ ignore-debug: precondition checks are under cfg(debug_assertions) // EMIT_MIR_FOR_EACH_PANIC_STRATEGY #![crate_type = "lib"] From 676f65a50fdb1a64436f0fd5b18a3afb5e673afb Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 29 Aug 2024 19:13:09 -0400 Subject: [PATCH 3/4] paper over the ICE --- compiler/rustc_mir_transform/src/instsimplify.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs index 5ede3e7b1a64c..b1034a5a88cf9 100644 --- a/compiler/rustc_mir_transform/src/instsimplify.rs +++ b/compiler/rustc_mir_transform/src/instsimplify.rs @@ -45,7 +45,12 @@ impl<'tcx> MirPass<'tcx> for InstSimplify { }; let preserve_ub_checks = attr::contains_name(tcx.hir().krate_attrs(), sym::rustc_preserve_ub_checks); - let remove_ub_checks = tcx.has_attr(def_id, sym::rustc_no_ubchecks); + // FIXME(async_closures) tcx.has_attr on async closures seems to ICE. Not sure why. + 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() { match statement.kind { From e001e0b3cebff14fbd9f97f8bdcecbb52692deac Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 30 Aug 2024 00:06:16 -0400 Subject: [PATCH 4/4] Another --- library/core/src/mem/manually_drop.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/mem/manually_drop.rs b/library/core/src/mem/manually_drop.rs index be5cee2e85267..64eaac8ea2835 100644 --- a/library/core/src/mem/manually_drop.rs +++ b/library/core/src/mem/manually_drop.rs @@ -111,6 +111,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.