From 88114f61b44dba22c6fa180e17d7638f2cf7a9d7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Mar 2018 14:33:22 -0700 Subject: [PATCH] rustc: Group linked libraries where needed This commit fixes a longstanding issue with the compiler with circular dependencies between libcore and libstd. The `core` crate requires at least one symbol, the ability to unwind. The `std` crate is the crate which actually defines this symbol, but the `std` crate also depends on the `core` crate. This circular dependency is in general disallowed in Rust as crates cannot have cycles amongst them. A special exception is made just for core/std, but this is also unfortunately incompatible with how GNU linkers work. GNU linkers will process undefined symbols in a left-to-right fashion, only actually linking an rlib like libstd if there are any symbols used from it. This strategy is incompatible with circular dependencies because if we otherwise don't use symbols from libstd we don't discover that we needed it until we're later processing libcore's symbols! To fix this GNU linkers support the `--start-group` and `--end-group` options which indicate "libraries between these markers may have circular dependencies amongst them. The linker invocation has been updated to automatically pass these arguments when we're invoking a GNU linker and automatically calculate where the arguments need to go (around libstd and libcore) Closes #18807 Closes #47074 --- src/librustc_trans/back/link.rs | 52 +++++++++++++++++++ src/librustc_trans/back/linker.rs | 26 ++++++++++ src/librustc_trans/base.rs | 10 ++++ src/librustc_trans/lib.rs | 3 ++ .../run-make-fulldeps/std-core-cycle/Makefile | 15 ++++++ .../run-make-fulldeps/std-core-cycle/bar.rs | 26 ++++++++++ .../run-make-fulldeps/std-core-cycle/foo.rs | 22 ++++++++ 7 files changed, 154 insertions(+) create mode 100644 src/test/run-make-fulldeps/std-core-cycle/Makefile create mode 100644 src/test/run-make-fulldeps/std-core-cycle/bar.rs create mode 100644 src/test/run-make-fulldeps/std-core-cycle/foo.rs diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 5bd63e0340e..6ba3ad879b7 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -32,6 +32,7 @@ use rustc::hir::def_id::CrateNum; use tempdir::TempDir; use rustc_back::{PanicStrategy, RelroLevel}; use rustc_back::target::TargetTriple; +use rustc_data_structures::fx::FxHashSet; use context::get_reloc_model; use llvm; @@ -1188,9 +1189,56 @@ fn add_upstream_rust_crates(cmd: &mut Linker, // crates. let deps = &trans.crate_info.used_crates_dynamic; + // 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; + let mut end_with = FxHashSet(); + let info = &trans.crate_info; + for &(cnum, _) in deps.iter().rev() { + if let Some(missing) = info.missing_lang_items.get(&cnum) { + end_with.extend(missing.iter().cloned()); + if end_with.len() > 0 && group_end.is_none() { + group_end = Some(cnum); + } + } + end_with.retain(|item| info.lang_item_to_crate.get(item) != Some(&cnum)); + if end_with.len() == 0 && 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; 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. @@ -1217,6 +1265,10 @@ fn add_upstream_rust_crates(cmd: &mut Linker, 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 diff --git a/src/librustc_trans/back/linker.rs b/src/librustc_trans/back/linker.rs index a9095a66aaa..ebcf06d63be 100644 --- a/src/librustc_trans/back/linker.rs +++ b/src/librustc_trans/back/linker.rs @@ -125,6 +125,8 @@ pub trait Linker { fn args(&mut self, args: &[String]); fn export_symbols(&mut self, tmpdir: &Path, crate_type: CrateType); fn subsystem(&mut self, subsystem: &str); + fn group_start(&mut self); + fn group_end(&mut self); // Should have been finalize(self), but we don't support self-by-value on trait objects (yet?). fn finalize(&mut self) -> Command; } @@ -420,6 +422,18 @@ impl<'a> Linker for GccLinker<'a> { ::std::mem::swap(&mut cmd, &mut self.cmd); cmd } + + fn group_start(&mut self) { + if !self.sess.target.target.options.is_like_osx { + self.linker_arg("--start-group"); + } + } + + fn group_end(&mut self) { + if !self.sess.target.target.options.is_like_osx { + self.linker_arg("--end-group"); + } + } } pub struct MsvcLinker<'a> { @@ -648,6 +662,10 @@ impl<'a> Linker for MsvcLinker<'a> { ::std::mem::swap(&mut cmd, &mut self.cmd); cmd } + + // MSVC doesn't need group indicators + fn group_start(&mut self) {} + fn group_end(&mut self) {} } pub struct EmLinker<'a> { @@ -810,6 +828,10 @@ impl<'a> Linker for EmLinker<'a> { ::std::mem::swap(&mut cmd, &mut self.cmd); cmd } + + // Appears not necessary on Emscripten + fn group_start(&mut self) {} + fn group_end(&mut self) {} } fn exported_symbols(tcx: TyCtxt, crate_type: CrateType) -> Vec { @@ -953,4 +975,8 @@ impl Linker for WasmLd { ::std::mem::swap(&mut cmd, &mut self.cmd); cmd } + + // Not needed for now with LLD + fn group_start(&mut self) {} + fn group_end(&mut self) {} } diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 3ab33c41346..7ab3499ead3 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1082,7 +1082,10 @@ impl CrateInfo { used_crate_source: FxHashMap(), wasm_custom_sections: BTreeMap::new(), wasm_imports: FxHashMap(), + lang_item_to_crate: FxHashMap(), + missing_lang_items: FxHashMap(), }; + let lang_items = tcx.lang_items(); let load_wasm_items = tcx.sess.crate_types.borrow() .iter() @@ -1128,6 +1131,13 @@ impl CrateInfo { } info.load_wasm_imports(tcx, 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); + } + } + info.missing_lang_items.insert(cnum, missing); } return info diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index bd33707b1c6..e8a1eb3071a 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -78,6 +78,7 @@ use rustc::dep_graph::DepGraph; use rustc::hir::def_id::CrateNum; use rustc::middle::cstore::MetadataLoader; use rustc::middle::cstore::{NativeLibrary, CrateSource, LibSource}; +use rustc::middle::lang_items::LangItem; use rustc::session::{Session, CompileIncomplete}; use rustc::session::config::{OutputFilenames, OutputType, PrintRequest}; use rustc::ty::{self, TyCtxt}; @@ -405,6 +406,8 @@ struct CrateInfo { used_crates_dynamic: Vec<(CrateNum, LibSource)>, wasm_custom_sections: BTreeMap>, wasm_imports: FxHashMap, + lang_item_to_crate: FxHashMap, + missing_lang_items: FxHashMap>>, } __build_diagnostic_array! { librustc_trans, DIAGNOSTICS } diff --git a/src/test/run-make-fulldeps/std-core-cycle/Makefile b/src/test/run-make-fulldeps/std-core-cycle/Makefile new file mode 100644 index 00000000000..adc5be8016c --- /dev/null +++ b/src/test/run-make-fulldeps/std-core-cycle/Makefile @@ -0,0 +1,15 @@ +-include ../tools.mk + +ifeq ($(UNAME),Darwin) +FLAGS := +else +ifdef IS_WINDOWS +FLAGS := +else +FLAGS := -C link-args=-Wl,--no-undefined +endif +endif + +all: + $(RUSTC) bar.rs + $(RUSTC) foo.rs $(FLAGS) diff --git a/src/test/run-make-fulldeps/std-core-cycle/bar.rs b/src/test/run-make-fulldeps/std-core-cycle/bar.rs new file mode 100644 index 00000000000..6def5b6f5e1 --- /dev/null +++ b/src/test/run-make-fulldeps/std-core-cycle/bar.rs @@ -0,0 +1,26 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(allocator_api)] +#![crate_type = "rlib"] + +use std::heap::*; + +pub struct A; + +unsafe impl<'a> Alloc for &'a A { + unsafe fn alloc(&mut self, _: Layout) -> Result<*mut u8, AllocErr> { + loop {} + } + + unsafe fn dealloc(&mut self, _ptr: *mut u8, _: Layout) { + loop {} + } +} diff --git a/src/test/run-make-fulldeps/std-core-cycle/foo.rs b/src/test/run-make-fulldeps/std-core-cycle/foo.rs new file mode 100644 index 00000000000..04742bba3c8 --- /dev/null +++ b/src/test/run-make-fulldeps/std-core-cycle/foo.rs @@ -0,0 +1,22 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(global_allocator)] +#![crate_type = "cdylib"] + +extern crate bar; + +#[global_allocator] +static A: bar::A = bar::A; + +#[no_mangle] +pub extern fn a(a: u32, b: u32) -> u32 { + a / b +}