Skip to content

Commit

Permalink
rustc: Swap link order of native libs/rust deps
Browse files Browse the repository at this point in the history
This commit swaps the order of linking local native libraries and upstream
native libraries on the linker command line. Detail of bugs this can cause can
be found in #28595, and this change also invalidates the test case that was
added for #12446 which is now considered a bug because the downstream dependency
would need to declare that it depends on the native library somehow.

Closes #28595
  • Loading branch information
alexcrichton committed Oct 1, 2015
1 parent 1c788d0 commit 9502df5
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 49 deletions.
19 changes: 12 additions & 7 deletions src/liballoc_jemalloc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,19 @@ extern crate libc;

use libc::{c_int, c_void, size_t};

// Linkage directives to pull in jemalloc and its dependencies.
//
// On some platforms we need to be sure to link in `pthread` which jemalloc
// depends on, and specifically on android we need to also link to libgcc.
// Currently jemalloc is compiled with gcc which will generate calls to
// intrinsics that are libgcc specific (e.g. those intrinsics aren't present in
// libcompiler-rt), so link that in to get that support.
#[link(name = "jemalloc", kind = "static")]
#[cfg_attr(target_os = "android", link(name = "gcc"))]
#[cfg_attr(all(not(windows),
not(target_os = "android"),
not(target_env = "musl")),
link(name = "pthread"))]
extern {
fn je_mallocx(size: size_t, flags: c_int) -> *mut c_void;
fn je_rallocx(ptr: *mut c_void, size: size_t, flags: c_int) -> *mut c_void;
Expand All @@ -37,13 +49,6 @@ extern {
fn je_nallocx(size: size_t, flags: c_int) -> size_t;
}

// -lpthread needs to occur after -ljemalloc, the earlier argument isn't enough
#[cfg(all(not(windows),
not(target_os = "android"),
not(target_env = "musl")))]
#[link(name = "pthread")]
extern {}

// The minimum alignment guaranteed by the architecture. This value is used to
// add fast paths for low alignment values. In practice, the alignment is a
// constant at the call site and the branch will be optimized out.
Expand Down
35 changes: 14 additions & 21 deletions src/librustc_trans/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -984,31 +984,24 @@ fn link_args(cmd: &mut Linker,
// such:
//
// 1. The local object that LLVM just generated
// 2. Upstream rust libraries
// 3. Local native libraries
// 2. Local native libraries
// 3. Upstream rust libraries
// 4. Upstream native libraries
//
// This is generally fairly natural, but some may expect 2 and 3 to be
// swapped. The reason that all native libraries are put last is that it's
// not recommended for a native library to depend on a symbol from a rust
// crate. If this is the case then a staticlib crate is recommended, solving
// the problem.
// The rationale behind this ordering is that those items lower down in the
// list can't depend on items higher up in the list. For example nothing can
// depend on what we just generated (e.g. that'd be a circular dependency).
// Upstream rust libraries are not allowed to depend on our local native
// libraries as that would violate the structure of the DAG, in that
// scenario they are required to link to them as well in a shared fashion.
//
// Additionally, it is occasionally the case that upstream rust libraries
// depend on a local native library. In the case of libraries such as
// lua/glfw/etc the name of the library isn't the same across all platforms,
// so only the consumer crate of a library knows the actual name. This means
// that downstream crates will provide the #[link] attribute which upstream
// crates will depend on. Hence local native libraries are after out
// upstream rust crates.
//
// In theory this means that a symbol in an upstream native library will be
// shadowed by a local native library when it wouldn't have been before, but
// this kind of behavior is pretty platform specific and generally not
// recommended anyway, so I don't think we're shooting ourself in the foot
// much with that.
add_upstream_rust_crates(cmd, sess, dylib, tmpdir);
// Note that upstream rust libraries may contain native dependencies as
// well, but they also can't depend on what we just started to add to the
// link line. And finally upstream native libraries can't depend on anything
// in this DAG so far because they're only dylibs and dylibs can only depend
// on other dylibs (e.g. other native deps).
add_local_native_libraries(cmd, sess);
add_upstream_rust_crates(cmd, sess, dylib, tmpdir);
add_upstream_native_libraries(cmd, sess);

// # Telling the linker what we're doing
Expand Down
6 changes: 0 additions & 6 deletions src/test/run-make/issue-12446/Makefile

This file was deleted.

2 changes: 0 additions & 2 deletions src/test/run-make/issue-12446/foo.c

This file was deleted.

6 changes: 6 additions & 0 deletions src/test/run-make/issue-28595/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-include ../tools.mk

all: $(call NATIVE_STATICLIB,a) $(call NATIVE_STATICLIB,b)
$(RUSTC) a.rs
$(RUSTC) b.rs
$(call RUN,b)
11 changes: 11 additions & 0 deletions src/test/run-make/issue-28595/a.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright 2015 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.

void a(void) {}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
Expand All @@ -10,10 +10,7 @@

#![crate_type = "rlib"]

#[link(name = "a", kind = "static")]
extern {
fn some_c_symbol();
}

pub fn foo() {
unsafe { some_c_symbol() }
pub fn a();
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
Expand All @@ -8,11 +8,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

extern crate foo;
extern void a(void);

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

fn main() {
foo::foo();
void b(void) {
a();
}

21 changes: 21 additions & 0 deletions src/test/run-make/issue-28595/b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2015 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 a;

#[link(name = "b", kind = "static")]
extern {
pub fn b();
}


fn main() {
unsafe { b(); }
}

0 comments on commit 9502df5

Please sign in to comment.