From a60e45f45d1b54120b439796681ae5cf8b70111d Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:09:50 +0000 Subject: [PATCH 1/7] [WIP] Use weak linkage instead of compiler generated shims This is still keeping the allocator shim for the oom handler and the __rust_no_alloc_shim_is_unstable symbol for now. TODO: Update comments everywhere and test on macOS and Windows --- compiler/rustc_ast/src/expand/allocator.rs | 4 - .../rustc_codegen_cranelift/src/allocator.rs | 62 ++------ compiler/rustc_codegen_gcc/src/allocator.rs | 35 +---- compiler/rustc_codegen_gcc/src/lib.rs | 3 +- compiler/rustc_codegen_llvm/src/allocator.rs | 36 +---- compiler/rustc_codegen_llvm/src/lib.rs | 3 +- .../src/back/symbol_export.rs | 4 +- compiler/rustc_codegen_ssa/src/base.rs | 9 +- .../rustc_codegen_ssa/src/traits/backend.rs | 1 - compiler/rustc_metadata/src/creader.rs | 2 + .../rustc_monomorphize/src/partitioning.rs | 1 + library/alloc/src/alloc.rs | 2 +- library/std/src/alloc.rs | 17 ++- src/tools/miri/src/shims/alloc.rs | 33 +---- src/tools/miri/src/shims/foreign_items.rs | 134 +++++++++--------- 15 files changed, 107 insertions(+), 239 deletions(-) diff --git a/compiler/rustc_ast/src/expand/allocator.rs b/compiler/rustc_ast/src/expand/allocator.rs index dd8d5ae624a3c..2c3ec40833bbc 100644 --- a/compiler/rustc_ast/src/expand/allocator.rs +++ b/compiler/rustc_ast/src/expand/allocator.rs @@ -11,10 +11,6 @@ pub fn global_fn_name(base: Symbol) -> String { format!("__rust_{base}") } -pub fn default_fn_name(base: Symbol) -> String { - format!("__rdl_{base}") -} - pub fn alloc_error_handler_name(alloc_error_handler_kind: AllocatorKind) -> &'static str { match alloc_error_handler_kind { AllocatorKind::Global => "__rg_oom", diff --git a/compiler/rustc_codegen_cranelift/src/allocator.rs b/compiler/rustc_codegen_cranelift/src/allocator.rs index 5e33b9d606fa5..8ab99edabfcd9 100644 --- a/compiler/rustc_codegen_cranelift/src/allocator.rs +++ b/compiler/rustc_codegen_cranelift/src/allocator.rs @@ -2,72 +2,34 @@ // Adapted from rustc use rustc_ast::expand::allocator::{ - ALLOCATOR_METHODS, AllocatorKind, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE, - alloc_error_handler_name, default_fn_name, global_fn_name, + AllocatorKind, NO_ALLOC_SHIM_IS_UNSTABLE, alloc_error_handler_name, }; -use rustc_codegen_ssa::base::allocator_kind_for_codegen; +use rustc_codegen_ssa::base::needs_allocator_shim; use rustc_session::config::OomStrategy; use crate::prelude::*; /// Returns whether an allocator shim was created pub(crate) fn codegen(tcx: TyCtxt<'_>, module: &mut dyn Module) -> bool { - let Some(kind) = allocator_kind_for_codegen(tcx) else { return false }; - codegen_inner( - module, - kind, - tcx.alloc_error_handler_kind(()).unwrap(), - tcx.sess.opts.unstable_opts.oom, - ); - true + if needs_allocator_shim(tcx) { + codegen_inner( + module, + tcx.alloc_error_handler_kind(()).unwrap(), + tcx.sess.opts.unstable_opts.oom, + ); + true + } else { + false + } } fn codegen_inner( module: &mut dyn Module, - kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind, oom_strategy: OomStrategy, ) { let usize_ty = module.target_config().pointer_type(); - if kind == AllocatorKind::Default { - for method in ALLOCATOR_METHODS { - let mut arg_tys = Vec::with_capacity(method.inputs.len()); - for input in method.inputs.iter() { - match input.ty { - AllocatorTy::Layout => { - arg_tys.push(usize_ty); // size - arg_tys.push(usize_ty); // align - } - AllocatorTy::Ptr => arg_tys.push(usize_ty), - AllocatorTy::Usize => arg_tys.push(usize_ty), - - AllocatorTy::ResultPtr | AllocatorTy::Unit => panic!("invalid allocator arg"), - } - } - let output = match method.output { - AllocatorTy::ResultPtr => Some(usize_ty), - AllocatorTy::Unit => None, - - AllocatorTy::Layout | AllocatorTy::Usize | AllocatorTy::Ptr => { - panic!("invalid allocator output") - } - }; - - let sig = Signature { - call_conv: module.target_config().default_call_conv, - params: arg_tys.iter().cloned().map(AbiParam::new).collect(), - returns: output.into_iter().map(AbiParam::new).collect(), - }; - crate::common::create_wrapper_function( - module, - sig, - &global_fn_name(method.name), - &default_fn_name(method.name), - ); - } - } - let sig = Signature { call_conv: module.target_config().default_call_conv, params: vec![AbiParam::new(usize_ty), AbiParam::new(usize_ty)], diff --git a/compiler/rustc_codegen_gcc/src/allocator.rs b/compiler/rustc_codegen_gcc/src/allocator.rs index 416f3231a13c4..54770476b4c5b 100644 --- a/compiler/rustc_codegen_gcc/src/allocator.rs +++ b/compiler/rustc_codegen_gcc/src/allocator.rs @@ -2,8 +2,7 @@ use gccjit::{Context, FunctionType, GlobalKind, ToRValue, Type}; #[cfg(feature = "master")] use gccjit::{FnAttribute, VarAttribute}; use rustc_ast::expand::allocator::{ - ALLOCATOR_METHODS, AllocatorKind, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE, - alloc_error_handler_name, default_fn_name, global_fn_name, + AllocatorKind, NO_ALLOC_SHIM_IS_UNSTABLE, alloc_error_handler_name, }; use rustc_middle::bug; use rustc_middle::ty::TyCtxt; @@ -17,7 +16,6 @@ pub(crate) unsafe fn codegen( tcx: TyCtxt<'_>, mods: &mut GccContext, _module_name: &str, - kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind, ) { let context = &mods.context; @@ -28,37 +26,6 @@ pub(crate) unsafe fn codegen( tws => bug!("Unsupported target word size for int: {}", tws), }; let i8 = context.new_type::(); - let i8p = i8.make_pointer(); - - if kind == AllocatorKind::Default { - for method in ALLOCATOR_METHODS { - let mut types = Vec::with_capacity(method.inputs.len()); - for input in method.inputs.iter() { - match input.ty { - AllocatorTy::Layout => { - types.push(usize); - types.push(usize); - } - AllocatorTy::Ptr => types.push(i8p), - AllocatorTy::Usize => types.push(usize), - - AllocatorTy::ResultPtr | AllocatorTy::Unit => panic!("invalid allocator arg"), - } - } - let output = match method.output { - AllocatorTy::ResultPtr => Some(i8p), - AllocatorTy::Unit => None, - - AllocatorTy::Layout | AllocatorTy::Usize | AllocatorTy::Ptr => { - panic!("invalid allocator output") - } - }; - let from_name = global_fn_name(method.name); - let to_name = default_fn_name(method.name); - - create_wrapper_function(tcx, context, &from_name, &to_name, &types, output); - } - } // FIXME(bjorn3): Add noreturn attribute create_wrapper_function( diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index ce88ac390216a..fbaf70575495f 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -290,7 +290,6 @@ impl ExtraBackendMethods for GccCodegenBackend { &self, tcx: TyCtxt<'_>, module_name: &str, - kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind, ) -> Self::Module { let mut mods = GccContext { @@ -301,7 +300,7 @@ impl ExtraBackendMethods for GccCodegenBackend { }; unsafe { - allocator::codegen(tcx, &mut mods, module_name, kind, alloc_error_handler_kind); + allocator::codegen(tcx, &mut mods, module_name, alloc_error_handler_kind); } mods } diff --git a/compiler/rustc_codegen_llvm/src/allocator.rs b/compiler/rustc_codegen_llvm/src/allocator.rs index 149ded28356b8..7353084c9b849 100644 --- a/compiler/rustc_codegen_llvm/src/allocator.rs +++ b/compiler/rustc_codegen_llvm/src/allocator.rs @@ -1,7 +1,6 @@ use libc::c_uint; use rustc_ast::expand::allocator::{ - ALLOCATOR_METHODS, AllocatorKind, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE, - alloc_error_handler_name, default_fn_name, global_fn_name, + AllocatorKind, NO_ALLOC_SHIM_IS_UNSTABLE, alloc_error_handler_name, }; use rustc_middle::bug; use rustc_middle::ty::TyCtxt; @@ -15,7 +14,6 @@ pub(crate) unsafe fn codegen( tcx: TyCtxt<'_>, module_llvm: &mut ModuleLlvm, module_name: &str, - kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind, ) { let llcx = &*module_llvm.llcx; @@ -29,38 +27,6 @@ pub(crate) unsafe fn codegen( } }; let i8 = unsafe { llvm::LLVMInt8TypeInContext(llcx) }; - let i8p = unsafe { llvm::LLVMPointerTypeInContext(llcx, 0) }; - - if kind == AllocatorKind::Default { - for method in ALLOCATOR_METHODS { - let mut args = Vec::with_capacity(method.inputs.len()); - for input in method.inputs.iter() { - match input.ty { - AllocatorTy::Layout => { - args.push(usize); // size - args.push(usize); // align - } - AllocatorTy::Ptr => args.push(i8p), - AllocatorTy::Usize => args.push(usize), - - AllocatorTy::ResultPtr | AllocatorTy::Unit => panic!("invalid allocator arg"), - } - } - let output = match method.output { - AllocatorTy::ResultPtr => Some(i8p), - AllocatorTy::Unit => None, - - AllocatorTy::Layout | AllocatorTy::Usize | AllocatorTy::Ptr => { - panic!("invalid allocator output") - } - }; - - let from_name = global_fn_name(method.name); - let to_name = default_fn_name(method.name); - - create_wrapper_function(tcx, llcx, llmod, &from_name, &to_name, &args, output, false); - } - } // rust alloc error handler create_wrapper_function( diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 14346795fda62..8bcd871f31c3c 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -112,12 +112,11 @@ impl ExtraBackendMethods for LlvmCodegenBackend { &self, tcx: TyCtxt<'tcx>, module_name: &str, - kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind, ) -> ModuleLlvm { let mut module_llvm = ModuleLlvm::new_metadata(tcx, module_name); unsafe { - allocator::codegen(tcx, &mut module_llvm, module_name, kind, alloc_error_handler_kind); + allocator::codegen(tcx, &mut module_llvm, module_name, alloc_error_handler_kind); } module_llvm } diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index f8f7bb2dbc69b..6e56d56b90533 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -16,7 +16,7 @@ use rustc_session::config::{CrateType, OomStrategy}; use rustc_target::spec::{SanitizerSet, TlsModel}; use tracing::debug; -use crate::base::allocator_kind_for_codegen; +use crate::base::needs_allocator_shim; fn threshold(tcx: TyCtxt<'_>) -> SymbolExportLevel { crates_export_threshold(tcx.crate_types()) @@ -206,7 +206,7 @@ fn exported_symbols_provider_local( } // Mark allocator shim symbols as exported only if they were generated. - if allocator_kind_for_codegen(tcx).is_some() { + if needs_allocator_shim(tcx) { for symbol_name in ALLOCATOR_METHODS .iter() .map(|method| format!("__rust_{}", method.name)) diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index d9fbf539fd3d5..427fbd5e535e3 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -5,7 +5,7 @@ use std::time::{Duration, Instant}; use itertools::Itertools; use rustc_abi::FIRST_VARIANT; -use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, AllocatorKind, global_fn_name}; +use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, global_fn_name}; use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; use rustc_data_structures::profiling::{get_resident_set_size, print_time_passes_entry}; use rustc_data_structures::sync::par_map; @@ -583,7 +583,7 @@ pub fn collect_debugger_visualizers_transitive( /// Decide allocator kind to codegen. If `Some(_)` this will be the same as /// `tcx.allocator_kind`, but it may be `None` in more cases (e.g. if using /// allocator definitions from a dylib dependency). -pub fn allocator_kind_for_codegen(tcx: TyCtxt<'_>) -> Option { +pub fn needs_allocator_shim(tcx: TyCtxt<'_>) -> bool { // If the crate doesn't have an `allocator_kind` set then there's definitely // no shim to generate. Otherwise we also check our dependency graph for all // our output crate types. If anything there looks like its a `Dynamic` @@ -594,7 +594,7 @@ pub fn allocator_kind_for_codegen(tcx: TyCtxt<'_>) -> Option { use rustc_middle::middle::dependency_format::Linkage; list.iter().any(|&linkage| linkage == Linkage::Dynamic) }); - if any_dynamic_crate { None } else { tcx.allocator_kind(()) } + if any_dynamic_crate { false } else { tcx.allocator_kind(()).is_some() } } pub fn codegen_crate( @@ -670,14 +670,13 @@ pub fn codegen_crate( start_async_codegen(backend.clone(), tcx, target_cpu, metadata, metadata_module); // Codegen an allocator shim, if necessary. - if let Some(kind) = allocator_kind_for_codegen(tcx) { + if needs_allocator_shim(tcx) { let llmod_id = cgu_name_builder.build_cgu_name(LOCAL_CRATE, &["crate"], Some("allocator")).to_string(); let module_llvm = tcx.sess.time("write_allocator_module", || { backend.codegen_allocator( tcx, &llmod_id, - kind, // If allocator_kind is Some then alloc_error_handler_kind must // also be Some. tcx.alloc_error_handler_kind(()).unwrap(), diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index ebcf118b90380..1d12172153821 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -104,7 +104,6 @@ pub trait ExtraBackendMethods: &self, tcx: TyCtxt<'tcx>, module_name: &str, - kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind, ) -> Self::Module; diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 6bad831279005..a7a0b54f80c08 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -1253,6 +1253,8 @@ fn global_allocator_spans(krate: &ast::Crate) -> Vec { fn visit_item(&mut self, item: &'ast ast::Item) { if item.ident.name == self.name && attr::contains_name(&item.attrs, sym::rustc_std_internal_symbol) + // Ignore the default allocator in libstd with weak linkage + && attr::find_by_name(&item.attrs, sym::linkage).is_none() { self.spans.push(item.span); } diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 3ef061195da50..457113c326351 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -899,6 +899,7 @@ fn mono_item_visibility<'tcx>( // Removal of these functions can't be done by LLVM but rather must be // done by the linker as it's a non-local decision. // + // 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 diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index e9b7f9856677c..8d46ddd9e0edd 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -14,7 +14,7 @@ extern "Rust" { // These are the magic symbols to call the global allocator. rustc generates // them to call `__rg_alloc` etc. if there is a `#[global_allocator]` attribute // (the code expanding that attribute macro generates those functions), or to call - // the default implementations in std (`__rdl_alloc` etc. in `library/std/src/alloc.rs`) + // the default implementations in std (weak symbols in `library/std/src/alloc.rs`) // otherwise. // The rustc fork of LLVM 14 and earlier also special-cases these function names to be able to optimize them // like `malloc`, `realloc`, and `free`, respectively. diff --git a/library/std/src/alloc.rs b/library/std/src/alloc.rs index 5d51d6a0c78a8..24b982c3e964b 100644 --- a/library/std/src/alloc.rs +++ b/library/std/src/alloc.rs @@ -382,9 +382,8 @@ pub fn rust_oom(layout: Layout) -> ! { #[unstable(feature = "alloc_internals", issue = "none")] pub mod __default_lib_allocator { use super::{GlobalAlloc, Layout, System}; - // These magic symbol names are used as a fallback for implementing the - // `__rust_alloc` etc symbols (see `src/liballoc/alloc.rs`) when there is - // no `#[global_allocator]` attribute. + // 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 @@ -393,7 +392,8 @@ pub mod __default_lib_allocator { // ABI #[rustc_std_internal_symbol] - pub unsafe extern "C" fn __rdl_alloc(size: usize, align: usize) -> *mut u8 { + #[linkage = "weak"] + pub unsafe extern "Rust" fn __rust_alloc(size: usize, align: usize) -> *mut u8 { // SAFETY: see the guarantees expected by `Layout::from_size_align` and // `GlobalAlloc::alloc`. unsafe { @@ -403,14 +403,16 @@ pub mod __default_lib_allocator { } #[rustc_std_internal_symbol] - pub unsafe extern "C" fn __rdl_dealloc(ptr: *mut u8, size: usize, align: usize) { + #[linkage = "weak"] + pub unsafe extern "Rust" fn __rust_dealloc(ptr: *mut u8, size: usize, align: usize) { // SAFETY: see the guarantees expected by `Layout::from_size_align` and // `GlobalAlloc::dealloc`. unsafe { System.dealloc(ptr, Layout::from_size_align_unchecked(size, align)) } } #[rustc_std_internal_symbol] - pub unsafe extern "C" fn __rdl_realloc( + #[linkage = "weak"] + pub unsafe extern "Rust" fn __rust_realloc( ptr: *mut u8, old_size: usize, align: usize, @@ -425,7 +427,8 @@ pub mod __default_lib_allocator { } #[rustc_std_internal_symbol] - pub unsafe extern "C" fn __rdl_alloc_zeroed(size: usize, align: usize) -> *mut u8 { + #[linkage = "weak"] + pub unsafe extern "Rust" fn __rust_alloc_zeroed(size: usize, align: usize) -> *mut u8 { // SAFETY: see the guarantees expected by `Layout::from_size_align` and // `GlobalAlloc::alloc_zeroed`. unsafe { diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index 323b95d5f5f23..d3eccf7c8a725 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -1,5 +1,4 @@ use rustc_abi::{Align, Size}; -use rustc_ast::expand::allocator::AllocatorKind; use crate::*; @@ -51,31 +50,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Emulates calling the internal __rust_* allocator functions - fn emulate_allocator( - &mut self, - default: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx>, - ) -> InterpResult<'tcx, EmulateItemResult> { - let this = self.eval_context_mut(); - - let Some(allocator_kind) = this.tcx.allocator_kind(()) else { - // in real code, this symbol does not exist without an allocator - return interp_ok(EmulateItemResult::NotSupported); - }; - - match allocator_kind { - AllocatorKind::Global => { - // 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) - } - AllocatorKind::Default => { - default(this)?; - interp_ok(EmulateItemResult::NeedsReturn) - } - } + 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, init: AllocInit) -> InterpResult<'tcx, Pointer> { diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 97bfb04f1f471..66e7df482fadc 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -9,6 +9,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::CrateNum; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::interpret::AllocInit; +use rustc_middle::mir::mono::Linkage; use rustc_middle::ty::Ty; use rustc_middle::{mir, ty}; use rustc_span::Symbol; @@ -134,7 +135,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { // Find it if it was not cached. - let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum)> = None; + let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum, bool)> = None; helpers::iter_exported_symbols(tcx, |cnum, def_id| { let attrs = tcx.codegen_fn_attrs(def_id); let symbol_name = if let Some(export_name) = attrs.export_name { @@ -145,39 +146,71 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Skip over items without an explicitly defined symbol name. return interp_ok(()); }; + let is_weak = + attrs.linkage.map_or(false, |linkage| linkage == Linkage::WeakAny); if symbol_name == link_name { - if let Some((original_instance, original_cnum)) = instance_and_crate { - // Make sure we are consistent wrt what is 'first' and 'second'. - let original_span = tcx.def_span(original_instance.def_id()).data(); - let span = tcx.def_span(def_id).data(); - if original_span < span { - throw_machine_stop!(TerminationInfo::MultipleSymbolDefinitions { - link_name, - first: original_span, - first_crate: tcx.crate_name(original_cnum), - second: span, - second_crate: tcx.crate_name(cnum), - }); - } else { - throw_machine_stop!(TerminationInfo::MultipleSymbolDefinitions { - link_name, - first: span, - first_crate: tcx.crate_name(cnum), - second: original_span, - second_crate: tcx.crate_name(original_cnum), - }); + if let Some((original_instance, original_cnum, original_is_weak)) = + instance_and_crate + { + match (is_weak, original_is_weak) { + (false, true) => { + // Original definition is a weak definition. Override it. + + instance_and_crate = + Some((ty::Instance::mono(tcx, def_id), cnum, is_weak)); + } + (true, false) => { + // Current definition is a weak definition. Keep the original one. + } + (true, true) | (false, false) => { + // Either both definitions are non-weak or both are weak. In + // either case return an error. For weak definitions we error + // because it is undefined which definition would have been + // picked by the linker. + + // Make sure we are consistent wrt what is 'first' and 'second'. + let original_span = + tcx.def_span(original_instance.def_id()).data(); + let span = tcx.def_span(def_id).data(); + if original_span < span { + throw_machine_stop!( + TerminationInfo::MultipleSymbolDefinitions { + link_name, + first: original_span, + first_crate: tcx.crate_name(original_cnum), + second: span, + second_crate: tcx.crate_name(cnum), + } + ); + } else { + throw_machine_stop!( + TerminationInfo::MultipleSymbolDefinitions { + link_name, + first: span, + first_crate: tcx.crate_name(cnum), + second: original_span, + second_crate: tcx.crate_name(original_cnum), + } + ); + } + } } + } else { + instance_and_crate = + Some((ty::Instance::mono(tcx, def_id), cnum, is_weak)); } - if !matches!(tcx.def_kind(def_id), DefKind::Fn | DefKind::AssocFn) { - throw_ub_format!( - "attempt to call an exported symbol that is not defined as a function" - ); - } - instance_and_crate = Some((ty::Instance::mono(tcx, def_id), cnum)); } interp_ok(()) })?; + if let Some((instance, _, _)) = instance_and_crate { + if !matches!(tcx.def_kind(instance.def_id()), DefKind::Fn | DefKind::AssocFn) { + throw_ub_format!( + "attempt to call an exported symbol that is not defined as a function" + ); + } + } + e.insert(instance_and_crate.map(|ic| ic.0)) } }; @@ -516,7 +549,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { }; match link_name.as_str() { - "__rust_alloc" => return this.emulate_allocator(default), + "__rust_alloc" => return this.emulate_allocator(), "miri_alloc" => { default(this)?; return interp_ok(EmulateItemResult::NeedsReturn); @@ -525,23 +558,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } } "__rust_alloc_zeroed" => { - return this.emulate_allocator(|this| { - // See the comment for `__rust_alloc` why `check_shim` is only called in the - // default case. - let [size, align] = this.check_shim(abi, Conv::Rust, link_name, args)?; - let size = this.read_target_usize(size)?; - let align = this.read_target_usize(align)?; - - this.check_rustc_alloc_request(size, align)?; - - let ptr = this.allocate_ptr( - Size::from_bytes(size), - Align::from_bytes(align).unwrap(), - MiriMemoryKind::Rust.into(), - AllocInit::Zero, - )?; - this.write_pointer(ptr, dest) - }); + return this.emulate_allocator(); } "__rust_dealloc" | "miri_dealloc" => { let default = |ecx: &mut MiriInterpCx<'tcx>| { @@ -569,7 +586,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { match link_name.as_str() { "__rust_dealloc" => { - return this.emulate_allocator(default); + return this.emulate_allocator(); } "miri_dealloc" => { default(this)?; @@ -579,30 +596,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } } "__rust_realloc" => { - return this.emulate_allocator(|this| { - // See the comment for `__rust_alloc` why `check_shim` is only called in the - // default case. - let [ptr, old_size, align, new_size] = - this.check_shim(abi, Conv::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)?; - let new_size = this.read_target_usize(new_size)?; - // No need to check old_size; we anyway check that they match the allocation. - - this.check_rustc_alloc_request(new_size, align)?; - - let align = Align::from_bytes(align).unwrap(); - let new_ptr = this.reallocate_ptr( - ptr, - Some((Size::from_bytes(old_size), align)), - Size::from_bytes(new_size), - align, - MiriMemoryKind::Rust.into(), - AllocInit::Uninit, - )?; - this.write_pointer(new_ptr, dest) - }); + return this.emulate_allocator(); } // C memory handling functions From 860328f63975d199290df242748f9717a68e2cd7 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 2/7] 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 | 93 +++++-------------- 5 files changed, 30 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index 6e56d56b90533..fbafab869aba3 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 457113c326351..6c8a6b3eb6b08 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -901,18 +901,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 d3eccf7c8a725..369aa68df89ed 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -49,16 +49,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, init: AllocInit) -> 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 66e7df482fadc..4081587fa4588 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -522,81 +522,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, Conv::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, Conv::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(), - AllocInit::Uninit, - )?; + 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(), + AllocInit::Uninit, + )?; - 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, Conv::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, Conv::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 From ba6f2a6b5041238c6c0c2df43e2a1fdf33c37135 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 10:14:18 +0000 Subject: [PATCH 3/7] Use weak linkage for the alloc error handler too --- compiler/rustc_ast/src/expand/allocator.rs | 7 -- .../src/alloc_error_handler.rs | 5 +- .../rustc_codegen_cranelift/src/allocator.rs | 30 +---- compiler/rustc_codegen_gcc/src/allocator.rs | 100 +-------------- compiler/rustc_codegen_gcc/src/lib.rs | 10 +- compiler/rustc_codegen_llvm/src/allocator.rs | 114 +----------------- compiler/rustc_codegen_llvm/src/lib.rs | 10 +- .../src/back/symbol_export.rs | 17 +-- compiler/rustc_codegen_ssa/src/base.rs | 11 +- .../rustc_codegen_ssa/src/traits/backend.rs | 8 +- compiler/rustc_metadata/src/creader.rs | 14 +-- .../src/rmeta/decoder/cstore_impl.rs | 1 - compiler/rustc_middle/src/query/mod.rs | 4 - library/alloc/src/alloc.rs | 9 +- library/alloc/src/lib.rs | 1 + library/std/src/alloc.rs | 7 +- src/tools/miri/src/shims/foreign_items.rs | 20 --- 17 files changed, 42 insertions(+), 326 deletions(-) diff --git a/compiler/rustc_ast/src/expand/allocator.rs b/compiler/rustc_ast/src/expand/allocator.rs index 2c3ec40833bbc..97cef7d08ac4e 100644 --- a/compiler/rustc_ast/src/expand/allocator.rs +++ b/compiler/rustc_ast/src/expand/allocator.rs @@ -11,13 +11,6 @@ pub fn global_fn_name(base: Symbol) -> String { format!("__rust_{base}") } -pub fn alloc_error_handler_name(alloc_error_handler_kind: AllocatorKind) -> &'static str { - match alloc_error_handler_kind { - AllocatorKind::Global => "__rg_oom", - AllocatorKind::Default => "__rdl_oom", - } -} - pub const NO_ALLOC_SHIM_IS_UNSTABLE: &str = "__rust_no_alloc_shim_is_unstable"; pub enum AllocatorTy { diff --git a/compiler/rustc_builtin_macros/src/alloc_error_handler.rs b/compiler/rustc_builtin_macros/src/alloc_error_handler.rs index cffc497860133..65cddf55083ad 100644 --- a/compiler/rustc_builtin_macros/src/alloc_error_handler.rs +++ b/compiler/rustc_builtin_macros/src/alloc_error_handler.rs @@ -56,7 +56,7 @@ pub(crate) fn expand( } // #[rustc_std_internal_symbol] -// unsafe fn __rg_oom(size: usize, align: usize) -> ! { +// unsafe fn __rust_alloc_error_handler(size: usize, align: usize) -> ! { // handler(core::alloc::Layout::from_size_align_unchecked(size, align)) // } fn generate_handler(cx: &ExtCtxt<'_>, handler: Ident, span: Span, sig_span: Span) -> Stmt { @@ -91,6 +91,7 @@ fn generate_handler(cx: &ExtCtxt<'_>, handler: Ident, span: Span, sig_span: Span let attrs = thin_vec![cx.attr_word(sym::rustc_std_internal_symbol, span)]; - let item = cx.item(span, Ident::from_str_and_span("__rg_oom", span), attrs, kind); + let item = + cx.item(span, Ident::from_str_and_span("__rust_alloc_error_handler", span), attrs, kind); cx.stmt_item(sig_span, item) } diff --git a/compiler/rustc_codegen_cranelift/src/allocator.rs b/compiler/rustc_codegen_cranelift/src/allocator.rs index 8ab99edabfcd9..5805f738d0a1a 100644 --- a/compiler/rustc_codegen_cranelift/src/allocator.rs +++ b/compiler/rustc_codegen_cranelift/src/allocator.rs @@ -1,9 +1,7 @@ //! Allocator shim // Adapted from rustc -use rustc_ast::expand::allocator::{ - AllocatorKind, NO_ALLOC_SHIM_IS_UNSTABLE, alloc_error_handler_name, -}; +use rustc_ast::expand::allocator::NO_ALLOC_SHIM_IS_UNSTABLE; use rustc_codegen_ssa::base::needs_allocator_shim; use rustc_session::config::OomStrategy; @@ -12,36 +10,14 @@ use crate::prelude::*; /// Returns whether an allocator shim was created pub(crate) fn codegen(tcx: TyCtxt<'_>, module: &mut dyn Module) -> bool { if needs_allocator_shim(tcx) { - codegen_inner( - module, - tcx.alloc_error_handler_kind(()).unwrap(), - tcx.sess.opts.unstable_opts.oom, - ); + codegen_inner(module, tcx.sess.opts.unstable_opts.oom); true } else { false } } -fn codegen_inner( - module: &mut dyn Module, - alloc_error_handler_kind: AllocatorKind, - oom_strategy: OomStrategy, -) { - let usize_ty = module.target_config().pointer_type(); - - let sig = Signature { - call_conv: module.target_config().default_call_conv, - params: vec![AbiParam::new(usize_ty), AbiParam::new(usize_ty)], - returns: vec![], - }; - crate::common::create_wrapper_function( - module, - sig, - "__rust_alloc_error_handler", - &alloc_error_handler_name(alloc_error_handler_kind), - ); - +fn codegen_inner(module: &mut dyn Module, oom_strategy: OomStrategy) { let data_id = module.declare_data(OomStrategy::SYMBOL, Linkage::Export, false, false).unwrap(); let mut data = DataDescription::new(); data.set_align(1); diff --git a/compiler/rustc_codegen_gcc/src/allocator.rs b/compiler/rustc_codegen_gcc/src/allocator.rs index 54770476b4c5b..5cf8d27f884e2 100644 --- a/compiler/rustc_codegen_gcc/src/allocator.rs +++ b/compiler/rustc_codegen_gcc/src/allocator.rs @@ -1,10 +1,7 @@ -use gccjit::{Context, FunctionType, GlobalKind, ToRValue, Type}; +use gccjit::GlobalKind; #[cfg(feature = "master")] -use gccjit::{FnAttribute, VarAttribute}; -use rustc_ast::expand::allocator::{ - AllocatorKind, NO_ALLOC_SHIM_IS_UNSTABLE, alloc_error_handler_name, -}; -use rustc_middle::bug; +use gccjit::VarAttribute; +use rustc_ast::expand::allocator::NO_ALLOC_SHIM_IS_UNSTABLE; use rustc_middle::ty::TyCtxt; use rustc_session::config::OomStrategy; @@ -12,31 +9,10 @@ use crate::GccContext; #[cfg(feature = "master")] use crate::base::symbol_visibility_to_gcc; -pub(crate) unsafe fn codegen( - tcx: TyCtxt<'_>, - mods: &mut GccContext, - _module_name: &str, - alloc_error_handler_kind: AllocatorKind, -) { +pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_name: &str) { let context = &mods.context; - let usize = match tcx.sess.target.pointer_width { - 16 => context.new_type::(), - 32 => context.new_type::(), - 64 => context.new_type::(), - tws => bug!("Unsupported target word size for int: {}", tws), - }; let i8 = context.new_type::(); - // FIXME(bjorn3): Add noreturn attribute - create_wrapper_function( - tcx, - context, - "__rust_alloc_error_handler", - alloc_error_handler_name(alloc_error_handler_kind), - &[usize, usize], - None, - ); - let name = OomStrategy::SYMBOL.to_string(); let global = context.new_global(None, GlobalKind::Exported, i8, name); #[cfg(feature = "master")] @@ -56,71 +32,3 @@ pub(crate) unsafe fn codegen( let value = context.new_rvalue_from_int(i8, 0); global.global_set_initializer_rvalue(value); } - -fn create_wrapper_function( - tcx: TyCtxt<'_>, - context: &Context<'_>, - from_name: &str, - to_name: &str, - types: &[Type<'_>], - output: Option>, -) { - let void = context.new_type::<()>(); - - let args: Vec<_> = types - .iter() - .enumerate() - .map(|(index, typ)| context.new_parameter(None, *typ, format!("param{}", index))) - .collect(); - let func = context.new_function( - None, - FunctionType::Exported, - output.unwrap_or(void), - &args, - from_name, - false, - ); - - #[cfg(feature = "master")] - func.add_attribute(FnAttribute::Visibility(symbol_visibility_to_gcc( - tcx.sess.default_visibility(), - ))); - - if tcx.sess.must_emit_unwind_tables() { - // TODO(antoyo): emit unwind tables. - } - - let args: Vec<_> = types - .iter() - .enumerate() - .map(|(index, typ)| context.new_parameter(None, *typ, format!("param{}", index))) - .collect(); - let callee = context.new_function( - None, - FunctionType::Extern, - output.unwrap_or(void), - &args, - to_name, - false, - ); - #[cfg(feature = "master")] - callee.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden)); - - let block = func.new_block("entry"); - - let args = args - .iter() - .enumerate() - .map(|(i, _)| func.get_param(i as i32).to_rvalue()) - .collect::>(); - let ret = context.new_call(None, callee, &args); - //llvm::LLVMSetTailCall(ret, True); - if output.is_some() { - block.end_with_return(None, ret); - } else { - block.end_with_void_return(None); - } - - // TODO(@Commeownist): Check if we need to emit some extra debugging info in certain circumstances - // as described in https://github.com/rust-lang/rust/commit/77a96ed5646f7c3ee8897693decc4626fe380643 -} diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index fbaf70575495f..57b492464cddd 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -93,7 +93,6 @@ use back::lto::{ThinBuffer, ThinData}; use gccjit::{CType, Context, OptimizationLevel}; #[cfg(feature = "master")] use gccjit::{TargetInfo, Version}; -use rustc_ast::expand::allocator::AllocatorKind; use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule}; use rustc_codegen_ssa::back::write::{ @@ -286,12 +285,7 @@ fn new_context<'gcc, 'tcx>(tcx: TyCtxt<'tcx>) -> Context<'gcc> { } impl ExtraBackendMethods for GccCodegenBackend { - fn codegen_allocator( - &self, - tcx: TyCtxt<'_>, - module_name: &str, - alloc_error_handler_kind: AllocatorKind, - ) -> Self::Module { + fn codegen_allocator(&self, tcx: TyCtxt<'_>, module_name: &str) -> Self::Module { let mut mods = GccContext { context: Arc::new(SyncContext::new(new_context(tcx))), relocation_model: tcx.sess.relocation_model(), @@ -300,7 +294,7 @@ impl ExtraBackendMethods for GccCodegenBackend { }; unsafe { - allocator::codegen(tcx, &mut mods, module_name, alloc_error_handler_kind); + allocator::codegen(tcx, &mut mods, module_name); } mods } diff --git a/compiler/rustc_codegen_llvm/src/allocator.rs b/compiler/rustc_codegen_llvm/src/allocator.rs index 7353084c9b849..5d7f7444ec208 100644 --- a/compiler/rustc_codegen_llvm/src/allocator.rs +++ b/compiler/rustc_codegen_llvm/src/allocator.rs @@ -1,45 +1,16 @@ -use libc::c_uint; -use rustc_ast::expand::allocator::{ - AllocatorKind, NO_ALLOC_SHIM_IS_UNSTABLE, alloc_error_handler_name, -}; -use rustc_middle::bug; +use rustc_ast::expand::allocator::NO_ALLOC_SHIM_IS_UNSTABLE; use rustc_middle::ty::TyCtxt; use rustc_session::config::{DebugInfo, OomStrategy}; use crate::common::AsCCharPtr; -use crate::llvm::{self, Context, False, Module, True, Type}; -use crate::{ModuleLlvm, attributes, debuginfo}; +use crate::llvm::{self, False}; +use crate::{ModuleLlvm, debuginfo}; -pub(crate) unsafe fn codegen( - tcx: TyCtxt<'_>, - module_llvm: &mut ModuleLlvm, - module_name: &str, - alloc_error_handler_kind: AllocatorKind, -) { +pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, module_llvm: &mut ModuleLlvm, module_name: &str) { let llcx = &*module_llvm.llcx; let llmod = module_llvm.llmod(); - let usize = unsafe { - match tcx.sess.target.pointer_width { - 16 => llvm::LLVMInt16TypeInContext(llcx), - 32 => llvm::LLVMInt32TypeInContext(llcx), - 64 => llvm::LLVMInt64TypeInContext(llcx), - tws => bug!("Unsupported target word size for int: {}", tws), - } - }; let i8 = unsafe { llvm::LLVMInt8TypeInContext(llcx) }; - // rust alloc error handler - create_wrapper_function( - tcx, - llcx, - llmod, - "__rust_alloc_error_handler", - alloc_error_handler_name(alloc_error_handler_kind), - &[usize, usize], // size, align - None, - true, - ); - unsafe { // __rust_alloc_error_handler_should_panic let name = OomStrategy::SYMBOL; @@ -62,80 +33,3 @@ pub(crate) unsafe fn codegen( dbg_cx.finalize(tcx.sess); } } - -fn create_wrapper_function( - tcx: TyCtxt<'_>, - llcx: &Context, - llmod: &Module, - from_name: &str, - to_name: &str, - args: &[&Type], - output: Option<&Type>, - no_return: bool, -) { - unsafe { - let ty = llvm::LLVMFunctionType( - output.unwrap_or_else(|| llvm::LLVMVoidTypeInContext(llcx)), - args.as_ptr(), - args.len() as c_uint, - False, - ); - let llfn = llvm::LLVMRustGetOrInsertFunction( - llmod, - from_name.as_c_char_ptr(), - from_name.len(), - ty, - ); - let no_return = if no_return { - // -> ! DIFlagNoReturn - let no_return = llvm::AttributeKind::NoReturn.create_attr(llcx); - attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[no_return]); - Some(no_return) - } else { - None - }; - - llvm::set_visibility(llfn, llvm::Visibility::from_generic(tcx.sess.default_visibility())); - - if tcx.sess.must_emit_unwind_tables() { - let uwtable = - attributes::uwtable_attr(llcx, tcx.sess.opts.unstable_opts.use_sync_unwind); - attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[uwtable]); - } - - let callee = - llvm::LLVMRustGetOrInsertFunction(llmod, to_name.as_c_char_ptr(), to_name.len(), ty); - if let Some(no_return) = no_return { - // -> ! DIFlagNoReturn - attributes::apply_to_llfn(callee, llvm::AttributePlace::Function, &[no_return]); - } - llvm::set_visibility(callee, llvm::Visibility::Hidden); - - let llbb = llvm::LLVMAppendBasicBlockInContext(llcx, llfn, c"entry".as_ptr()); - - let llbuilder = llvm::LLVMCreateBuilderInContext(llcx); - llvm::LLVMPositionBuilderAtEnd(llbuilder, llbb); - let args = args - .iter() - .enumerate() - .map(|(i, _)| llvm::LLVMGetParam(llfn, i as c_uint)) - .collect::>(); - let ret = llvm::LLVMBuildCallWithOperandBundles( - llbuilder, - ty, - callee, - args.as_ptr(), - args.len() as c_uint, - [].as_ptr(), - 0 as c_uint, - c"".as_ptr(), - ); - llvm::LLVMSetTailCall(ret, True); - if output.is_some() { - llvm::LLVMBuildRet(llbuilder, ret); - } else { - llvm::LLVMBuildRetVoid(llbuilder); - } - llvm::LLVMDisposeBuilder(llbuilder); - } -} diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 8bcd871f31c3c..7e837096d97e1 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -30,7 +30,6 @@ use back::owned_target_machine::OwnedTargetMachine; use back::write::{create_informational_target_machine, create_target_machine}; use errors::{AutoDiffWithoutLTO, ParseTargetMachineConfig}; pub use llvm_util::target_features_cfg; -use rustc_ast::expand::allocator::AllocatorKind; use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule}; use rustc_codegen_ssa::back::write::{ @@ -108,15 +107,10 @@ impl Drop for TimeTraceProfiler { } impl ExtraBackendMethods for LlvmCodegenBackend { - fn codegen_allocator<'tcx>( - &self, - tcx: TyCtxt<'tcx>, - module_name: &str, - alloc_error_handler_kind: AllocatorKind, - ) -> ModuleLlvm { + fn codegen_allocator<'tcx>(&self, tcx: TyCtxt<'tcx>, module_name: &str) -> ModuleLlvm { let mut module_llvm = ModuleLlvm::new_metadata(tcx, module_name); unsafe { - allocator::codegen(tcx, &mut module_llvm, module_name, alloc_error_handler_kind); + allocator::codegen(tcx, &mut module_llvm, module_name); } module_llvm } diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index fbafab869aba3..b813d56f465e6 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -207,17 +207,12 @@ 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 - ["__rust_alloc_error_handler".to_string(), OomStrategy::SYMBOL.to_string()] - { - let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name)); - - symbols.push((exported_symbol, SymbolExportInfo { - level: SymbolExportLevel::Rust, - kind: SymbolExportKind::Text, - used: false, - })); - } + let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &OomStrategy::SYMBOL)); + symbols.push((exported_symbol, SymbolExportInfo { + level: SymbolExportLevel::Rust, + kind: SymbolExportKind::Text, + used: false, + })); let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, NO_ALLOC_SHIM_IS_UNSTABLE)); diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 427fbd5e535e3..e16e496ec2caf 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -673,15 +673,8 @@ pub fn codegen_crate( if needs_allocator_shim(tcx) { let llmod_id = cgu_name_builder.build_cgu_name(LOCAL_CRATE, &["crate"], Some("allocator")).to_string(); - let module_llvm = tcx.sess.time("write_allocator_module", || { - backend.codegen_allocator( - tcx, - &llmod_id, - // If allocator_kind is Some then alloc_error_handler_kind must - // also be Some. - tcx.alloc_error_handler_kind(()).unwrap(), - ) - }); + let module_llvm = + tcx.sess.time("write_allocator_module", || backend.codegen_allocator(tcx, &llmod_id)); ongoing_codegen.wait_for_signal_to_codegen_item(); ongoing_codegen.check_for_errors(tcx.sess); diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index 1d12172153821..ff5a001fb82e5 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -1,7 +1,6 @@ use std::any::Any; use std::hash::Hash; -use rustc_ast::expand::allocator::AllocatorKind; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::sync::{DynSend, DynSync}; use rustc_metadata::EncodedMetadata; @@ -100,12 +99,7 @@ pub trait CodegenBackend { pub trait ExtraBackendMethods: CodegenBackend + WriteBackendMethods + Sized + Send + Sync + DynSend + DynSync { - fn codegen_allocator<'tcx>( - &self, - tcx: TyCtxt<'tcx>, - module_name: &str, - alloc_error_handler_kind: AllocatorKind, - ) -> Self::Module; + fn codegen_allocator<'tcx>(&self, tcx: TyCtxt<'tcx>, module_name: &str) -> Self::Module; /// This generates the codegen unit and returns it along with /// a `u64` giving an estimate of the unit's processing cost. diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index a7a0b54f80c08..de9b30fde6729 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -8,7 +8,7 @@ use std::time::Duration; use std::{cmp, env, iter}; use proc_macro::bridge::client::ProcMacro; -use rustc_ast::expand::allocator::{AllocatorKind, alloc_error_handler_name, global_fn_name}; +use rustc_ast::expand::allocator::{AllocatorKind, global_fn_name}; use rustc_ast::{self as ast, *}; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::owned_slice::OwnedSlice; @@ -24,8 +24,8 @@ use rustc_index::IndexVec; use rustc_middle::bug; use rustc_middle::ty::{TyCtxt, TyCtxtFeed}; use rustc_session::config::{ - self, CrateType, ExtendedTargetModifierInfo, ExternLocation, OptionsTargetModifiers, - TargetModifier, + self, CrateType, ExtendedTargetModifierInfo, ExternLocation, OomStrategy, + OptionsTargetModifiers, TargetModifier, }; use rustc_session::cstore::{CrateDepKind, CrateSource, ExternCrate, ExternCrateSource}; use rustc_session::lint::{self, BuiltinLintDiag}; @@ -269,10 +269,6 @@ impl CStore { self.allocator_kind } - pub(crate) fn alloc_error_handler_kind(&self) -> Option { - self.alloc_error_handler_kind - } - pub(crate) fn has_global_allocator(&self) -> bool { self.has_global_allocator } @@ -1277,6 +1273,8 @@ fn alloc_error_handler_spans(krate: &ast::Crate) -> Vec { fn visit_item(&mut self, item: &'ast ast::Item) { if item.ident.name == self.name && attr::contains_name(&item.attrs, sym::rustc_std_internal_symbol) + // Ignore the default alloc error handler in libstd with weak linkage + && attr::find_by_name(&item.attrs, sym::linkage).is_none() { self.spans.push(item.span); } @@ -1284,7 +1282,7 @@ fn alloc_error_handler_spans(krate: &ast::Crate) -> Vec { } } - let name = Symbol::intern(alloc_error_handler_name(AllocatorKind::Global)); + let name = Symbol::intern(OomStrategy::SYMBOL); let mut f = Finder { name, spans: Vec::new() }; visit::walk_crate(&mut f, krate); f.spans diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 776b081a4630f..af0aeca497a57 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -435,7 +435,6 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) { provide_cstore_hooks(providers); providers.queries = rustc_middle::query::Providers { allocator_kind: |tcx, ()| CStore::from_tcx(tcx).allocator_kind(), - alloc_error_handler_kind: |tcx, ()| CStore::from_tcx(tcx).alloc_error_handler_kind(), is_private_dep: |_tcx, LocalCrate| false, native_library: |tcx, id| { tcx.native_libraries(id.krate) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d94efe2d7d6f8..3562f343f907d 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2109,10 +2109,6 @@ rustc_queries! { eval_always desc { "getting the allocator kind for the current crate" } } - query alloc_error_handler_kind(_: ()) -> Option { - eval_always - desc { "alloc error handler kind for the current crate" } - } query upvars_mentioned(def_id: DefId) -> Option<&'tcx FxIndexMap> { desc { |tcx| "collecting upvars mentioned in `{}`", tcx.def_path_str(def_id) } diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 8d46ddd9e0edd..958659bb5399e 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -357,8 +357,8 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 { #[cfg(not(no_global_oom_handling))] extern "Rust" { // This is the magic symbol to call the global alloc error handler. rustc generates - // it to call `__rg_oom` if there is a `#[alloc_error_handler]`, or to call the - // default implementations below (`__rdl_oom`) otherwise. + // it if there is a `#[alloc_error_handler]`, or to the weak implementations below + // is called otherwise. fn __rust_alloc_error_handler(size: usize, align: usize) -> !; } @@ -422,10 +422,9 @@ pub use std::alloc::handle_alloc_error; #[allow(unused_attributes)] #[unstable(feature = "alloc_internals", issue = "none")] pub mod __alloc_error_handler { - // called via generated `__rust_alloc_error_handler` if there is no - // `#[alloc_error_handler]`. #[rustc_std_internal_symbol] - pub unsafe fn __rdl_oom(size: usize, _align: usize) -> ! { + #[linkage = "weak"] + pub unsafe extern "Rust" fn __rust_alloc_error_handler(size: usize, _align: usize) -> ! { extern "Rust" { // This symbol is emitted by rustc next to __rust_alloc_error_handler. // Its value depends on the -Zoom={panic,abort} compiler option. diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 0bb7c432cc35f..97b822b34135c 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -126,6 +126,7 @@ #![feature(iter_next_chunk)] #![feature(layout_for_ptr)] #![feature(legacy_receiver_trait)] +#![feature(linkage)] #![feature(local_waker)] #![feature(maybe_uninit_slice)] #![feature(maybe_uninit_uninit_array_transpose)] diff --git a/library/std/src/alloc.rs b/library/std/src/alloc.rs index 300f530ca764b..849ef537aa4ef 100644 --- a/library/std/src/alloc.rs +++ b/library/std/src/alloc.rs @@ -357,9 +357,10 @@ fn default_alloc_error_hook(layout: Layout) { // This is the default path taken on OOM, and the only path taken on stable with std. // Crucially, it does *not* call any user-defined code, and therefore users do not have to // worry about allocation failure causing reentrancy issues. That makes it different from - // the default `__rdl_oom` defined in alloc (i.e., the default alloc error handler that is - // called when there is no `#[alloc_error_handler]`), which triggers a regular panic and - // thus can invoke a user-defined panic hook, executing arbitrary user-defined code. + // the default `__rust_alloc_error_handler` defined in alloc (i.e., the default alloc error + // handler that is called when there is no `#[alloc_error_handler]`), which triggers a + // regular panic and thus can invoke a user-defined panic hook, executing arbitrary + // user-defined code. rtprintpanic!("memory allocation of {} bytes failed\n", layout.size()); } } diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 4081587fa4588..501dfd07ee6ca 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -4,7 +4,6 @@ use std::path::Path; use rustc_abi::{Align, AlignFromBytesError, Size}; use rustc_apfloat::Float; -use rustc_ast::expand::allocator::alloc_error_handler_name; use rustc_hir::def::DefKind; use rustc_hir::def_id::CrateNum; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; @@ -50,25 +49,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Option<(&'tcx mir::Body<'tcx>, ty::Instance<'tcx>)>> { let this = self.eval_context_mut(); - // Some shims forward to other MIR bodies. - match link_name.as_str() { - "__rust_alloc_error_handler" => { - // Forward to the right symbol that implements this function. - let Some(handler_kind) = this.tcx.alloc_error_handler_kind(()) else { - // in real code, this symbol does not exist without an allocator - throw_unsup_format!( - "`__rust_alloc_error_handler` cannot be called when no alloc error handler is set" - ); - }; - let name = alloc_error_handler_name(handler_kind); - let handler = this - .lookup_exported_symbol(Symbol::intern(name))? - .expect("missing alloc error handler symbol"); - return interp_ok(Some(handler)); - } - _ => {} - } - // The rest either implements the logic, or falls back to `lookup_exported_symbol`. match this.emulate_foreign_item_inner(link_name, abi, args, dest)? { EmulateItemResult::NeedsReturn => { From c7cf080a35191f86d6abb7bd387f6a6c43c6a239 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 11:02:11 +0000 Subject: [PATCH 4/7] Provide a default for __rust_alloc_error_handler_should_panic --- library/alloc/src/alloc.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 958659bb5399e..54d9b2730ef97 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -357,7 +357,7 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 { #[cfg(not(no_global_oom_handling))] extern "Rust" { // This is the magic symbol to call the global alloc error handler. rustc generates - // it if there is a `#[alloc_error_handler]`, or to the weak implementations below + // it if there is an `#[alloc_error_handler]`, or to the weak implementations below // is called otherwise. fn __rust_alloc_error_handler(size: usize, align: usize) -> !; } @@ -425,13 +425,16 @@ pub mod __alloc_error_handler { #[rustc_std_internal_symbol] #[linkage = "weak"] pub unsafe extern "Rust" fn __rust_alloc_error_handler(size: usize, _align: usize) -> ! { - extern "Rust" { - // This symbol is emitted by rustc next to __rust_alloc_error_handler. - // Its value depends on the -Zoom={panic,abort} compiler option. - static __rust_alloc_error_handler_should_panic: u8; - } - - if unsafe { __rust_alloc_error_handler_should_panic != 0 } { + // This symbol is normally overwritten by rustc next to __rust_alloc_error_handler. + // However when skipping the allocator handler shim the value here is used which + // corresponds to -Zoom=abort. + // Its value depends on the -Zoom={panic,abort} compiler option. + #[rustc_std_internal_symbol] + #[linkage = "weak"] + #[allow(non_upper_case_globals)] + static __rust_alloc_error_handler_should_panic: u8 = 0; + + if __rust_alloc_error_handler_should_panic != 0 { panic!("memory allocation of {size} bytes failed") } else { core::panicking::panic_nounwind_fmt( From 4abf2501cc067cb24b6dfadac64648c0e4c65341 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 10:42:40 +0000 Subject: [PATCH 5/7] Revert some miri changes --- library/alloc/src/alloc.rs | 10 +- src/tools/miri/src/helpers.rs | 15 ++ src/tools/miri/src/shims/alloc.rs | 29 ++++ src/tools/miri/src/shims/foreign_items.rs | 140 +++++++++++++++--- .../fail/alloc/alloc_error_handler.stderr | 2 +- .../alloc/alloc_error_handler_custom.stderr | 2 +- .../alloc/alloc_error_handler_no_std.stderr | 2 +- 7 files changed, 169 insertions(+), 31 deletions(-) diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 54d9b2730ef97..d3d329ba51ff8 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -12,12 +12,12 @@ use core::ptr::{self, NonNull}; extern "Rust" { // These are the magic symbols to call the global allocator. rustc generates - // them to call `__rg_alloc` etc. if there is a `#[global_allocator]` attribute - // (the code expanding that attribute macro generates those functions), or to call + // them to call `GLOBAL.alloc` etc. if there is a `#[global_allocator]` attribute + // (the code expanding that attribute macro generates those functions), or if not // the default implementations in std (weak symbols in `library/std/src/alloc.rs`) - // otherwise. - // The rustc fork of LLVM 14 and earlier also special-cases these function names to be able to optimize them - // like `malloc`, `realloc`, and `free`, respectively. + // is used otherwise. + // The rustc fork of LLVM 14 and earlier also special-cases these function names to + // be able to optimize them like `malloc`, `realloc`, and `free`, respectively. #[rustc_allocator] #[rustc_nounwind] fn __rust_alloc(size: usize, align: usize) -> *mut u8; diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index a26f12cdfb1e2..1156e4b3d4faa 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -980,6 +980,21 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return interp_ok(()); } + if ["__rust_alloc", "__rust_alloc_zeroed", "__rust_realloc", "__rust_dealloc"] + .contains(&link_name.as_str()) + { + let attrs = self.eval_context_ref().tcx.codegen_fn_attrs(instance.def_id()); + if attrs + .linkage + .map_or(false, |linkage| linkage == rustc_middle::mir::mono::Linkage::WeakAny) + && attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) + { + // We intentionally intercept the allocator methods even though libstd provides + // default implementations. + return interp_ok(()); + } + } + throw_machine_stop!(TerminationInfo::SymbolShimClashing { link_name, span: body.span.data(), diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index 369aa68df89ed..323b95d5f5f23 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -1,4 +1,5 @@ use rustc_abi::{Align, Size}; +use rustc_ast::expand::allocator::AllocatorKind; use crate::*; @@ -49,6 +50,34 @@ 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, + default: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx>, + ) -> InterpResult<'tcx, EmulateItemResult> { + let this = self.eval_context_mut(); + + let Some(allocator_kind) = this.tcx.allocator_kind(()) else { + // in real code, this symbol does not exist without an allocator + return interp_ok(EmulateItemResult::NotSupported); + }; + + match allocator_kind { + AllocatorKind::Global => { + // 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) + } + AllocatorKind::Default => { + default(this)?; + interp_ok(EmulateItemResult::NeedsReturn) + } + } + } + fn malloc(&mut self, size: u64, init: AllocInit) -> 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 501dfd07ee6ca..33389bb92f5b8 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -1,5 +1,6 @@ use std::collections::hash_map::Entry; use std::io::Write; +use std::iter; use std::path::Path; use rustc_abi::{Align, AlignFromBytesError, Size}; @@ -502,34 +503,127 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } // Rust allocation - "miri_alloc" => { - let [size, align] = this.check_shim(abi, Conv::Rust, link_name, args)?; - let size = this.read_target_usize(size)?; - let align = this.read_target_usize(align)?; + "__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, Conv::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!(), + }; - this.check_rustc_alloc_request(size, align)?; + let ptr = ecx.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + memory_kind.into(), + AllocInit::Uninit, + )?; - let ptr = this.allocate_ptr( - Size::from_bytes(size), - Align::from_bytes(align).unwrap(), - MiriMemoryKind::Miri.into(), - AllocInit::Uninit, - )?; + ecx.write_pointer(ptr, dest) + }; - this.write_pointer(ptr, dest)?; + match link_name.as_str() { + "__rust_alloc" => return this.emulate_allocator(default), + "miri_alloc" => { + default(this)?; + return interp_ok(EmulateItemResult::NeedsReturn); + } + _ => unreachable!(), + } } - "miri_dealloc" => { - let [ptr, old_size, align] = this.check_shim(abi, Conv::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)?; + "__rust_alloc_zeroed" => { + return this.emulate_allocator(|this| { + // See the comment for `__rust_alloc` why `check_shim` is only called in the + // default case. + let [size, align] = this.check_shim(abi, Conv::Rust, link_name, args)?; + let size = this.read_target_usize(size)?; + let align = this.read_target_usize(align)?; + + this.check_rustc_alloc_request(size, align)?; + + let ptr = this.allocate_ptr( + Size::from_bytes(size), + Align::from_bytes(align).unwrap(), + MiriMemoryKind::Rust.into(), + AllocInit::Zero, + )?; - // 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(), - )?; + // We just allocated this, the access is definitely in-bounds. + this.write_bytes_ptr( + ptr.into(), + iter::repeat(0u8).take(usize::try_from(size).unwrap()), + ) + .unwrap(); + this.write_pointer(ptr, dest) + }); + } + "__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, Conv::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(), + ) + }; + + match link_name.as_str() { + "__rust_dealloc" => { + return this.emulate_allocator(default); + } + "miri_dealloc" => { + default(this)?; + return interp_ok(EmulateItemResult::NeedsReturn); + } + _ => unreachable!(), + } + } + "__rust_realloc" => { + return this.emulate_allocator(|this| { + // See the comment for `__rust_alloc` why `check_shim` is only called in the + // default case. + let [ptr, old_size, align, new_size] = + this.check_shim(abi, Conv::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)?; + let new_size = this.read_target_usize(new_size)?; + // No need to check old_size; we anyway check that they match the allocation. + + this.check_rustc_alloc_request(new_size, align)?; + + let align = Align::from_bytes(align).unwrap(); + let new_ptr = this.reallocate_ptr( + ptr, + Some((Size::from_bytes(old_size), align)), + Size::from_bytes(new_size), + align, + MiriMemoryKind::Rust.into(), + AllocInit::Uninit, + )?; + this.write_pointer(new_ptr, dest) + }); } // C memory handling functions diff --git a/src/tools/miri/tests/fail/alloc/alloc_error_handler.stderr b/src/tools/miri/tests/fail/alloc/alloc_error_handler.stderr index 3642f3f28ca2a..79a357cd541d1 100644 --- a/src/tools/miri/tests/fail/alloc/alloc_error_handler.stderr +++ b/src/tools/miri/tests/fail/alloc/alloc_error_handler.stderr @@ -9,7 +9,7 @@ LL | ABORT(); = note: inside `std::sys::pal::PLATFORM::abort_internal` at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC = note: inside `std::process::abort` at RUSTLIB/std/src/process.rs:LL:CC = note: inside `std::alloc::rust_oom` at RUSTLIB/std/src/alloc.rs:LL:CC - = note: inside `std::alloc::_::__rg_oom` at RUSTLIB/std/src/alloc.rs:LL:CC + = note: inside `std::alloc::_::__rust_alloc_error_handler` at RUSTLIB/std/src/alloc.rs:LL:CC = note: inside `std::alloc::handle_alloc_error::rt_error` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `std::alloc::handle_alloc_error` at RUSTLIB/alloc/src/alloc.rs:LL:CC note: inside `main` diff --git a/src/tools/miri/tests/fail/alloc/alloc_error_handler_custom.stderr b/src/tools/miri/tests/fail/alloc/alloc_error_handler_custom.stderr index 1a9e757433994..ce39bac3b4edd 100644 --- a/src/tools/miri/tests/fail/alloc/alloc_error_handler_custom.stderr +++ b/src/tools/miri/tests/fail/alloc/alloc_error_handler_custom.stderr @@ -7,7 +7,7 @@ LL | core::intrinsics::abort(); | = note: BACKTRACE: = note: inside `alloc_error_handler` at tests/fail/alloc/alloc_error_handler_custom.rs:LL:CC -note: inside `_::__rg_oom` +note: inside `_::__rust_alloc_error_handler` --> tests/fail/alloc/alloc_error_handler_custom.rs:LL:CC | LL | #[alloc_error_handler] diff --git a/src/tools/miri/tests/fail/alloc/alloc_error_handler_no_std.stderr b/src/tools/miri/tests/fail/alloc/alloc_error_handler_no_std.stderr index 6b4266b9a8b5d..9d7ab7fcde312 100644 --- a/src/tools/miri/tests/fail/alloc/alloc_error_handler_no_std.stderr +++ b/src/tools/miri/tests/fail/alloc/alloc_error_handler_no_std.stderr @@ -9,7 +9,7 @@ LL | core::intrinsics::abort(); | = note: BACKTRACE: = note: inside `panic_handler` at tests/fail/alloc/alloc_error_handler_no_std.rs:LL:CC - = note: inside `alloc::alloc::__alloc_error_handler::__rdl_oom` at RUSTLIB/alloc/src/alloc.rs:LL:CC + = note: inside `alloc::alloc::__alloc_error_handler::__rust_alloc_error_handler` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `alloc::alloc::handle_alloc_error::rt_error` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `alloc::alloc::handle_alloc_error` at RUSTLIB/alloc/src/alloc.rs:LL:CC note: inside `miri_start` From 8126c551c836caea4610883a7faf1583a8b78c81 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 12:33:19 +0000 Subject: [PATCH 6/7] Use cfg(bootstrap) as necessary --- library/alloc/src/alloc.rs | 24 ++++++++++++++++++ library/std/src/alloc.rs | 52 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index d3d329ba51ff8..f182409730fbe 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -421,6 +421,7 @@ pub use std::alloc::handle_alloc_error; #[doc(hidden)] #[allow(unused_attributes)] #[unstable(feature = "alloc_internals", issue = "none")] +#[cfg(not(bootstrap))] pub mod __alloc_error_handler { #[rustc_std_internal_symbol] #[linkage = "weak"] @@ -444,3 +445,26 @@ pub mod __alloc_error_handler { } } } + +#[cfg(all(not(no_global_oom_handling), not(test)))] +#[doc(hidden)] +#[allow(unused_attributes)] +#[unstable(feature = "alloc_internals", issue = "none")] +#[cfg(bootstrap)] +pub mod __alloc_error_handler { + #[rustc_std_internal_symbol] + pub unsafe fn __rdl_oom(size: usize, _align: usize) -> ! { + extern "Rust" { + static __rust_alloc_error_handler_should_panic: u8; + } + + if unsafe { __rust_alloc_error_handler_should_panic != 0 } { + panic!("memory allocation of {size} bytes failed") + } else { + core::panicking::panic_nounwind_fmt( + format_args!("memory allocation of {size} bytes failed"), + /* force_no_backtrace */ false, + ) + } + } +} diff --git a/library/std/src/alloc.rs b/library/std/src/alloc.rs index 849ef537aa4ef..2c64da8f5640e 100644 --- a/library/std/src/alloc.rs +++ b/library/std/src/alloc.rs @@ -381,6 +381,7 @@ pub fn rust_oom(layout: Layout) -> ! { #[doc(hidden)] #[allow(unused_attributes)] #[unstable(feature = "alloc_internals", issue = "none")] +#[cfg(not(bootstrap))] pub mod __default_lib_allocator { use super::{GlobalAlloc, Layout, System}; // These are used as a fallback for implementing the `__rust_alloc`, etc symbols @@ -437,3 +438,54 @@ pub mod __default_lib_allocator { } } } + +#[cfg(not(test))] +#[doc(hidden)] +#[allow(unused_attributes)] +#[unstable(feature = "alloc_internals", issue = "none")] +#[cfg(bootstrap)] +pub mod __default_lib_allocator { + use super::{GlobalAlloc, Layout, System}; + + #[rustc_std_internal_symbol] + pub unsafe extern "C" fn __rdl_alloc(size: usize, align: usize) -> *mut u8 { + // SAFETY: see the guarantees expected by `Layout::from_size_align` and + // `GlobalAlloc::alloc`. + unsafe { + let layout = Layout::from_size_align_unchecked(size, align); + System.alloc(layout) + } + } + + #[rustc_std_internal_symbol] + pub unsafe extern "C" fn __rdl_dealloc(ptr: *mut u8, size: usize, align: usize) { + // SAFETY: see the guarantees expected by `Layout::from_size_align` and + // `GlobalAlloc::dealloc`. + unsafe { System.dealloc(ptr, Layout::from_size_align_unchecked(size, align)) } + } + + #[rustc_std_internal_symbol] + pub unsafe extern "C" fn __rdl_realloc( + ptr: *mut u8, + old_size: usize, + align: usize, + new_size: usize, + ) -> *mut u8 { + // SAFETY: see the guarantees expected by `Layout::from_size_align` and + // `GlobalAlloc::realloc`. + unsafe { + let old_layout = Layout::from_size_align_unchecked(old_size, align); + System.realloc(ptr, old_layout, new_size) + } + } + + #[rustc_std_internal_symbol] + pub unsafe extern "C" fn __rdl_alloc_zeroed(size: usize, align: usize) -> *mut u8 { + // SAFETY: see the guarantees expected by `Layout::from_size_align` and + // `GlobalAlloc::alloc_zeroed`. + unsafe { + let layout = Layout::from_size_align_unchecked(size, align); + System.alloc_zeroed(layout) + } + } +} From 8f53ab0e037ca2c0fcb5babdad15311230d433eb Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 20 Dec 2024 13:55:06 +0000 Subject: [PATCH 7/7] Update tests --- tests/run-make/bin-emit-no-symbols/rmake.rs | 2 +- tests/ui/sanitizer/dataflow-abilist.txt | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/run-make/bin-emit-no-symbols/rmake.rs b/tests/run-make/bin-emit-no-symbols/rmake.rs index 5586e53c05084..8cd7b7800f69a 100644 --- a/tests/run-make/bin-emit-no-symbols/rmake.rs +++ b/tests/run-make/bin-emit-no-symbols/rmake.rs @@ -12,5 +12,5 @@ fn main() { let out = llvm_readobj().input("app.o").arg("--symbols").run(); out.assert_stdout_contains("rust_begin_unwind"); out.assert_stdout_contains("rust_eh_personality"); - out.assert_stdout_contains("__rg_oom"); + out.assert_stdout_contains("__rust_alloc_error_handler"); } diff --git a/tests/ui/sanitizer/dataflow-abilist.txt b/tests/ui/sanitizer/dataflow-abilist.txt index fe04838f5493e..d925101e5f9f3 100644 --- a/tests/ui/sanitizer/dataflow-abilist.txt +++ b/tests/ui/sanitizer/dataflow-abilist.txt @@ -490,11 +490,6 @@ fun:__dfso_*=uninstrumented fun:__dfso_*=discard # Rust functions. -fun:__rdl_alloc=uninstrumented -fun:__rdl_alloc_zeroed=uninstrumented -fun:__rdl_dealloc=uninstrumented -fun:__rdl_realloc=uninstrumented -fun:__rg_oom=uninstrumented fun:__rust_alloc=uninstrumented fun:__rust_alloc_error_handler=uninstrumented fun:__rust_alloc_zeroed=uninstrumented