Auto merge of #100036 - DrMeepster:box_free_free_box, r=oli-obk

Remove `box_free` lang item

This PR removes the `box_free` lang item, replacing it with `Box`'s `Drop` impl. Box dropping is still slightly magic because the contained value is still dropped by the compiler.

compiler/rustc_codegen_cranelift/example/mini_core.rs

@@ -546,7 +546,8 @@ impl<T> Box<T> {
 
 impl<T: ?Sized, A> Drop for Box<T, A> {
     fn drop(&mut self) {
-        // drop is currently performed by compiler.
+        // inner value is dropped by compiler
+        libc::free(self.0.pointer.0 as *mut u8);
     }
 }
 
@@ -563,11 +564,6 @@ unsafe fn allocate(size: usize, _align: usize) -> *mut u8 {
     libc::malloc(size)
 }
 
-#[lang = "box_free"]
-unsafe fn box_free<T: ?Sized>(ptr: Unique<T>, _alloc: ()) {
-    libc::free(ptr.pointer.0 as *mut u8);
-}
-
 #[lang = "drop"]
 pub trait Drop {
     fn drop(&mut self);

compiler/rustc_codegen_gcc/example/mini_core.rs

@@ -490,7 +490,8 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized, A: Allocator> CoerceUnsized<Box<U, A>> fo
 
 impl<T: ?Sized, A: Allocator> Drop for Box<T, A> {
     fn drop(&mut self) {
-        // drop is currently performed by compiler.
+        // inner value is dropped by compiler
+        libc::free(self.pointer.0 as *mut u8);
     }
 }
 
@@ -507,11 +508,6 @@ unsafe fn allocate(size: usize, _align: usize) -> *mut u8 {
     libc::malloc(size)
 }
 
-#[lang = "box_free"]
-unsafe fn box_free<T: ?Sized>(ptr: Unique<T>, _alloc: ()) {
-    libc::free(ptr.pointer.0 as *mut u8);
-}
-
 #[lang = "drop"]
 pub trait Drop {
     fn drop(&mut self);

compiler/rustc_hir/src/lang_items.rs

@@ -250,7 +250,6 @@ language_item_table! {
     FormatUnsafeArg,         sym::format_unsafe_arg,   format_unsafe_arg,          Target::Struct,         GenericRequirement::None;
 
     ExchangeMalloc,          sym::exchange_malloc,     exchange_malloc_fn,         Target::Fn,             GenericRequirement::None;
-    BoxFree,                 sym::box_free,            box_free_fn,                Target::Fn,             GenericRequirement::Minimum(1);
     DropInPlace,             sym::drop_in_place,       drop_in_place_fn,           Target::Fn,             GenericRequirement::Minimum(1);
     AllocLayout,             sym::alloc_layout,        alloc_layout,               Target::Struct,         GenericRequirement::None;
 

compiler/rustc_mir_dataflow/src/elaborate_drops.rs

@@ -409,8 +409,15 @@ where
         self.drop_ladder(fields, succ, unwind).0
     }
 
+    /// Drops the T contained in a `Box<T>` if it has not been moved out of
     #[instrument(level = "debug", ret)]
-    fn open_drop_for_box(&mut self, adt: ty::AdtDef<'tcx>, substs: SubstsRef<'tcx>) -> BasicBlock {
+    fn open_drop_for_box_contents(
+        &mut self,
+        adt: ty::AdtDef<'tcx>,
+        substs: SubstsRef<'tcx>,
+        succ: BasicBlock,
+        unwind: Unwind,
+    ) -> BasicBlock {
         // drop glue is sent straight to codegen
         // box cannot be directly dereferenced
         let unique_ty = adt.non_enum_variant().fields[FieldIdx::new(0)].ty(self.tcx(), substs);
@@ -425,11 +432,7 @@ where
 
         let interior_path = self.elaborator.deref_subpath(self.path);
 
-        let succ = self.box_free_block(adt, substs, self.succ, self.unwind);
-        let unwind_succ =
-            self.unwind.map(|unwind| self.box_free_block(adt, substs, unwind, Unwind::InCleanup));
-
-        self.drop_subpath(interior, interior_path, succ, unwind_succ)
+        self.drop_subpath(interior, interior_path, succ, unwind)
     }
 
     #[instrument(level = "debug", ret)]
@@ -453,7 +456,15 @@ where
             self.open_drop_for_adt_contents(adt, substs)
         };
 
-        if adt.has_dtor(self.tcx()) {
+        if adt.is_box() {
+            // we need to drop the inside of the box before running the destructor
+            let succ = self.destructor_call_block(contents_drop);
+            let unwind = contents_drop
+                .1
+                .map(|unwind| self.destructor_call_block((unwind, Unwind::InCleanup)));
+
+            self.open_drop_for_box_contents(adt, substs, succ, unwind)
+        } else if adt.has_dtor(self.tcx()) {
             self.destructor_call_block(contents_drop)
         } else {
             contents_drop.0
@@ -650,7 +661,13 @@ where
             }),
             is_cleanup: unwind.is_cleanup(),
         };
-        self.elaborator.patch().new_block(result)
+
+        let destructor_block = self.elaborator.patch().new_block(result);
+
+        let block_start = Location { block: destructor_block, statement_index: 0 };
+        self.elaborator.clear_drop_flag(block_start, self.path, DropFlagMode::Shallow);
+
+        self.drop_flag_test_block(destructor_block, succ, unwind)
     }
 
     /// Create a loop that drops an array:
@@ -851,13 +868,7 @@ where
                 self.open_drop_for_tuple(&tys)
             }
             ty::Tuple(fields) => self.open_drop_for_tuple(fields),
-            ty::Adt(def, substs) => {
-                if def.is_box() {
-                    self.open_drop_for_box(*def, substs)
-                } else {
-                    self.open_drop_for_adt(*def, substs)
-                }
-            }
+            ty::Adt(def, substs) => self.open_drop_for_adt(*def, substs),
             ty::Dynamic(..) => self.complete_drop(self.succ, self.unwind),
             ty::Array(ety, size) => {
                 let size = size.try_eval_target_usize(self.tcx(), self.elaborator.param_env());
@@ -905,65 +916,6 @@ where
         blk
     }
 
-    /// Creates a block that frees the backing memory of a `Box` if its drop is required (either
-    /// statically or by checking its drop flag).
-    ///
-    /// The contained value will not be dropped.
-    fn box_free_block(
-        &mut self,
-        adt: ty::AdtDef<'tcx>,
-        substs: SubstsRef<'tcx>,
-        target: BasicBlock,
-        unwind: Unwind,
-    ) -> BasicBlock {
-        let block = self.unelaborated_free_block(adt, substs, target, unwind);
-        self.drop_flag_test_block(block, target, unwind)
-    }
-
-    /// Creates a block that frees the backing memory of a `Box` (without dropping the contained
-    /// value).
-    fn unelaborated_free_block(
-        &mut self,
-        adt: ty::AdtDef<'tcx>,
-        substs: SubstsRef<'tcx>,
-        target: BasicBlock,
-        unwind: Unwind,
-    ) -> BasicBlock {
-        let tcx = self.tcx();
-        let unit_temp = Place::from(self.new_temp(tcx.mk_unit()));
-        let free_func = tcx.require_lang_item(LangItem::BoxFree, Some(self.source_info.span));
-        let args = adt
-            .variant(FIRST_VARIANT)
-            .fields
-            .iter()
-            .enumerate()
-            .map(|(i, f)| {
-                let field = FieldIdx::new(i);
-                let field_ty = f.ty(tcx, substs);
-                Operand::Move(tcx.mk_place_field(self.place, field, field_ty))
-            })
-            .collect();
-
-        let call = TerminatorKind::Call {
-            func: Operand::function_handle(tcx, free_func, substs, self.source_info.span),
-            args,
-            destination: unit_temp,
-            target: Some(target),
-            unwind: if unwind.is_cleanup() {
-                UnwindAction::Terminate
-            } else {
-                UnwindAction::Continue
-            },
-            from_hir_call: false,
-            fn_span: self.source_info.span,
-        }; // FIXME(#43234)
-        let free_block = self.new_block(unwind, call);
-
-        let block_start = Location { block: free_block, statement_index: 0 };
-        self.elaborator.clear_drop_flag(block_start, self.path, DropFlagMode::Shallow);
-        free_block
-    }
-
     fn drop_block(&mut self, target: BasicBlock, unwind: Unwind) -> BasicBlock {
         let block = TerminatorKind::Drop {
             place: self.place,

compiler/rustc_monomorphize/src/collector.rs

@@ -123,12 +123,6 @@
 //! pointers to these functions even if they never get called anywhere. This can
 //! be seen as a special case of taking a function reference.
 //!
-//! #### Boxes
-//! Since `Box` expression have special compiler support, no explicit calls to
-//! `exchange_malloc()` and `box_free()` may show up in MIR, even if the
-//! compiler will generate them. We have to observe `Rvalue::Box` expressions
-//! and Box-typed drop-statements for that purpose.
-//!
 //!
 //! Interaction with Cross-Crate Inlining
 //! -------------------------------------

compiler/rustc_span/src/symbol.rs

@@ -432,7 +432,6 @@ symbols! {
         bool,
         borrowck_graphviz_format,
         borrowck_graphviz_postflow,
-        box_free,
         box_new,
         box_patterns,
         box_syntax,

library/alloc/src/alloc.rs

@@ -4,8 +4,10 @@
 
 #[cfg(not(test))]
 use core::intrinsics;
+#[cfg(all(bootstrap, not(test)))]
 use core::intrinsics::{min_align_of_val, size_of_val};
 
+#[cfg(all(bootstrap, not(test)))]
 use core::ptr::Unique;
 #[cfg(not(test))]
 use core::ptr::{self, NonNull};
@@ -335,14 +337,15 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
     }
 }
 
-#[cfg_attr(not(test), lang = "box_free")]
+#[cfg(all(bootstrap, not(test)))]
+#[lang = "box_free"]
 #[inline]
 // This signature has to be the same as `Box`, otherwise an ICE will happen.
 // When an additional parameter to `Box` is added (like `A: Allocator`), this has to be added here as
 // well.
 // For example if `Box` is changed to  `struct Box<T: ?Sized, A: Allocator>(Unique<T>, A)`,
 // this function has to be changed to `fn box_free<T: ?Sized, A: Allocator>(Unique<T>, A)` as well.
-pub(crate) unsafe fn box_free<T: ?Sized, A: Allocator>(ptr: Unique<T>, alloc: A) {
+unsafe fn box_free<T: ?Sized, A: Allocator>(ptr: Unique<T>, alloc: A) {
     unsafe {
         let size = size_of_val(ptr.as_ref());
         let align = min_align_of_val(ptr.as_ref());

library/alloc/src/boxed.rs

@@ -1211,8 +1211,16 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Box<T, A> {
+    #[inline]
     fn drop(&mut self) {
-        // FIXME: Do nothing, drop is currently performed by compiler.
+        // the T in the Box is dropped by the compiler before the destructor is run
+
+        let ptr = self.0;
+
+        unsafe {
+            let layout = Layout::for_value_raw(ptr.as_ptr());
+            self.1.deallocate(From::from(ptr.cast()), layout)
+        }
     }
 }
 

library/alloc/src/rc.rs

@@ -270,7 +270,7 @@ use core::slice::from_raw_parts_mut;
 #[cfg(not(no_global_oom_handling))]
 use crate::alloc::handle_alloc_error;
 #[cfg(not(no_global_oom_handling))]
-use crate::alloc::{box_free, WriteCloneIntoRaw};
+use crate::alloc::WriteCloneIntoRaw;
 use crate::alloc::{AllocError, Allocator, Global, Layout};
 use crate::borrow::{Cow, ToOwned};
 #[cfg(not(no_global_oom_handling))]
@@ -1442,23 +1442,21 @@ impl<T: ?Sized> Rc<T> {
     }
 
     #[cfg(not(no_global_oom_handling))]
-    fn from_box(v: Box<T>) -> Rc<T> {
+    fn from_box(src: Box<T>) -> Rc<T> {
         unsafe {
-            let (box_unique, alloc) = Box::into_unique(v);
-            let bptr = box_unique.as_ptr();
-
-            let value_size = size_of_val(&*bptr);
-            let ptr = Self::allocate_for_ptr(bptr);
+            let value_size = size_of_val(&*src);
+            let ptr = Self::allocate_for_ptr(&*src);
 
             // Copy value as bytes
             ptr::copy_nonoverlapping(
-                bptr as *const T as *const u8,
+                &*src as *const T as *const u8,
                 &mut (*ptr).value as *mut _ as *mut u8,
                 value_size,
             );
 
             // Free the allocation without dropping its contents
-            box_free(box_unique, alloc);
+            let src = Box::from_raw(Box::into_raw(src) as *mut mem::ManuallyDrop<T>);
+            drop(src);
 
             Self::from_ptr(ptr)
         }

library/alloc/src/sync.rs

@@ -33,7 +33,7 @@ use core::sync::atomic::Ordering::{Acquire, Relaxed, Release};
 #[cfg(not(no_global_oom_handling))]
 use crate::alloc::handle_alloc_error;
 #[cfg(not(no_global_oom_handling))]
-use crate::alloc::{box_free, WriteCloneIntoRaw};
+use crate::alloc::WriteCloneIntoRaw;
 use crate::alloc::{AllocError, Allocator, Global, Layout};
 use crate::borrow::{Cow, ToOwned};
 use crate::boxed::Box;
@@ -1360,23 +1360,21 @@ impl<T: ?Sized> Arc<T> {
     }
 
     #[cfg(not(no_global_oom_handling))]
-    fn from_box(v: Box<T>) -> Arc<T> {
+    fn from_box(src: Box<T>) -> Arc<T> {
         unsafe {
-            let (box_unique, alloc) = Box::into_unique(v);
-            let bptr = box_unique.as_ptr();
-
-            let value_size = size_of_val(&*bptr);
-            let ptr = Self::allocate_for_ptr(bptr);
+            let value_size = size_of_val(&*src);
+            let ptr = Self::allocate_for_ptr(&*src);
 
             // Copy value as bytes
             ptr::copy_nonoverlapping(
-                bptr as *const T as *const u8,
+                &*src as *const T as *const u8,
                 &mut (*ptr).data as *mut _ as *mut u8,
                 value_size,
             );
 
             // Free the allocation without dropping its contents
-            box_free(box_unique, alloc);
+            let src = Box::from_raw(Box::into_raw(src) as *mut mem::ManuallyDrop<T>);
+            drop(src);
 
             Self::from_ptr(ptr)
         }

src/doc/unstable-book/src/language-features/lang-items.md

@@ -11,8 +11,8 @@ it exists. The marker is the attribute `#[lang = "..."]` and there are
 various different values of `...`, i.e. various different 'lang
 items'.
 
-For example, `Box` pointers require two lang items, one for allocation
-and one for deallocation. A freestanding program that uses the `Box`
+For example, `Box` pointers require a lang item for allocation.
+A freestanding program that uses the `Box`
 sugar for dynamic allocations via `malloc` and `free`:
 
 ```rust,ignore (libc-is-finicky)
@@ -48,9 +48,10 @@ unsafe fn allocate(size: usize, _align: usize) -> *mut u8 {
     p
 }
 
-#[lang = "box_free"]
-unsafe fn box_free<T: ?Sized>(ptr: *mut T) {
-    libc::free(ptr as *mut libc::c_void)
+impl<T> Drop for Box<T> {
+    fn drop(&mut self) {
+      libc::free(self.0.0.0 as *mut libc::c_void)
+    }
 }
 
 #[start]
@@ -84,8 +85,8 @@ Other features provided by lang items include:
   `contravariant_lifetime`, etc.
 
 Lang items are loaded lazily by the compiler; e.g. if one never uses
-`Box` then there is no need to define functions for `exchange_malloc`
-and `box_free`. `rustc` will emit an error when an item is needed
+`Box` then there is no need to define a function for `exchange_malloc`.
+`rustc` will emit an error when an item is needed
 but not found in the current crate or any that it depends on.
 
 Most lang items are defined by `libcore`, but if you're trying to build
@@ -250,7 +251,6 @@ the source code.
 - Allocations
   - `owned_box`: `liballoc/boxed.rs`
   - `exchange_malloc`: `liballoc/heap.rs`
-  - `box_free`: `liballoc/heap.rs`
 - Operands
   - `not`: `libcore/ops/bit.rs`
   - `bitand`: `libcore/ops/bit.rs`

src/tools/miri/tests/fail/alloc/stack_free.stderr

@@ -9,7 +9,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    = note: BACKTRACE:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
-   = note: inside `alloc::alloc::box_free::<i32, std::alloc::Global>` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `<std::boxed::Box<i32> as std::ops::Drop>::drop` at RUSTLIB/alloc/src/boxed.rs:LL:CC
    = note: inside `std::ptr::drop_in_place::<std::boxed::Box<i32>> - shim(Some(std::boxed::Box<i32>))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
    = note: inside `std::mem::drop::<std::boxed::Box<i32>>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
 note: inside `main`