Skip to content
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.

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

rustc: Group linked libraries where needed #49316

Merged
merged 1 commit into from
Mar 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add an assert!(group_start.is_some() == group_end.is_some())?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah unfortunately that assertion tripped when I added it, so I ended up having to implement it this way for libstd specifically

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
}