Auto merge of #100404 - BelovDV:linker_group, r=petrochenkov

change stdlib circular dependencies handling

Remove group_start and group_end, add dependencies to symbols.o
Implements the suggestion from #85805 (comment)
r? `@petrochenkov`

Author: bors

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -1,6 +1,6 @@
 use rustc_arena::TypedArena;
 use rustc_ast::CRATE_NODE_ID;
-use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::temp_dir::MaybeTempDir;
 use rustc_errors::{ErrorGuaranteed, Handler};
@@ -1714,6 +1714,13 @@ fn add_post_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor
 /// that are necessary for the linking. They are only present in symbol table but not actually
 /// used in any sections, so the linker will therefore pick relevant rlibs for linking, but
 /// unused `#[no_mangle]` or `#[used]` can still be discard by GC sections.
+///
+/// There's a few internal crates in the standard library (aka libcore and
+/// libstd) which actually have a circular dependence upon one another. This
+/// currently arises through "weak lang items" where libcore requires things
+/// like `rust_begin_unwind` but libstd ends up defining it. To get this
+/// circular dependence to work correctly we declare some of these things
+/// in this synthetic object.
 fn add_linked_symbol_object(
     cmd: &mut dyn Linker,
     sess: &Session,
@@ -2333,65 +2340,10 @@ fn add_upstream_rust_crates<'a>(
     // crates.
     let deps = &codegen_results.crate_info.used_crates;
 
-    // There's a few internal crates in the standard library (aka libcore and
-    // libstd) which actually have a circular dependence upon one another. This
-    // currently arises through "weak lang items" where libcore requires things
-    // like `rust_begin_unwind` but libstd ends up defining it. To get this
-    // circular dependence to work correctly in all situations we'll need to be
-    // sure to correctly apply the `--start-group` and `--end-group` options to
-    // GNU linkers, otherwise if we don't use any other symbol from the standard
-    // library it'll get discarded and the whole application won't link.
-    //
-    // In this loop we're calculating the `group_end`, after which crate to
-    // pass `--end-group` and `group_start`, before which crate to pass
-    // `--start-group`. We currently do this by passing `--end-group` after
-    // the first crate (when iterating backwards) that requires a lang item
-    // defined somewhere else. Once that's set then when we've defined all the
-    // necessary lang items we'll pass `--start-group`.
-    //
-    // Note that this isn't amazing logic for now but it should do the trick
-    // for the current implementation of the standard library.
-    let mut group_end = None;
-    let mut group_start = None;
-    // Crates available for linking thus far.
-    let mut available = FxHashSet::default();
-    // Crates required to satisfy dependencies discovered so far.
-    let mut required = FxHashSet::default();
-
-    let info = &codegen_results.crate_info;
-    for &cnum in deps.iter().rev() {
-        if let Some(missing) = info.missing_lang_items.get(&cnum) {
-            let missing_crates = missing.iter().map(|i| info.lang_item_to_crate.get(i).copied());
-            required.extend(missing_crates);
-        }
-
-        required.insert(Some(cnum));
-        available.insert(Some(cnum));
-
-        if required.len() > available.len() && group_end.is_none() {
-            group_end = Some(cnum);
-        }
-        if required.len() == available.len() && group_end.is_some() {
-            group_start = Some(cnum);
-            break;
-        }
-    }
-
-    // If we didn't end up filling in all lang items from upstream crates then
-    // we'll be filling it in with our crate. This probably means we're the
-    // standard library itself, so skip this for now.
-    if group_end.is_some() && group_start.is_none() {
-        group_end = None;
-    }
-
     let mut compiler_builtins = None;
     let search_path = OnceCell::new();
 
     for &cnum in deps.iter() {
-        if group_start == Some(cnum) {
-            cmd.group_start();
-        }
-
         // We may not pass all crates through to the linker. Some crates may
         // appear statically in an existing dylib, meaning we'll pick up all the
         // symbols from the dylib.
@@ -2451,6 +2403,14 @@ fn add_upstream_rust_crates<'a>(
                                 bundle: Some(false),
                                 whole_archive: Some(false) | None,
                             } => {
+                                // HACK/FIXME: Fixup a circular dependency between libgcc and libc
+                                // with glibc. This logic should be moved to the libc crate.
+                                if sess.target.os == "linux"
+                                    && sess.target.env == "gnu"
+                                    && name == "c"
+                                {
+                                    cmd.link_staticlib("gcc", false);
+                                }
                                 cmd.link_staticlib(name, lib.verbatim.unwrap_or(false));
                             }
                             NativeLibKind::LinkArg => {
@@ -2470,10 +2430,6 @@ fn add_upstream_rust_crates<'a>(
             }
             Linkage::Dynamic => add_dynamic_crate(cmd, sess, &src.dylib.as_ref().unwrap().0),
         }
-
-        if group_end == Some(cnum) {
-            cmd.group_end();
-        }
     }
 
     // compiler-builtins are always placed last to ensure that they're

compiler/rustc_codegen_ssa/src/back/linker.rs

@@ -183,8 +183,6 @@ pub trait Linker {
     fn no_default_libraries(&mut self);
     fn export_symbols(&mut self, tmpdir: &Path, crate_type: CrateType, symbols: &[String]);
     fn subsystem(&mut self, subsystem: &str);
-    fn group_start(&mut self);
-    fn group_end(&mut self);
     fn linker_plugin_lto(&mut self);
     fn add_eh_frame_header(&mut self) {}
     fn add_no_exec(&mut self) {}
@@ -730,18 +728,6 @@ impl<'a> Linker for GccLinker<'a> {
         self.hint_dynamic(); // Reset to default before returning the composed command line.
     }
 
-    fn group_start(&mut self) {
-        if self.takes_hints() {
-            self.linker_arg("--start-group");
-        }
-    }
-
-    fn group_end(&mut self) {
-        if self.takes_hints() {
-            self.linker_arg("--end-group");
-        }
-    }
-
     fn linker_plugin_lto(&mut self) {
         match self.sess.opts.cg.linker_plugin_lto {
             LinkerPluginLto::Disabled => {
@@ -1019,10 +1005,6 @@ impl<'a> Linker for MsvcLinker<'a> {
         }
     }
 
-    // MSVC doesn't need group indicators
-    fn group_start(&mut self) {}
-    fn group_end(&mut self) {}
-
     fn linker_plugin_lto(&mut self) {
         // Do nothing
     }
@@ -1165,10 +1147,6 @@ impl<'a> Linker for EmLinker<'a> {
         // noop
     }
 
-    // Appears not necessary on Emscripten
-    fn group_start(&mut self) {}
-    fn group_end(&mut self) {}
-
     fn linker_plugin_lto(&mut self) {
         // Do nothing
     }
@@ -1344,10 +1322,6 @@ impl<'a> Linker for WasmLd<'a> {
 
     fn subsystem(&mut self, _subsystem: &str) {}
 
-    // Not needed for now with LLD
-    fn group_start(&mut self) {}
-    fn group_end(&mut self) {}
-
     fn linker_plugin_lto(&mut self) {
         // Do nothing for now
     }
@@ -1476,14 +1450,6 @@ impl<'a> Linker for L4Bender<'a> {
         self.hint_static(); // Reset to default before returning the composed command line.
     }
 
-    fn group_start(&mut self) {
-        self.cmd.arg("--start-group");
-    }
-
-    fn group_end(&mut self) {
-        self.cmd.arg("--end-group");
-    }
-
     fn linker_plugin_lto(&mut self) {}
 
     fn control_flow_guard(&mut self) {}
@@ -1664,10 +1630,6 @@ impl<'a> Linker for PtxLinker<'a> {
 
     fn subsystem(&mut self, _subsystem: &str) {}
 
-    fn group_start(&mut self) {}
-
-    fn group_end(&mut self) {}
-
     fn linker_plugin_lto(&mut self) {}
 }
 
@@ -1777,9 +1739,5 @@ impl<'a> Linker for BpfLinker<'a> {
 
     fn subsystem(&mut self, _subsystem: &str) {}
 
-    fn group_start(&mut self) {}
-
-    fn group_end(&mut self) {}
-
     fn linker_plugin_lto(&mut self) {}
 }

compiler/rustc_codegen_ssa/src/base.rs

@@ -12,7 +12,7 @@ use crate::traits::*;
 use crate::{CachedModuleCodegen, CompiledModule, CrateInfo, MemFlags, ModuleCodegen, ModuleKind};
 
 use rustc_attr as attr;
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::profiling::{get_resident_set_size, print_time_passes_entry};
 
 use rustc_data_structures::sync::par_iter;
@@ -21,10 +21,12 @@ use rustc_data_structures::sync::ParallelIterator;
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
 use rustc_hir::lang_items::LangItem;
+use rustc_hir::weak_lang_items::WEAK_ITEMS_SYMBOLS;
 use rustc_index::vec::Idx;
 use rustc_metadata::EncodedMetadata;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
 use rustc_middle::middle::exported_symbols;
+use rustc_middle::middle::exported_symbols::SymbolExportKind;
 use rustc_middle::middle::lang_items;
 use rustc_middle::mir::mono::{CodegenUnit, CodegenUnitNameBuilder, MonoItem};
 use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout};
@@ -34,6 +36,7 @@ use rustc_session::cgu_reuse_tracker::CguReuse;
 use rustc_session::config::{self, CrateType, EntryFnType, OutputType};
 use rustc_session::Session;
 use rustc_span::symbol::sym;
+use rustc_span::Symbol;
 use rustc_span::{DebuggerVisualizerFile, DebuggerVisualizerType};
 use rustc_target::abi::{Align, VariantIdx};
 
@@ -815,21 +818,16 @@ impl CrateInfo {
             crate_name: Default::default(),
             used_crates,
             used_crate_source: Default::default(),
-            lang_item_to_crate: Default::default(),
-            missing_lang_items: Default::default(),
             dependency_formats: tcx.dependency_formats(()).clone(),
             windows_subsystem,
             natvis_debugger_visualizers: Default::default(),
         };
-        let lang_items = tcx.lang_items();
-
         let crates = tcx.crates(());
 
         let n_crates = crates.len();
         info.native_libraries.reserve(n_crates);
         info.crate_name.reserve(n_crates);
         info.used_crate_source.reserve(n_crates);
-        info.missing_lang_items.reserve(n_crates);
 
         for &cnum in crates.iter() {
             info.native_libraries
@@ -847,17 +845,37 @@ impl CrateInfo {
             if tcx.is_no_builtins(cnum) {
                 info.is_no_builtins.insert(cnum);
             }
-            let missing = tcx.missing_lang_items(cnum);
-            for &item in missing.iter() {
-                if let Ok(id) = lang_items.require(item) {
-                    info.lang_item_to_crate.insert(item, id.krate);
-                }
-            }
+        }
 
-            // No need to look for lang items that don't actually need to exist.
-            let missing =
-                missing.iter().cloned().filter(|&l| lang_items::required(tcx, l)).collect();
-            info.missing_lang_items.insert(cnum, missing);
+        // Handle circular dependencies in the standard library.
+        // See comment before `add_linked_symbol_object` function for the details.
+        // With msvc-like linkers it's both unnecessary (they support circular dependencies),
+        // and causes linking issues (when weak lang item symbols are "privatized" by LTO).
+        let target = &tcx.sess.target;
+        if !target.is_like_msvc {
+            let missing_weak_lang_items: FxHashSet<&Symbol> = info
+                .used_crates
+                .iter()
+                .flat_map(|cnum| {
+                    tcx.missing_lang_items(*cnum)
+                        .iter()
+                        .filter(|l| lang_items::required(tcx, **l))
+                        .filter_map(|item| WEAK_ITEMS_SYMBOLS.get(item))
+                })
+                .collect();
+            let prefix = if target.is_like_windows && target.arch == "x86" { "_" } else { "" };
+            info.linked_symbols
+                .iter_mut()
+                .filter(|(crate_type, _)| {
+                    !matches!(crate_type, CrateType::Rlib | CrateType::Staticlib)
+                })
+                .for_each(|(_, linked_symbols)| {
+                    linked_symbols.extend(
+                        missing_weak_lang_items
+                            .iter()
+                            .map(|item| (format!("{prefix}{item}"), SymbolExportKind::Text)),
+                    )
+                });
         }
 
         let embed_visualizers = tcx.sess.crate_types().iter().any(|&crate_type| match crate_type {
@@ -878,7 +896,7 @@ impl CrateInfo {
             }
         });
 
-        if tcx.sess.target.is_like_msvc && embed_visualizers {
+        if target.is_like_msvc && embed_visualizers {
             info.natvis_debugger_visualizers =
                 collect_debugger_visualizers_transitive(tcx, DebuggerVisualizerType::Natvis);
         }

compiler/rustc_codegen_ssa/src/lib.rs

@@ -25,7 +25,6 @@ use rustc_ast as ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::sync::Lrc;
 use rustc_hir::def_id::CrateNum;
-use rustc_hir::LangItem;
 use rustc_middle::dep_graph::WorkProduct;
 use rustc_middle::middle::dependency_format::Dependencies;
 use rustc_middle::middle::exported_symbols::SymbolExportKind;
@@ -152,8 +151,6 @@ pub struct CrateInfo {
     pub used_libraries: Vec<NativeLib>,
     pub used_crate_source: FxHashMap<CrateNum, Lrc<CrateSource>>,
     pub used_crates: Vec<CrateNum>,
-    pub lang_item_to_crate: FxHashMap<LangItem, CrateNum>,
-    pub missing_lang_items: FxHashMap<CrateNum, Vec<LangItem>>,
     pub dependency_formats: Lrc<Dependencies>,
     pub windows_subsystem: Option<String>,
     pub natvis_debugger_visualizers: BTreeSet<DebuggerVisualizerFile>,

compiler/rustc_hir/src/weak_lang_items.rs

@@ -18,6 +18,12 @@ pub static WEAK_ITEMS_REFS: LazyLock<FxIndexMap<Symbol, LangItem>> = LazyLock::n
     map
 });
 
+pub static WEAK_ITEMS_SYMBOLS: LazyLock<FxIndexMap<LangItem, Symbol>> = LazyLock::new(|| {
+    let mut map = FxIndexMap::default();
+    $(map.insert(LangItem::$item, sym::$sym);)*
+    map
+});
+
 pub fn link_name(attrs: &[ast::Attribute]) -> Option<Symbol>
 {
     lang_items::extract(attrs).and_then(|(name, _)| {