Skip to content

Commit

Permalink
Auto merge of #35637 - japaric:no-builtins-lto, r=alexcrichton
Browse files Browse the repository at this point in the history
exclude `#![no_builtins]` crates from LTO

this prevents intrinsics like `memcpy` from being mis-optimized to
infinite recursive calls when LTO is used.

fixes #31544
closes #35540

---

r? @alexcrichton
cc @Amanieu
  • Loading branch information
bors authored Aug 16, 2016
2 parents 1de5b7e + e996405 commit db7300d
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ pub trait CrateStore<'tcx> {
fn plugin_registrar_fn(&self, cnum: ast::CrateNum) -> Option<DefId>;
fn native_libraries(&self, cnum: ast::CrateNum) -> Vec<(NativeLibraryKind, String)>;
fn reachable_ids(&self, cnum: ast::CrateNum) -> Vec<DefId>;
fn is_no_builtins(&self, cnum: ast::CrateNum) -> bool;

// resolve
fn def_index_for_def_key(&self,
Expand Down Expand Up @@ -428,6 +429,7 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
fn native_libraries(&self, cnum: ast::CrateNum) -> Vec<(NativeLibraryKind, String)>
{ bug!("native_libraries") }
fn reachable_ids(&self, cnum: ast::CrateNum) -> Vec<DefId> { bug!("reachable_ids") }
fn is_no_builtins(&self, cnum: ast::CrateNum) -> bool { bug!("is_no_builtins") }

// resolve
fn def_key(&self, def: DefId) -> hir_map::DefKey { bug!("def_key") }
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_metadata/csearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,10 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
decoder::get_reachable_ids(&cdata)
}

fn is_no_builtins(&self, cnum: ast::CrateNum) -> bool {
attr::contains_name(&self.crate_attrs(cnum), "no_builtins")
}

fn def_index_for_def_key(&self,
cnum: ast::CrateNum,
def: DefKey)
Expand Down
24 changes: 15 additions & 9 deletions src/librustc_trans/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ fn add_upstream_rust_crates(cmd: &mut Linker,
Linkage::IncludedFromDylib => {}
Linkage::Static => {
add_static_crate(cmd, sess, tmpdir, crate_type,
&src.rlib.unwrap().0)
&src.rlib.unwrap().0, sess.cstore.is_no_builtins(cnum))
}
Linkage::Dynamic => {
add_dynamic_crate(cmd, sess, &src.dylib.unwrap().0)
Expand All @@ -964,12 +964,16 @@ fn add_upstream_rust_crates(cmd: &mut Linker,
// * For LTO, we remove upstream object files.
// * For dylibs we remove metadata and bytecode from upstream rlibs
//
// When performing LTO, all of the bytecode from the upstream libraries has
// already been included in our object file output. As a result we need to
// remove the object files in the upstream libraries so the linker doesn't
// try to include them twice (or whine about duplicate symbols). We must
// continue to include the rest of the rlib, however, as it may contain
// static native libraries which must be linked in.
// When performing LTO, almost(*) all of the bytecode from the upstream
// libraries has already been included in our object file output. As a
// result we need to remove the object files in the upstream libraries so
// the linker doesn't try to include them twice (or whine about duplicate
// symbols). We must continue to include the rest of the rlib, however, as
// it may contain static native libraries which must be linked in.
//
// (*) Crates marked with `#![no_builtins]` don't participate in LTO and
// their bytecode wasn't included. The object files in those libraries must
// still be passed to the linker.
//
// When making a dynamic library, linkers by default don't include any
// object files in an archive if they're not necessary to resolve the link.
Expand All @@ -989,7 +993,8 @@ fn add_upstream_rust_crates(cmd: &mut Linker,
sess: &Session,
tmpdir: &Path,
crate_type: config::CrateType,
cratepath: &Path) {
cratepath: &Path,
is_a_no_builtins_crate: bool) {
if !sess.lto() && crate_type != config::CrateTypeDylib {
cmd.link_rlib(&fix_windows_verbatim_for_gcc(cratepath));
return
Expand All @@ -1013,7 +1018,8 @@ fn add_upstream_rust_crates(cmd: &mut Linker,
}
let canonical = f.replace("-", "_");
let canonical_name = name.replace("-", "_");
if sess.lto() && canonical.starts_with(&canonical_name) &&
if sess.lto() && !is_a_no_builtins_crate &&
canonical.starts_with(&canonical_name) &&
canonical.ends_with(".o") {
let num = &f[name.len()..f.len() - 2];
if num.len() > 0 && num[1..].parse::<u32>().is_ok() {
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_trans/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ pub fn run(sess: &session::Session, llmod: ModuleRef,
// For each of our upstream dependencies, find the corresponding rlib and
// load the bitcode from the archive. Then merge it into the current LLVM
// module that we've got.
link::each_linked_rlib(sess, &mut |_, path| {
link::each_linked_rlib(sess, &mut |cnum, path| {
// `#![no_builtins]` crates don't participate in LTO.
if sess.cstore.is_no_builtins(cnum) {
return;
}

let archive = ArchiveRO::open(&path).expect("wanted an rlib");
let bytecodes = archive.iter().filter_map(|child| {
child.ok().and_then(|c| c.name().map(|name| (name, c)))
Expand Down
9 changes: 9 additions & 0 deletions src/test/run-make/no-builtins-lto/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-include ../tools.mk

all:
# Compile a `#![no_builtins]` rlib crate
$(RUSTC) no_builtins.rs
# Build an executable that depends on that crate using LTO. The no_builtins crate doesn't
# participate in LTO, so its rlib must be explicitly linked into the final binary. Verify this by
# grepping the linker arguments.
$(RUSTC) main.rs -C lto -Z print-link-args | grep 'libno_builtins.rlib'
13 changes: 13 additions & 0 deletions src/test/run-make/no-builtins-lto/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2016 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.

extern crate no_builtins;

fn main() {}
12 changes: 12 additions & 0 deletions src/test/run-make/no-builtins-lto/no_builtins.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2016 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.

#![crate_type = "lib"]
#![no_builtins]

0 comments on commit db7300d

Please sign in to comment.