Skip to content

Commit

Permalink
rustc: Deterministically link upstream C libraries
Browse files Browse the repository at this point in the history
Previously, upstream C libraries were linked in a nondeterministic fashion
because they were collected through iter_crate_data() which is a nodeterministic
traversal of a hash map. When upstream rlibs had interdependencies among their
native libraries (such as libfoo depending on libc), then the ordering would
occasionally be wrong, causing linkage to fail.

This uses the topologically sorted list of libraries to collect native
libraries, so if a native library depends on libc it just needs to make sure
that the rust crate depends on liblibc.
  • Loading branch information
alexcrichton committed Apr 11, 2014
1 parent 8b6091e commit e6072fa
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/librustc/back/link.rs
Expand Up @@ -1487,15 +1487,24 @@ fn add_upstream_rust_crates(args: &mut Vec<~str>, sess: &Session,
// crate as well.
//
// The use case for this is a little subtle. In theory the native
// dependencies of a crate a purely an implementation detail of the crate
// dependencies of a crate are purely an implementation detail of the crate
// itself, but the problem arises with generic and inlined functions. If a
// generic function calls a native function, then the generic function must
// be instantiated in the target crate, meaning that the native symbol must
// also be resolved in the target crate.
fn add_upstream_native_libraries(args: &mut Vec<~str>, sess: &Session) {
let cstore = &sess.cstore;
cstore.iter_crate_data(|cnum, _| {
let libs = csearch::get_native_libraries(cstore, cnum);
// Be sure to use a topological sorting of crates becuase there may be
// interdependencies between native libraries. When passing -nodefaultlibs,
// for example, almost all native libraries depend on libc, so we have to
// make sure that's all the way at the right (liblibc is near the base of
// the dependency chain).
//
// This passes RequireStatic, but the actual requirement doesn't matter,
// we're just getting an ordering of crate numbers, we're not worried about
// the paths.
let crates = sess.cstore.get_used_crates(cstore::RequireStatic);
for (cnum, _) in crates.move_iter() {
let libs = csearch::get_native_libraries(&sess.cstore, cnum);
for &(kind, ref lib) in libs.iter() {
match kind {
cstore::NativeUnknown => args.push("-l" + *lib),
Expand All @@ -1508,5 +1517,5 @@ fn add_upstream_native_libraries(args: &mut Vec<~str>, sess: &Session) {
}
}
}
});
}
}
15 changes: 15 additions & 0 deletions src/test/run-make/interdependent-c-libraries/Makefile
@@ -0,0 +1,15 @@
-include ../tools.mk

# The rust crate foo will link to the native library foo, while the rust crate
# bar will link to the native library bar. There is also a dependency between
# the native library bar to the natibe library foo.
#
# This test ensures that the ordering of -lfoo and -lbar on the command line is
# correct to complete the linkage. If passed as "-lfoo -lbar", then the 'foo'
# library will be stripped out, and the linkage will fail.

all: $(call STATICLIB,foo) $(call STATICLIB,bar)
$(RUSTC) foo.rs
$(RUSTC) bar.rs
$(RUSTC) main.rs -Z print-link-args

3 changes: 3 additions & 0 deletions src/test/run-make/interdependent-c-libraries/bar.c
@@ -0,0 +1,3 @@
void foo();

void bar() { foo(); }
23 changes: 23 additions & 0 deletions src/test/run-make/interdependent-c-libraries/bar.rs
@@ -0,0 +1,23 @@
// Copyright 2014 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 = "rlib"]

extern crate foo;

#[link(name = "bar")]
extern {
fn bar();
}

pub fn doit() {
unsafe { bar(); }
}

1 change: 1 addition & 0 deletions src/test/run-make/interdependent-c-libraries/foo.c
@@ -0,0 +1 @@
void foo() {}
20 changes: 20 additions & 0 deletions src/test/run-make/interdependent-c-libraries/foo.rs
@@ -0,0 +1,20 @@
// Copyright 2014 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 = "rlib"]

#[link(name = "foo")]
extern {
fn foo();
}

pub fn doit() {
unsafe { foo(); }
}
16 changes: 16 additions & 0 deletions src/test/run-make/interdependent-c-libraries/main.rs
@@ -0,0 +1,16 @@
// Copyright 2014 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 foo;
extern crate bar;

fn main() {
bar::doit();
}

5 comments on commit e6072fa

@bors
Copy link
Contributor

@bors bors commented on e6072fa Apr 13, 2014

Choose a reason for hiding this comment

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

saw approval from brson
at alexcrichton@e6072fa

@bors
Copy link
Contributor

@bors bors commented on e6072fa Apr 13, 2014

Choose a reason for hiding this comment

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

merging alexcrichton/rust/c-linkage-oh-my = e6072fa into auto

@bors
Copy link
Contributor

@bors bors commented on e6072fa Apr 13, 2014

Choose a reason for hiding this comment

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

alexcrichton/rust/c-linkage-oh-my = e6072fa merged ok, testing candidate = 2f79054

@bors
Copy link
Contributor

@bors bors commented on e6072fa Apr 13, 2014

@bors
Copy link
Contributor

@bors bors commented on e6072fa Apr 13, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = 2f79054

Please sign in to comment.