From 1f405735078ac34be300fd2bd7c532aaa0bb6fc5 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Jun 2025 10:22:49 -0400 Subject: [PATCH 1/2] Skip collecting no-op DropGlue in vtables Since 122662 this no longer gets used in vtables, so we're safe to fully drop generating these empty functions. Those are eventually cleaned up by LLVM, but it's wasteful to produce them in the first place. This also adds a missing test for fn-ptr casts, which do still need to generate no-op drop glue. It's possible a future optimization could point all of those at the same drop glue (e.g., for *mut ()) rather than for each separate type, but that would require extra work for CFI and isn't particularly easy to do anyway. --- compiler/rustc_monomorphize/src/collector.rs | 12 ++++++++-- .../item-collection/drop-glue-noop.rs | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tests/codegen-units/item-collection/drop-glue-noop.rs diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 173030e0326e8..e90e32ebebb9f 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -949,6 +949,9 @@ fn visit_instance_use<'tcx>( } ty::InstanceKind::DropGlue(_, None) => { // Don't need to emit noop drop glue if we are calling directly. + // + // Note that we also optimize away the call to visit_instance_use in vtable construction + // (see create_mono_items_for_vtable_methods). if !is_direct_call { output.push(create_fn_mono_item(tcx, instance, source)); } @@ -1177,8 +1180,13 @@ fn create_mono_items_for_vtable_methods<'tcx>( output.extend(methods); } - // Also add the destructor. - visit_drop_use(tcx, impl_ty, false, source, output); + // Also add the destructor, if it's necessary. + // + // This matches the check in vtable_allocation_provider in middle/ty/vtable.rs, + // if we don't need drop we're not adding an actual pointer to the vtable. + if impl_ty.needs_drop(tcx, ty::TypingEnv::fully_monomorphized()) { + visit_drop_use(tcx, impl_ty, false, source, output); + } } /// Scans the CTFE alloc in order to find function pointers and statics that must be monomorphized. diff --git a/tests/codegen-units/item-collection/drop-glue-noop.rs b/tests/codegen-units/item-collection/drop-glue-noop.rs new file mode 100644 index 0000000000000..604ba883bb289 --- /dev/null +++ b/tests/codegen-units/item-collection/drop-glue-noop.rs @@ -0,0 +1,23 @@ +//@ compile-flags:-Clink-dead-code -Zmir-opt-level=0 + +#![deny(dead_code)] +#![crate_type = "lib"] + +//~ MONO_ITEM fn start +#[no_mangle] +pub fn start(_: isize, _: *const *const u8) -> isize { + // No item produced for this, it's a no-op drop and so is removed. + unsafe { + std::ptr::drop_in_place::(&mut 0); + } + + // No choice but to codegen for indirect drop as a function pointer, since we have to produce a + // function with the right signature. In vtables we can avoid that (tested in + // instantiation-through-vtable.rs) because we special case null pointer for drop glue since + // #122662. + // + //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ drop_glue_noop-cgu.0[External] + std::ptr::drop_in_place:: as unsafe fn(*mut u64); + + 0 +} From b4568d9135e55cfc8739b465970aa636a0d45c1b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Jun 2025 10:36:53 -0400 Subject: [PATCH 2/2] Fix tests to drop now-skipped codegen --- .../instantiation-through-vtable.rs | 2 -- .../item-collection/non-generic-closures.rs | 29 ++++++++++++++++--- .../codegen-units/item-collection/unsizing.rs | 24 +++++++++++---- ...tadata-id-itanium-cxx-abi-drop-in-place.rs | 14 +++++---- 4 files changed, 53 insertions(+), 16 deletions(-) diff --git a/tests/codegen-units/item-collection/instantiation-through-vtable.rs b/tests/codegen-units/item-collection/instantiation-through-vtable.rs index 8f13fd558083c..7882a526b6829 100644 --- a/tests/codegen-units/item-collection/instantiation-through-vtable.rs +++ b/tests/codegen-units/item-collection/instantiation-through-vtable.rs @@ -24,7 +24,6 @@ impl Trait for Struct { pub fn start(_: isize, _: *const *const u8) -> isize { let s1 = Struct { _a: 0u32 }; - //~ MONO_ITEM fn std::ptr::drop_in_place::> - shim(None) @@ instantiation_through_vtable-cgu.0[External] //~ MONO_ITEM fn as Trait>::foo //~ MONO_ITEM fn as Trait>::bar let r1 = &s1 as &Trait; @@ -32,7 +31,6 @@ pub fn start(_: isize, _: *const *const u8) -> isize { r1.bar(); let s1 = Struct { _a: 0u64 }; - //~ MONO_ITEM fn std::ptr::drop_in_place::> - shim(None) @@ instantiation_through_vtable-cgu.0[External] //~ MONO_ITEM fn as Trait>::foo //~ MONO_ITEM fn as Trait>::bar let _ = &s1 as &Trait; diff --git a/tests/codegen-units/item-collection/non-generic-closures.rs b/tests/codegen-units/item-collection/non-generic-closures.rs index 124fe7e3b69a0..2d9c461e6fd2d 100644 --- a/tests/codegen-units/item-collection/non-generic-closures.rs +++ b/tests/codegen-units/item-collection/non-generic-closures.rs @@ -1,4 +1,4 @@ -//@ compile-flags:-Clink-dead-code -Zinline-mir=no +//@ compile-flags:-Clink-dead-code -Zinline-mir=no -O #![deny(dead_code)] #![crate_type = "lib"] @@ -22,9 +22,8 @@ fn assigned_to_variable_but_not_executed() { //~ MONO_ITEM fn assigned_to_variable_executed_indirectly @@ non_generic_closures-cgu.0[External] fn assigned_to_variable_executed_indirectly() { //~ MONO_ITEM fn assigned_to_variable_executed_indirectly::{closure#0} @@ non_generic_closures-cgu.0[External] - //~ MONO_ITEM fn <{closure@TEST_PATH:28:13: 28:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[External] - //~ MONO_ITEM fn <{closure@TEST_PATH:28:13: 28:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[External] - //~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:28:13: 28:21}> - shim(None) @@ non_generic_closures-cgu.0[External] + //~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[External] + //~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[External] let f = |a: i32| { let _ = a + 2; }; @@ -40,6 +39,20 @@ fn assigned_to_variable_executed_directly() { f(4); } +// Make sure we generate mono items for stateful closures that need dropping +//~ MONO_ITEM fn with_drop @@ non_generic_closures-cgu.0[External] +fn with_drop(v: PresentDrop) { + //~ MONO_ITEM fn with_drop::{closure#0} @@ non_generic_closures-cgu.0[External] + //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(Some(PresentDrop)) @@ non_generic_closures-cgu.0[Internal] + //~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:49:14: 49:24}> - shim(Some({closure@TEST_PATH:49:14: 49:24})) @@ non_generic_closures-cgu.0[Internal] + + let _f = |a: usize| { + let _ = a + 2; + //~ MONO_ITEM fn std::mem::drop:: @@ non_generic_closures-cgu.0[External] + drop(v); + }; +} + //~ MONO_ITEM fn start @@ non_generic_closures-cgu.0[External] #[no_mangle] pub fn start(_: isize, _: *const *const u8) -> isize { @@ -47,6 +60,7 @@ pub fn start(_: isize, _: *const *const u8) -> isize { assigned_to_variable_but_not_executed(); assigned_to_variable_executed_directly(); assigned_to_variable_executed_indirectly(); + with_drop(PresentDrop); 0 } @@ -55,3 +69,10 @@ pub fn start(_: isize, _: *const *const u8) -> isize { fn run_closure(f: &Fn(i32)) { f(3); } + +struct PresentDrop; + +impl Drop for PresentDrop { + //~ MONO_ITEM fn ::drop @@ non_generic_closures-cgu.0[External] + fn drop(&mut self) {} +} diff --git a/tests/codegen-units/item-collection/unsizing.rs b/tests/codegen-units/item-collection/unsizing.rs index 15e42bce2495d..b751d2153a949 100644 --- a/tests/codegen-units/item-collection/unsizing.rs +++ b/tests/codegen-units/item-collection/unsizing.rs @@ -1,4 +1,4 @@ -//@ compile-flags:-Zmir-opt-level=0 +//@ compile-flags:-Zmir-opt-level=0 -O #![deny(dead_code)] #![feature(coerce_unsized)] @@ -42,33 +42,47 @@ struct Wrapper(#[allow(dead_code)] *const T); impl, U: ?Sized> CoerceUnsized> for Wrapper {} +struct PresentDrop; + +impl Drop for PresentDrop { + fn drop(&mut self) {} +} + +// Custom Coercion Case +impl Trait for PresentDrop { + fn foo(&self) {} +} + //~ MONO_ITEM fn start #[no_mangle] pub fn start(_: isize, _: *const *const u8) -> isize { // simple case let bool_sized = &true; - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _bool_unsized = bool_sized as &Trait; let char_sized = &'a'; - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _char_unsized = char_sized as &Trait; // struct field let struct_sized = &Struct { _a: 1, _b: 2, _c: 3.0f64 }; - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _struct_unsized = struct_sized as &Struct; // custom coercion let wrapper_sized = Wrapper(&0u32); - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _wrapper_sized = wrapper_sized as Wrapper; + // with drop + let droppable = &PresentDrop; + //~ MONO_ITEM fn ::drop @@ unsizing-cgu.0[Internal] + //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(Some(PresentDrop)) @@ unsizing-cgu.0[Internal] + //~ MONO_ITEM fn ::foo + let droppable = droppable as &dyn Trait; + false.foo(); 0 diff --git a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs index 2a7eca6fc1963..8fec275fd0646 100644 --- a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs +++ b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs @@ -1,5 +1,9 @@ // Verifies that type metadata identifiers for drop functions are emitted correctly. // +// Non needs_drop drop glue isn't codegen'd at all, so we don't try to check the IDs there. But we +// do check it's not emitted which should help catch bugs if we do start generating it again in the +// future. +// //@ needs-sanitizer-cfi //@ compile-flags: -Clto -Cno-prepopulate-passes -Copt-level=0 -Zsanitizer=cfi -Ctarget-feature=-crt-static @@ -10,18 +14,18 @@ // CHECK: call i1 @llvm.type.test(ptr {{%.+}}, metadata !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE") struct EmptyDrop; -// CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}EmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK-NOT: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}EmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -struct NonEmptyDrop; +struct PresentDrop; -impl Drop for NonEmptyDrop { +impl Drop for PresentDrop { fn drop(&mut self) {} - // CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}NonEmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} + // CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}PresentDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} } pub fn foo() { let _ = Box::new(EmptyDrop) as Box; - let _ = Box::new(NonEmptyDrop) as Box; + let _ = Box::new(PresentDrop) as Box; } // CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE"}