Skip to content

Commit

Permalink
rustc: Group linked libraries where needed
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton committed Mar 29, 2018
1 parent ae544ee commit 88114f6
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 0 deletions.
52 changes: 52 additions & 0 deletions src/librustc_trans/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
26 changes: 26 additions & 0 deletions src/librustc_trans/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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<String> {
Expand Down Expand Up @@ -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) {}
}
10 changes: 10 additions & 0 deletions src/librustc_trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_trans/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -405,6 +406,8 @@ struct CrateInfo {
used_crates_dynamic: Vec<(CrateNum, LibSource)>,
wasm_custom_sections: BTreeMap<String, Vec<u8>>,
wasm_imports: FxHashMap<String, String>,
lang_item_to_crate: FxHashMap<LangItem, CrateNum>,
missing_lang_items: FxHashMap<CrateNum, Lrc<Vec<LangItem>>>,
}

__build_diagnostic_array! { librustc_trans, DIAGNOSTICS }
15 changes: 15 additions & 0 deletions src/test/run-make-fulldeps/std-core-cycle/Makefile
Original file line number Diff line number Diff line change
@@ -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)
26 changes: 26 additions & 0 deletions src/test/run-make-fulldeps/std-core-cycle/bar.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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 {}
}
}
22 changes: 22 additions & 0 deletions src/test/run-make-fulldeps/std-core-cycle/foo.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
}

0 comments on commit 88114f6

Please sign in to comment.