Skip to content

Commit

Permalink
Auto merge of #100404 - BelovDV:linker_group, r=petrochenkov
Browse files Browse the repository at this point in the history 
change stdlib circular dependencies handling

Remove group_start and group_end, add dependencies to symbols.o
Implements the suggestion from #85805 (comment)
r? `@petrochenkov`
  • Loading branch information
@bors
bors committed Sep 6, 2022
2 parents 78a891d + c34047c commit 699bfa8
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 122 deletions.
76 changes: 16 additions & 60 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 => {
Expand All @@ -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
Expand Down
42 changes: 0 additions & 42 deletions compiler/rustc_codegen_ssa/src/back/linker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {}
Expand Down Expand Up @@ -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) {}
}

Expand Down Expand Up @@ -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) {}
}
52 changes: 35 additions & 17 deletions compiler/rustc_codegen_ssa/src/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand All @@ -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};

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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);
}
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_ssa/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_hir/src/weak_lang_items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, _)| {
Expand Down

0 comments on commit 699bfa8

Please sign in to comment.