diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index 621004e756ec1..2871326b28b5a 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -284,6 +284,10 @@ pub(crate) fn set_comdat(llmod: &Module, llglobal: &Value, name: &CStr) { } } +pub(crate) fn count_params(llfn: &Value) -> c_uint { + LLVMCountParams(llfn) +} + /// Safe wrapper around `LLVMGetParam`, because segfaults are no fun. pub(crate) fn get_param(llfn: &Value, index: c_uint) -> &Value { unsafe { diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index 838db689e7292..fc203712f9c03 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::ffi::CString; use rustc_abi::AddressSpace; @@ -6,13 +7,17 @@ use rustc_hir::attrs::Linkage; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::bug; +use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs; use rustc_middle::mir::mono::Visibility; use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv, LayoutOf}; -use rustc_middle::ty::{self, Instance, TypeVisitableExt}; +use rustc_middle::ty::{self, Instance, Ty, TypeVisitableExt}; use rustc_session::config::CrateType; +use rustc_target::callconv::FnAbi; use rustc_target::spec::{Arch, RelocModel}; use tracing::debug; +use crate::abi::FnAbiLlvmExt; +use crate::builder::Builder; use crate::context::CodegenCx; use crate::errors::SymbolAlreadyDefined; use crate::type_of::LayoutLlvmExt; @@ -45,7 +50,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { self.assume_dso_local(g, false); let attrs = self.tcx.codegen_instance_attrs(instance.def); - self.add_aliases(g, &attrs.foreign_item_symbol_aliases); + self.add_static_aliases(g, &attrs.foreign_item_symbol_aliases); self.instances.borrow_mut().insert(instance, g); } @@ -59,11 +64,29 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { ) { assert!(!instance.args.has_infer()); - let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty()); + let attrs = self.tcx.codegen_instance_attrs(instance.def); + + let lldecl = + self.predefine_without_aliases(instance, &attrs, linkage, visibility, symbol_name); + self.add_function_aliases(instance, lldecl, &attrs, &attrs.foreign_item_symbol_aliases); + + self.instances.borrow_mut().insert(instance, lldecl); + } +} + +impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { + fn predefine_without_aliases( + &self, + instance: Instance<'tcx>, + attrs: &Cow<'_, CodegenFnAttrs>, + linkage: Linkage, + visibility: Visibility, + symbol_name: &str, + ) -> &'ll llvm::Value { + let fn_abi: &FnAbi<'tcx, Ty<'tcx>> = self.fn_abi_of_instance(instance, ty::List::empty()); let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance)); llvm::set_linkage(lldecl, base::linkage_to_llvm(linkage)); - let attrs = self.tcx.codegen_instance_attrs(instance.def); - base::set_link_section(lldecl, &attrs); + base::set_link_section(lldecl, attrs); if (linkage == Linkage::LinkOnceODR || linkage == Linkage::WeakODR) && self.tcx.sess.target.supports_comdat() { @@ -84,20 +107,45 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { self.assume_dso_local(lldecl, false); - self.add_aliases(lldecl, &attrs.foreign_item_symbol_aliases); - - self.instances.borrow_mut().insert(instance, lldecl); + lldecl } -} -impl CodegenCx<'_, '_> { - fn add_aliases(&self, aliasee: &llvm::Value, aliases: &[(DefId, Linkage, Visibility)]) { + /// LLVM has the concept of an `alias`. + /// We need this for the "externally implementable items" feature, + /// though it's generally useful. + /// + /// On macos, though this might be a more general problem, function symbols + /// have a fixed target architecture. This is necessary, since macos binaries + /// may contain code for both ARM and x86 macs. + /// + /// LLVM *can* add attributes for target architecture to function symbols, + /// cannot do so for statics, but importantly, also cannot for aliases + /// *even* when aliases may refer to a function symbol. + /// + /// This is not a problem: instead of using LLVM aliases, we can just generate + /// a new function symbol (with target architecture!) which effectively comes down to: + /// + /// ```ignore (illustrative example) + /// fn alias_name(...args) { + /// original_name(...args) + /// } + /// ``` + /// + /// That's also an alias. + /// + /// This does mean that the alias symbol has a different address than the original symbol + /// (assuming no optimizations by LLVM occur). This is unacceptable for statics. + /// So for statics we do want to use LLVM aliases, which is fine, + /// since for those we don't care about target architecture anyway. + /// + /// So, this function is for static aliases. See [`add_function_aliases`](Self::add_function_aliases) for the alternative. + fn add_static_aliases(&self, aliasee: &llvm::Value, aliases: &[(DefId, Linkage, Visibility)]) { let ty = self.get_type_of_global(aliasee); for (alias, linkage, visibility) in aliases { let symbol_name = self.tcx.symbol_name(Instance::mono(self.tcx, *alias)); + tracing::debug!("STATIC ALIAS: {alias:?} {linkage:?} {visibility:?}"); - tracing::debug!("ALIAS: {alias:?} {linkage:?} {visibility:?}"); let lldecl = llvm::add_alias( self.llmod, ty, @@ -111,6 +159,54 @@ impl CodegenCx<'_, '_> { } } + /// See [`add_static_aliases`](Self::add_static_aliases) for docs. + fn add_function_aliases( + &self, + aliasee_instance: Instance<'tcx>, + aliasee: &'ll llvm::Value, + attrs: &Cow<'_, CodegenFnAttrs>, + aliases: &[(DefId, Linkage, Visibility)], + ) { + for (alias, linkage, visibility) in aliases { + let symbol_name = self.tcx.symbol_name(Instance::mono(self.tcx, *alias)); + tracing::debug!("FUNCTION ALIAS: {alias:?} {linkage:?} {visibility:?}"); + + // predefine another copy of the original instance + // with a new symbol name + let alias_lldecl = self.predefine_without_aliases( + aliasee_instance, + attrs, + *linkage, + *visibility, + symbol_name.name, + ); + + let fn_abi: &FnAbi<'tcx, Ty<'tcx>> = + self.fn_abi_of_instance(aliasee_instance, ty::List::empty()); + + // both the alias and the aliasee have the same ty + let fn_ty = fn_abi.llvm_type(self); + let start_llbb = Builder::append_block(self, alias_lldecl, "start"); + let mut start_bx = Builder::build(self, start_llbb); + + let num_params = llvm::count_params(alias_lldecl); + let mut args = Vec::with_capacity(num_params as usize); + for index in 0..num_params { + args.push(llvm::get_param(alias_lldecl, index)); + } + + start_bx.tail_call( + fn_ty, + Some(attrs), + fn_abi, + aliasee, + &args, + None, + Some(aliasee_instance), + ); + } + } + /// A definition or declaration can be assumed to be local to a group of /// libraries that form a single DSO or executable. /// Marks the local as DSO if so. diff --git a/tests/ui/eii/default/auxiliary/decl_with_default_panics.rs b/tests/ui/eii/default/auxiliary/decl_with_default_panics.rs new file mode 100644 index 0000000000000..14778a40cde4a --- /dev/null +++ b/tests/ui/eii/default/auxiliary/decl_with_default_panics.rs @@ -0,0 +1,10 @@ +//@ no-prefer-dynamic +//@ needs-unwind +//@ exec-env:RUST_BACKTRACE=1 +#![crate_type = "rlib"] +#![feature(extern_item_impls)] + +#[eii(eii1)] +pub fn decl1(x: u64) { + panic!("{}", x); +} diff --git a/tests/ui/eii/default/call_default.rs b/tests/ui/eii/default/call_default.rs index b479baa804449..c13df48e99217 100644 --- a/tests/ui/eii/default/call_default.rs +++ b/tests/ui/eii/default/call_default.rs @@ -5,16 +5,6 @@ //@ ignore-backends: gcc // FIXME: linking on windows (speciifcally mingw) not yet supported, see tracking issue #125418 //@ ignore-windows -// Functions can have target-cpu applied. On apple-darwin this is super important, -// since you can have binaries which mix x86 and aarch64 code that are compatible -// with both architectures. So we can't just reject target_cpu on EIIs since apple -// puts them on by default. The problem: we generate aliases. And aliases cannot -// get target_cpu applied to them. So, instead we should, in the case of functions, -// generate a shim function. For statics aliases should keep working in theory. -// In fact, aliases are only necessary for statics. For functions we could just -// always generate a shim and a previous version of EII did so but I was sad -// that that'd never support statics. -//@ ignore-macos // Tests EIIs with default implementations. // When there's no explicit declaration, the default should be called from the declaring crate. #![feature(extern_item_impls)] diff --git a/tests/ui/eii/default/call_default_panics.rs b/tests/ui/eii/default/call_default_panics.rs new file mode 100644 index 0000000000000..f71fddb71ba26 --- /dev/null +++ b/tests/ui/eii/default/call_default_panics.rs @@ -0,0 +1,35 @@ +//@ no-prefer-dynamic +//@ aux-build: decl_with_default_panics.rs +//@ edition: 2021 +//@ run-pass +//@ needs-unwind +//@ exec-env:RUST_BACKTRACE=1 +//@ ignore-backends: gcc +// FIXME: linking on windows (speciifcally mingw) not yet supported, see tracking issue #125418 +//@ ignore-windows +// A small test to make sure that unwinding works properly. +// +// Functions can have target-cpu applied. On apple-darwin this is super important, +// since you can have binaries which mix x86 and aarch64 code that are compatible +// with both architectures. So we can't just reject target_cpu on EIIs since apple +// puts them on by default. The problem: we generate aliases. And aliases cannot +// get target_cpu applied to them. So, instead we should, in the case of functions, +// generate a shim function. For statics aliases should keep working. +// However, to make this work properly, +// on LLVM we generate shim functions instead of function aliases. +// Little extra functions that look like +// ``` +// function alias_symbol(*args) {return (tailcall) aliasee(*args);} +// ``` +// This is a simple test to make sure that we can unwind through these, +// and that this wrapper function effectively doesn't show up in the trace. +#![feature(extern_item_impls)] + +extern crate decl_with_default_panics; + +fn main() { + let result = std::panic::catch_unwind(|| { + decl_with_default_panics::decl1(10); + }); + assert!(result.is_err()); +} diff --git a/tests/ui/eii/default/call_default_panics.run.stderr b/tests/ui/eii/default/call_default_panics.run.stderr new file mode 100644 index 0000000000000..3edd721667369 --- /dev/null +++ b/tests/ui/eii/default/call_default_panics.run.stderr @@ -0,0 +1,10 @@ + +thread 'main' ($TID) panicked at $DIR/auxiliary/decl_with_default_panics.rs:14:5: +10 +stack backtrace: + 0: __rustc::rust_begin_unwind + 1: core::panicking::panic_fmt + 2: core::panicking::panic_display:: + 3: decl_with_default_panics::_::decl1 + 4: call_default_panics::main +note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.