From d1797389f3120fcf0e865361a07171f6ac1f4c6e Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 09:15:04 +0000 Subject: [PATCH] Update comments and simplify miri allocator handling a bit --- .../src/back/symbol_export.rs | 8 +- .../rustc_monomorphize/src/partitioning.rs | 15 +-- library/std/src/alloc.rs | 3 +- src/tools/miri/src/shims/alloc.rs | 10 -- src/tools/miri/src/shims/foreign_items.rs | 92 +++++-------------- 5 files changed, 30 insertions(+), 98 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index f8040149f13b0..0d5761ed9afae 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -1,6 +1,6 @@ use std::collections::hash_map::Entry::*; -use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, NO_ALLOC_SHIM_IS_UNSTABLE}; +use rustc_ast::expand::allocator::NO_ALLOC_SHIM_IS_UNSTABLE; use rustc_data_structures::unord::UnordMap; use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LOCAL_CRATE, LocalDefId}; @@ -207,10 +207,8 @@ fn exported_symbols_provider_local( // Mark allocator shim symbols as exported only if they were generated. if needs_allocator_shim(tcx) { - for symbol_name in ALLOCATOR_METHODS - .iter() - .map(|method| format!("__rust_{}", method.name)) - .chain(["__rust_alloc_error_handler".to_string(), OomStrategy::SYMBOL.to_string()]) + for symbol_name in + ["__rust_alloc_error_handler".to_string(), OomStrategy::SYMBOL.to_string()] { let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name)); diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index e4933c08208bf..cb3f83984578f 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -889,18 +889,9 @@ fn mono_item_visibility<'tcx>( // // FIXME update comment // * Second is "std internal symbols". Currently this is primarily used - // for allocator symbols. Allocators are a little weird in their - // implementation, but the idea is that the compiler, at the last - // minute, defines an allocator with an injected object file. The - // `alloc` crate references these symbols (`__rust_alloc`) and the - // definition doesn't get hooked up until a linked crate artifact is - // generated. - // - // The symbols synthesized by the compiler (`__rust_alloc`) are thin - // veneers around the actual implementation, some other symbol which - // implements the same ABI. These symbols (things like `__rg_alloc`, - // `__rdl_alloc`, `__rde_alloc`, etc), are all tagged with "std - // internal symbols". + // for allocator symbols and the unwinder runtime to allow cyclic + // dependencies between the defining and using crate and to allow + // replacing them. // // The std-internal symbols here **should not show up in a dll as an // exported interface**, so they return `false` from diff --git a/library/std/src/alloc.rs b/library/std/src/alloc.rs index 24b982c3e964b..300f530ca764b 100644 --- a/library/std/src/alloc.rs +++ b/library/std/src/alloc.rs @@ -385,8 +385,7 @@ pub mod __default_lib_allocator { // These are used as a fallback for implementing the `__rust_alloc`, etc symbols // (see `src/liballoc/alloc.rs`) when there is no `#[global_allocator]` attribute. - // for symbol names src/librustc_ast/expand/allocator.rs - // for signatures src/librustc_allocator/lib.rs + // for symbol names and signatures see compiler/rustc_ast/src/expand/allocator.rs // linkage directives are provided as part of the current compiler allocator // ABI diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index 2e9f6287c212d..4dc9ba354a05a 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -51,16 +51,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Align::from_bytes(prev_power_of_two(size)).unwrap() } - /// Emulates calling the internal __rust_* allocator functions - fn emulate_allocator(&mut self) -> InterpResult<'tcx, EmulateItemResult> { - // When `#[global_allocator]` is used, `__rust_*` is defined by the macro expansion - // of this attribute. As such we have to call an exported Rust function, - // and not execute any Miri shim. Somewhat unintuitively doing so is done - // by returning `NotSupported`, which triggers the `lookup_exported_symbol` - // fallback case in `emulate_foreign_item`. - interp_ok(EmulateItemResult::NotSupported) - } - fn malloc(&mut self, size: u64, zero_init: bool) -> InterpResult<'tcx, Pointer> { let this = self.eval_context_mut(); let align = this.malloc_align(size); diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index caa48fe8fcf5c..7d0b2f93c12ae 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -527,80 +527,34 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } // Rust allocation - "__rust_alloc" | "miri_alloc" => { - let default = |ecx: &mut MiriInterpCx<'tcx>| { - // Only call `check_shim` when `#[global_allocator]` isn't used. When that - // macro is used, we act like no shim exists, so that the exported function can run. - let [size, align] = ecx.check_shim(abi, ExternAbi::Rust, link_name, args)?; - let size = ecx.read_target_usize(size)?; - let align = ecx.read_target_usize(align)?; - - ecx.check_rustc_alloc_request(size, align)?; - - let memory_kind = match link_name.as_str() { - "__rust_alloc" => MiriMemoryKind::Rust, - "miri_alloc" => MiriMemoryKind::Miri, - _ => unreachable!(), - }; + "miri_alloc" => { + let [size, align] = this.check_shim(abi, ExternAbi::Rust, link_name, args)?; + let size = this.read_target_usize(size)?; + let align = this.read_target_usize(align)?; - let ptr = ecx.allocate_ptr( - Size::from_bytes(size), - Align::from_bytes(align).unwrap(), - memory_kind.into(), - )?; + this.check_rustc_alloc_request(size, align)?; - ecx.write_pointer(ptr, dest) - }; + let ptr = this.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + MiriMemoryKind::Miri.into(), + )?; - match link_name.as_str() { - "__rust_alloc" => return this.emulate_allocator(), - "miri_alloc" => { - default(this)?; - return interp_ok(EmulateItemResult::NeedsReturn); - } - _ => unreachable!(), - } + this.write_pointer(ptr, dest); } - "__rust_alloc_zeroed" => { - return this.emulate_allocator(); - } - "__rust_dealloc" | "miri_dealloc" => { - let default = |ecx: &mut MiriInterpCx<'tcx>| { - // See the comment for `__rust_alloc` why `check_shim` is only called in the - // default case. - let [ptr, old_size, align] = - ecx.check_shim(abi, ExternAbi::Rust, link_name, args)?; - let ptr = ecx.read_pointer(ptr)?; - let old_size = ecx.read_target_usize(old_size)?; - let align = ecx.read_target_usize(align)?; - - let memory_kind = match link_name.as_str() { - "__rust_dealloc" => MiriMemoryKind::Rust, - "miri_dealloc" => MiriMemoryKind::Miri, - _ => unreachable!(), - }; - - // No need to check old_size/align; we anyway check that they match the allocation. - ecx.deallocate_ptr( - ptr, - Some((Size::from_bytes(old_size), Align::from_bytes(align).unwrap())), - memory_kind.into(), - ) - }; + "miri_dealloc" => { + let [ptr, old_size, align] = + this.check_shim(abi, ExternAbi::Rust, link_name, args)?; + let ptr = this.read_pointer(ptr)?; + let old_size = this.read_target_usize(old_size)?; + let align = this.read_target_usize(align)?; - match link_name.as_str() { - "__rust_dealloc" => { - return this.emulate_allocator(); - } - "miri_dealloc" => { - default(this)?; - return interp_ok(EmulateItemResult::NeedsReturn); - } - _ => unreachable!(), - } - } - "__rust_realloc" => { - return this.emulate_allocator(); + // No need to check old_size/align; we anyway check that they match the allocation. + this.deallocate_ptr( + ptr, + Some((Size::from_bytes(old_size), Align::from_bytes(align).unwrap())), + MiriMemoryKind::Miri.into(), + ); } // C memory handling functions