Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove box_free lang item #100036

Merged
merged 1 commit into from
Jun 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions compiler/rustc_codegen_cranelift/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
beepster4096 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -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);
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_codegen_gcc/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
100 changes: 26 additions & 74 deletions compiler/rustc_mir_dataflow/src/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)]
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//! -------------------------------------
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ symbols! {
bool,
borrowck_graphviz_format,
borrowck_graphviz_postflow,
box_free,
box_new,
box_patterns,
box_syntax,
Expand Down
7 changes: 5 additions & 2 deletions library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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());
Expand Down
10 changes: 9 additions & 1 deletion library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk, maybe that inline increased binary size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

box_free was also marked as inline, so I don't think that is the cause.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, you've probably seen this before, but just to bring everyone up to speed, one way this can be made less magical is an unsafe trait DerefMove (with similar "must be uniformly impl'd regardless of generic parameters" restriction as Drop) that makes a type into "split content and deallocation".

DerefMove::deref_move, without new language features, would likely need to return -> *mut T, and the compiler would use it wherever it assumes today built-in deref works on Box<T> (and Deref/DerefMut would be too weak), including the Drop(*some_box) it does just before invoking Drop::drop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This idea is actually the end goal of all my Box related PRs


let ptr = self.0;

unsafe {
let layout = Layout::for_value_raw(ptr.as_ptr());
self.1.deallocate(From::from(ptr.cast()), layout)
}
}
}

Expand Down
16 changes: 7 additions & 9 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -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)
}
Expand Down
16 changes: 7 additions & 9 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down
16 changes: 8 additions & 8 deletions src/doc/unstable-book/src/language-features/lang-items.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
beepster4096 marked this conversation as resolved.
Show resolved Hide resolved
A freestanding program that uses the `Box`
sugar for dynamic allocations via `malloc` and `free`:

```rust,ignore (libc-is-finicky)
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/alloc/stack_free.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Loading