Skip to content

change stdlib circular dependencies handling #100404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 7, 2022
Merged

change stdlib circular dependencies handling #100404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b672715
change stdlib circular dependencies handling
belovdv Aug 15, 2022
c34047c
Fix CI failures on windows and aarch64-linux
petrochenkov Sep 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 16 additions & 60 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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