Auto merge of #144678 - jdonszelmann:no-mangle-extern, r=bjorn3

Make no_mangle on foreign items explicit instead of implicit

for a followup PR I'm working on I need some foreign items to mangle. I could add a new attribute: `no_no_mangle` or something silly like that but by explicitly putting `no_mangle` in the codegen fn attrs of foreign items we can default it to `no_mangle` and then easily remove it when we don't want it.

I guess you'd know about this r? `@bjorn3.` Shouldn't be too hard to review :)

Builds on rust-lang/rust#144655 which should merge first.

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -569,7 +569,7 @@ fn symbol_export_level(tcx: TyCtxt<'_>, sym_def_id: DefId) -> SymbolExportLevel
     // core/std/allocators/etc. For example symbols used to hook up allocation
     // are not considered for export
     let codegen_fn_attrs = tcx.codegen_fn_attrs(sym_def_id);
-    let is_extern = codegen_fn_attrs.contains_extern_indicator();
+    let is_extern = codegen_fn_attrs.contains_extern_indicator(tcx, sym_def_id);
     let std_internal =
         codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL);
 

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -443,6 +443,27 @@ fn apply_overrides(tcx: TyCtxt<'_>, did: LocalDefId, codegen_fn_attrs: &mut Code
     if tcx.should_inherit_track_caller(did) {
         codegen_fn_attrs.flags |= CodegenFnAttrFlags::TRACK_CALLER;
     }
+
+    // Foreign items by default use no mangling for their symbol name.
+    if tcx.is_foreign_item(did) {
+        // There's a few exceptions to this rule though:
+        if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
+            // * `#[rustc_std_internal_symbol]` mangles the symbol name in a special way
+            //   both for exports and imports through foreign items. This is handled further,
+            //   during symbol mangling logic.
+        } else if codegen_fn_attrs.link_name.is_some() {
+            // * This can be overridden with the `#[link_name]` attribute
+        } else {
+            // NOTE: there's one more exception that we cannot apply here. On wasm,
+            // some items cannot be `no_mangle`.
+            // However, we don't have enough information here to determine that.
+            // As such, no_mangle foreign items on wasm that have the same defid as some
+            // import will *still* be mangled despite this.
+            //
+            // if none of the exceptions apply; apply no_mangle
+            codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
+        }
+    }
 }
 
 fn check_result(

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -3,6 +3,7 @@ use std::borrow::Cow;
 use rustc_abi::Align;
 use rustc_ast::expand::autodiff_attrs::AutoDiffAttrs;
 use rustc_hir::attrs::{InlineAttr, InstructionSetAttr, OptimizeAttr};
+use rustc_hir::def_id::DefId;
 use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 use rustc_span::Symbol;
 use rustc_target::spec::SanitizerSet;
@@ -193,7 +194,11 @@ impl CodegenFnAttrs {
     /// * `#[linkage]` is present
     ///
     /// Keep this in sync with the logic for the unused_attributes for `#[inline]` lint.
-    pub fn contains_extern_indicator(&self) -> bool {
+    pub fn contains_extern_indicator(&self, tcx: TyCtxt<'_>, did: DefId) -> bool {
+        if tcx.is_foreign_item(did) {
+            return false;
+        }
+
         self.flags.contains(CodegenFnAttrFlags::NO_MANGLE)
             || self.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
             || self.export_name.is_some()

compiler/rustc_middle/src/mir/mono.rs

@@ -151,7 +151,7 @@ impl<'tcx> MonoItem<'tcx> {
         // instantiation:
         // We emit an unused_attributes lint for this case, which should be kept in sync if possible.
         let codegen_fn_attrs = tcx.codegen_instance_attrs(instance.def);
-        if codegen_fn_attrs.contains_extern_indicator()
+        if codegen_fn_attrs.contains_extern_indicator(tcx, instance.def.def_id())
             || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED)
         {
             return InstantiationMode::GloballyShared { may_conflict: false };

compiler/rustc_mir_transform/src/cross_crate_inline.rs

@@ -18,7 +18,7 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id);
     // If this has an extern indicator, then this function is globally shared and thus will not
     // generate cgu-internal copies which would make it cross-crate inlinable.
-    if codegen_fn_attrs.contains_extern_indicator() {
+    if codegen_fn_attrs.contains_extern_indicator(tcx, def_id.into()) {
         return false;
     }
 

compiler/rustc_passes/src/check_attr.rs

@@ -563,7 +563,24 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         match target {
             Target::Fn
             | Target::Closure
-            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {}
+            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {
+                // `#[inline]` is ignored if the symbol must be codegened upstream because it's exported.
+                if let Some(did) = hir_id.as_owner()
+                    && self.tcx.def_kind(did).has_codegen_attrs()
+                    && kind != &InlineAttr::Never
+                {
+                    let attrs = self.tcx.codegen_fn_attrs(did);
+                    // Not checking naked as `#[inline]` is forbidden for naked functions anyways.
+                    if attrs.contains_extern_indicator(self.tcx, did.into()) {
+                        self.tcx.emit_node_span_lint(
+                            UNUSED_ATTRIBUTES,
+                            hir_id,
+                            attr_span,
+                            errors::InlineIgnoredForExported {},
+                        );
+                    }
+                }
+            }
             Target::Method(MethodKind::Trait { body: false }) | Target::ForeignFn => {
                 self.tcx.emit_node_span_lint(
                     UNUSED_ATTRIBUTES,
@@ -590,23 +607,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                 self.dcx().emit_err(errors::InlineNotFnOrClosure { attr_span, defn_span });
             }
         }
-
-        // `#[inline]` is ignored if the symbol must be codegened upstream because it's exported.
-        if let Some(did) = hir_id.as_owner()
-            && self.tcx.def_kind(did).has_codegen_attrs()
-            && kind != &InlineAttr::Never
-        {
-            let attrs = self.tcx.codegen_fn_attrs(did);
-            // Not checking naked as `#[inline]` is forbidden for naked functions anyways.
-            if attrs.contains_extern_indicator() {
-                self.tcx.emit_node_span_lint(
-                    UNUSED_ATTRIBUTES,
-                    hir_id,
-                    attr_span,
-                    errors::InlineIgnoredForExported {},
-                );
-            }
-        }
     }
 
     /// Checks that `#[coverage(..)]` is applied to a function/closure/method,

compiler/rustc_passes/src/dead.rs

@@ -703,7 +703,7 @@ fn has_allow_dead_code_or_lang_attr(
 
             // #[used], #[no_mangle], #[export_name], etc also keeps the item alive
             // forcefully, e.g., for placing it in a specific section.
-            cg_attrs.contains_extern_indicator()
+            cg_attrs.contains_extern_indicator(tcx, def_id.into())
                 || cg_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
                 || cg_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
         }

compiler/rustc_passes/src/reachable.rs

@@ -183,7 +183,7 @@ impl<'tcx> ReachableContext<'tcx> {
             } else {
                 CodegenFnAttrs::EMPTY
             };
-            let is_extern = codegen_attrs.contains_extern_indicator();
+            let is_extern = codegen_attrs.contains_extern_indicator(self.tcx, search_item.into());
             if is_extern {
                 self.reachable_symbols.insert(search_item);
             }
@@ -423,8 +423,9 @@ fn has_custom_linkage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     if !tcx.def_kind(def_id).has_codegen_attrs() {
         return false;
     }
+
     let codegen_attrs = tcx.codegen_fn_attrs(def_id);
-    codegen_attrs.contains_extern_indicator()
+    codegen_attrs.contains_extern_indicator(tcx, def_id.into())
         // FIXME(nbdd0121): `#[used]` are marked as reachable here so it's picked up by
         // `linked_symbols` in cg_ssa. They won't be exported in binary or cdylib due to their
         // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs.

compiler/rustc_symbol_mangling/src/lib.rs

@@ -218,32 +218,33 @@ fn compute_symbol_name<'tcx>(
         }
     }
 
-    // Foreign items by default use no mangling for their symbol name. There's a
+    let wasm_import_module_exception_force_mangling = {
-    // few exceptions to this rule though:
+        // * On the wasm32 targets there is a bug (or feature) in LLD [1] where the
-    //
+        //   same-named symbol when imported from different wasm modules will get
-    // * This can be overridden with the `#[link_name]` attribute
+        //   hooked up incorrectly. As a result foreign symbols, on the wasm target,
-    //
+        //   with a wasm import module, get mangled. Additionally our codegen will
-    // * On the wasm32 targets there is a bug (or feature) in LLD [1] where the
+        //   deduplicate symbols based purely on the symbol name, but for wasm this
-    //   same-named symbol when imported from different wasm modules will get
+        //   isn't quite right because the same-named symbol on wasm can come from
-    //   hooked up incorrectly. As a result foreign symbols, on the wasm target,
+        //   different modules. For these reasons if `#[link(wasm_import_module)]`
-    //   with a wasm import module, get mangled. Additionally our codegen will
+        //   is present we mangle everything on wasm because the demangled form will
-    //   deduplicate symbols based purely on the symbol name, but for wasm this
+        //   show up in the `wasm-import-name` custom attribute in LLVM IR.
-    //   isn't quite right because the same-named symbol on wasm can come from
+        //
-    //   different modules. For these reasons if `#[link(wasm_import_module)]`
+        // [1]: https://bugs.llvm.org/show_bug.cgi?id=44316
-    //   is present we mangle everything on wasm because the demangled form will
+        //
-    //   show up in the `wasm-import-name` custom attribute in LLVM IR.
+        // So, on wasm if a foreign item loses its `#[no_mangle]`, it might *still*
-    //
+        // be mangled if we're forced to. Note: I don't like this.
-    // * `#[rustc_std_internal_symbol]` mangles the symbol name in a special way
+        // These kinds of exceptions should be added during the `codegen_attrs` query.
-    //   both for exports and imports through foreign items. This is handled above.
+        // However, we don't have the wasm import module map there yet.
-    // [1]: https://bugs.llvm.org/show_bug.cgi?id=44316
+        tcx.is_foreign_item(def_id)
-    if tcx.is_foreign_item(def_id)
+            && tcx.sess.target.is_like_wasm
-        && (!tcx.sess.target.is_like_wasm
+            && tcx.wasm_import_module_map(LOCAL_CRATE).contains_key(&def_id.into())
-            || !tcx.wasm_import_module_map(def_id.krate).contains_key(&def_id))
+    };
+
+    if let Some(name) = attrs.link_name
+        && !wasm_import_module_exception_force_mangling
     {
-        if let Some(name) = attrs.link_name {
+        // Use provided name
-            return name.to_string();
+        return name.to_string();
-        }
-        return tcx.item_name(def_id).to_string();
     }
 
     if let Some(name) = attrs.export_name {
@@ -251,7 +252,9 @@ fn compute_symbol_name<'tcx>(
         return name.to_string();
     }
 
-    if attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE) {
+    if attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE)
+        && !wasm_import_module_exception_force_mangling
+    {
         // Don't mangle
         return tcx.item_name(def_id).to_string();
     }

src/tools/clippy/clippy_lints/src/missing_inline.rs

@@ -190,5 +190,5 @@ impl<'tcx> LateLintPass<'tcx> for MissingInline {
 /// and a rustc warning would be triggered, see #15301
 fn fn_is_externally_exported(cx: &LateContext<'_>, def_id: DefId) -> bool {
     let attrs = cx.tcx.codegen_fn_attrs(def_id);
-    attrs.contains_extern_indicator()
+    attrs.contains_extern_indicator(cx.tcx, def_id)
 }

src/tools/miri/src/bin/miri.rs

@@ -279,7 +279,7 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
                                 return None;
                             }
                             let codegen_fn_attrs = tcx.codegen_fn_attrs(local_def_id);
-                            if codegen_fn_attrs.contains_extern_indicator()
+                            if codegen_fn_attrs.contains_extern_indicator(tcx, local_def_id.into())
                                 || codegen_fn_attrs
                                     .flags
                                     .contains(CodegenFnAttrFlags::USED_COMPILER)

src/tools/miri/src/helpers.rs

@@ -132,7 +132,7 @@ pub fn iter_exported_symbols<'tcx>(
     for def_id in crate_items.definitions() {
         let exported = tcx.def_kind(def_id).has_codegen_attrs() && {
             let codegen_attrs = tcx.codegen_fn_attrs(def_id);
-            codegen_attrs.contains_extern_indicator()
+            codegen_attrs.contains_extern_indicator(tcx, def_id.into())
                 || codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
                 || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
                 || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)

tests/run-make/wasm-panic-small/rmake.rs

@@ -24,5 +24,5 @@ fn test(cfg: &str) {
 
     let bytes = rfs::read("foo.wasm");
     println!("{}", bytes.len());
-    assert!(bytes.len() < 40_000);
+    assert!(bytes.len() < 40_000, "bytes len was: {}", bytes.len());
 }