Rename symbols rather than add symbol aliases for EIIs when possible

compiler/rustc_codegen_llvm/src/mono_item.rs

@@ -51,7 +51,9 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
         self.assume_dso_local(g, false);
 
         let attrs = self.tcx.codegen_instance_attrs(instance.def);
-        self.add_static_aliases(g, &attrs.foreign_item_symbol_aliases);
+        if let Some(alias) = &attrs.foreign_item_symbol_alias {
+            self.add_static_alias(g, alias);
+        }
 
         self.instances.borrow_mut().insert(instance, g);
     }
@@ -69,7 +71,9 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
 
         let lldecl =
             self.predefine_without_aliases(instance, &attrs, linkage, visibility, symbol_name);
-        self.add_function_aliases(instance, lldecl, &attrs, &attrs.foreign_item_symbol_aliases);
+        if let Some(alias) = &attrs.foreign_item_symbol_alias {
+            self.add_function_alias(instance, lldecl, &attrs, alias);
+        }
 
         self.instances.borrow_mut().insert(instance, lldecl);
     }
@@ -130,89 +134,89 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
     /// 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)]) {
+    /// So, this function is for static aliases. See [`add_function_alias`](Self::add_function_alias) for the alternative.
+    fn add_static_alias(
+        &self,
+        aliasee: &llvm::Value,
+        (alias, linkage, visibility): &(DefId, Linkage, Visibility),
+    ) {
         let ty = self.get_type_of_global(aliasee);
 
-        for (alias, linkage, visibility) in aliases {
-            let instance = Instance::mono(self.tcx, *alias);
-            let symbol_name = self.tcx.symbol_name(instance);
-            tracing::debug!("STATIC ALIAS: {alias:?} {linkage:?} {visibility:?}");
-
-            let lldecl = llvm::add_alias(
-                self.llmod,
-                ty,
-                AddressSpace::ZERO,
-                aliasee,
-                &CString::new(symbol_name.name).unwrap(),
-            );
-            // Add the alias name to the set of cached items, so there is no duplicate
-            // instance added to it during the normal `external static` codegen
-            let prev_entry = self.instances.borrow_mut().insert(instance, lldecl);
-
-            // If there already was a previous entry, then `add_static_aliases` was called multiple times for the same `alias`
-            // which would result in incorrect codegen
-            assert!(prev_entry.is_none(), "An instance was already present for {instance:?}");
-
-            llvm::set_visibility(lldecl, base::visibility_to_llvm(*visibility));
-            llvm::set_linkage(lldecl, base::linkage_to_llvm(*linkage));
-        }
+        let instance = Instance::mono(self.tcx, *alias);
+        let symbol_name = self.tcx.symbol_name(instance);
+        tracing::debug!("STATIC ALIAS: {alias:?} {linkage:?} {visibility:?}");
+
+        let lldecl = llvm::add_alias(
+            self.llmod,
+            ty,
+            AddressSpace::ZERO,
+            aliasee,
+            &CString::new(symbol_name.name).unwrap(),
+        );
+        // Add the alias name to the set of cached items, so there is no duplicate
+        // instance added to it during the normal `external static` codegen
+        let prev_entry = self.instances.borrow_mut().insert(instance, lldecl);
+
+        // If there already was a previous entry, then `add_static_aliases` was called multiple times for the same `alias`
+        // which would result in incorrect codegen
+        assert!(prev_entry.is_none(), "An instance was already present for {instance:?}");
+
+        llvm::set_visibility(lldecl, base::visibility_to_llvm(*visibility));
+        llvm::set_linkage(lldecl, base::linkage_to_llvm(*linkage));
     }
 
-    /// See [`add_static_aliases`](Self::add_static_aliases) for docs.
-    fn add_function_aliases(
+    /// See [`add_static_alias`](Self::add_static_alias) for docs.
+    fn add_function_alias(
         &self,
         aliasee_instance: Instance<'tcx>,
         aliasee: &'ll llvm::Value,
         attrs: &Cow<'_, CodegenFnAttrs>,
-        aliases: &[(DefId, Linkage, Visibility)],
+        (alias, linkage, visibility): &(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: generating fn {} that calls {aliasee_instance:?} ({alias:?} {linkage:?} {visibility:?})",
-                symbol_name.name
-            );
-
-            // 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));
-            }
+        let symbol_name = self.tcx.symbol_name(Instance::mono(self.tcx, *alias));
+        tracing::debug!(
+            "FUNCTION ALIAS: generating fn {} that calls {aliasee_instance:?} ({alias:?} {linkage:?} {visibility:?})",
+            symbol_name.name
+        );
+
+        // 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));
+        }
 
-            let call = start_bx.call(
-                fn_ty,
-                Some(attrs),
-                Some(fn_abi),
-                aliasee,
-                &args,
-                None,
-                Some(aliasee_instance),
-            );
-
-            match &fn_abi.ret.mode {
-                PassMode::Ignore | PassMode::Indirect { .. } => start_bx.ret_void(),
-                PassMode::Direct(_) | PassMode::Pair { .. } | PassMode::Cast { .. } => {
-                    start_bx.ret(call)
-                }
+        let call = start_bx.call(
+            fn_ty,
+            Some(attrs),
+            Some(fn_abi),
+            aliasee,
+            &args,
+            None,
+            Some(aliasee_instance),
+        );
+
+        match &fn_abi.ret.mode {
+            PassMode::Ignore | PassMode::Indirect { .. } => start_bx.ret_void(),
+            PassMode::Direct(_) | PassMode::Pair { .. } | PassMode::Cast { .. } => {
+                start_bx.ret(call)
             }
         }
     }

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -222,8 +222,8 @@ pub fn exported_non_generic_symbols_helper<'tcx>(
     }
 
     symbols.extend(sorted.iter().flat_map(|&(&def_id, &info)| {
-        tcx.codegen_fn_attrs(def_id).foreign_item_symbol_aliases.iter().map(
-            move |&(foreign_item, _linkage, _visibility)| {
+        tcx.codegen_fn_attrs(def_id).foreign_item_symbol_alias.map(
+            move |(foreign_item, _linkage, _visibility)| {
                 (ExportedSymbol::NonGeneric(foreign_item), info)
             },
         )

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -13,10 +13,10 @@ use rustc_middle::middle::codegen_fn_attrs::{
 };
 use rustc_middle::mono::Visibility;
 use rustc_middle::query::Providers;
-use rustc_middle::ty::{self as ty, TyCtxt};
+use rustc_middle::ty::{self as ty, Instance, TyCtxt};
 use rustc_session::diagnostics::feature_err;
 use rustc_session::lint;
-use rustc_span::{Span, sym};
+use rustc_span::{Span, Symbol, sym};
 use rustc_target::spec::Os;
 
 use crate::diagnostics;
@@ -93,8 +93,11 @@ fn process_builtin_attrs(
             AttributeKind::LinkSection { name } => codegen_fn_attrs.link_section = Some(*name),
             AttributeKind::NoMangle(attr_span) => {
                 interesting_spans.no_mangle = Some(*attr_span);
-                if tcx.opt_item_name(did.to_def_id()).is_some() {
-                    codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
+                if let Some(name) = tcx.opt_item_name(did.to_def_id()) {
+                    // Don't override #[link_name] or #[export_name]
+                    if codegen_fn_attrs.symbol_name.is_none() {
+                        codegen_fn_attrs.symbol_name = Some(name);
+                    }
                 } else {
                     tcx.dcx()
                         .span_delayed_bug(*attr_span, "no_mangle should be on a named function");
@@ -255,11 +258,17 @@ fn process_builtin_attrs(
                     continue;
                 }
 
-                codegen_fn_attrs.foreign_item_symbol_aliases.push((
-                    foreign_item,
-                    if i.is_default { Linkage::WeakAny } else { Linkage::External },
-                    Visibility::Default,
-                ));
+                if !i.is_default {
+                    // FIXME is tcx.symbol_name() here safe or should we move this to
+                    // rustc_symbol_mangling?
+                    codegen_fn_attrs.symbol_name = Some(Symbol::intern(
+                        tcx.symbol_name(Instance::mono(tcx, foreign_item)).name,
+                    ));
+                } else {
+                    assert!(codegen_fn_attrs.foreign_item_symbol_alias.is_none());
+                    codegen_fn_attrs.foreign_item_symbol_alias =
+                        Some((foreign_item, Linkage::WeakAny, Visibility::Default));
+                }
                 codegen_fn_attrs.flags |= CodegenFnAttrFlags::EXTERNALLY_IMPLEMENTABLE_ITEM;
 
                 // If the declaration is `#[track_caller]`, derive it onto the implementation
@@ -402,6 +411,8 @@ fn apply_overrides(tcx: TyCtxt<'_>, did: LocalDefId, codegen_fn_attrs: &mut Code
             //   get the same symbol name as the *mangled* foreign item they refer to so that's all good.
         } else if codegen_fn_attrs.symbol_name.is_some() {
             // * This can be overridden with the `#[link_name]` attribute
+        } else if codegen_fn_attrs.link_ordinal.is_some() {
+            // * `#[link_ordinal]` and `#[link_name]` are incompatible with each other
         } else {
             // NOTE: there's one more exception that we cannot apply here. On wasm,
             // some items cannot be `no_mangle`.
@@ -410,7 +421,7 @@ fn apply_overrides(tcx: TyCtxt<'_>, did: LocalDefId, codegen_fn_attrs: &mut Code
             // import will *still* be mangled despite this.
             //
             // if none of the exceptions apply; apply no_mangle
-            codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
+            codegen_fn_attrs.symbol_name = Some(tcx.item_name(did));
         }
     }
 }
@@ -543,22 +554,24 @@ fn handle_lang_items(
     // strippable by the linker.
     //
     // Additionally weak lang items have predetermined symbol names.
-    if let Some(lang_item) = lang_item
+    let link_name_override = if let Some(lang_item) = lang_item
         && let Some(link_name) = lang_item.link_name()
     {
         codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL;
-        codegen_fn_attrs.symbol_name = Some(link_name);
-    }
+        Some(link_name)
+    } else {
+        None
+    };
 
-    // error when using no_mangle on a lang item item
+    // error when using no_mangle, or export_name on a lang item item
     if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
-        && codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE)
+        && codegen_fn_attrs.symbol_name.is_some()
     {
         let mut err = tcx
             .dcx()
             .struct_span_err(
                 interesting_spans.no_mangle.unwrap_or_default(),
-                "`#[no_mangle]` cannot be used on internal language items",
+                "`#[no_mangle]` and `#[export_name]` cannot be used on internal language items",
             )
             .with_note("Rustc requires this item to have a specific mangled name.")
             .with_span_label(tcx.def_span(did), "should be the internal language item");
@@ -574,6 +587,10 @@ fn handle_lang_items(
         }
         err.emit();
     }
+
+    if let Some(link_name_override) = link_name_override {
+        codegen_fn_attrs.symbol_name = Some(link_name_override);
+    }
 }
 
 /// Generate the [`CodegenFnAttrs`] for an item (identified by the [`LocalDefId`]).

compiler/rustc_lint/src/foreign_modules.rs

@@ -77,23 +77,11 @@ struct ClashingExternDeclarations {
     seen_decls: UnordMap<Symbol, hir::OwnerId>,
 }
 
-/// Differentiate between whether the name for an extern decl came from the link_name attribute or
-/// just from declaration itself. This is important because we don't want to report clashes on
-/// symbol name if they don't actually clash because one or the other links against a symbol with a
-/// different name.
-enum SymbolName {
-    /// The name of the symbol + the span of the annotation which introduced the link name.
-    Link(Symbol, Span),
-    /// No link name, so just the name of the symbol.
-    Normal(Symbol),
-}
-
-impl SymbolName {
-    fn get_name(&self) -> Symbol {
-        match self {
-            SymbolName::Link(s, _) | SymbolName::Normal(s) => *s,
-        }
-    }
+struct SymbolName {
+    /// The name of the symbol that will be linked against.
+    link_name: Symbol,
+    /// The span of the annotation which introduced the link name.
+    span: Option<Span>,
 }
 
 impl ClashingExternDeclarations {
@@ -140,7 +128,7 @@ impl ClashingExternDeclarations {
 
             // Finally, emit the diagnostic.
             let this = tcx.item_name(this_fi.owner_id.to_def_id());
-            let orig = orig.get_name();
+            let orig = orig.link_name;
             let previous_decl_label = get_relevant_span(tcx, existing_did);
             let mismatch_label = get_relevant_span(tcx, this_fi.owner_id);
             let sub =
@@ -176,28 +164,24 @@ impl ClashingExternDeclarations {
 /// the name specified in a #[link_name = ...] attribute if one was specified, else, just the
 /// symbol's name.
 fn name_of_extern_decl(tcx: TyCtxt<'_>, fi: hir::OwnerId) -> SymbolName {
-    if let Some((overridden_link_name, overridden_link_name_span)) =
-        tcx.codegen_fn_attrs(fi).symbol_name.map(|overridden_link_name| {
-            // FIXME: Instead of searching through the attributes again to get span
-            // information, we could have codegen_fn_attrs also give span information back for
-            // where the attribute was defined. However, until this is found to be a
-            // bottleneck, this does just fine.
-            (overridden_link_name, find_attr!(tcx, fi, LinkName {span, ..} => *span).unwrap())
-        })
-    {
-        SymbolName::Link(overridden_link_name, overridden_link_name_span)
-    } else {
-        SymbolName::Normal(tcx.item_name(fi.to_def_id()))
+    // FIXME if symbol_name is not set, this is likely a #[rustc_std_internal_symbol] or EII which
+    // actually have their name mangled and thus should use the mangled name here.
+    let link_name =
+        tcx.codegen_fn_attrs(fi).symbol_name.unwrap_or_else(|| tcx.item_name(fi.to_def_id()));
+    SymbolName {
+        link_name,
+        // FIXME: Instead of searching through the attributes again to get span
+        // information, we could have codegen_fn_attrs also give span information back for
+        // where the attribute was defined. However, until this is found to be a
+        // bottleneck, this does just fine.
+        span: find_attr!(tcx, fi, LinkName {span, ..} => *span),
     }
 }
 
 /// We want to ensure that we use spans for both decls that include where the
 /// name was defined, whether that was from the link_name attribute or not.
 fn get_relevant_span(tcx: TyCtxt<'_>, fi: hir::OwnerId) -> Span {
-    match name_of_extern_decl(tcx, fi) {
-        SymbolName::Normal(_) => tcx.def_span(fi),
-        SymbolName::Link(_, annot_span) => annot_span,
-    }
+    if let Some(span) = name_of_extern_decl(tcx, fi).span { span } else { tcx.def_span(fi) }
 }
 
 /// Checks whether two types are structurally the same enough that the declarations shouldn't