From 9502df5798f2d7de41fe59927fddd894f7769a73 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 23 Sep 2015 09:23:31 -0700 Subject: [PATCH] rustc: Swap link order of native libs/rust deps 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 --- src/liballoc_jemalloc/lib.rs | 19 ++++++---- src/librustc_trans/back/link.rs | 35 ++++++++----------- src/test/run-make/issue-12446/Makefile | 6 ---- src/test/run-make/issue-12446/foo.c | 2 -- src/test/run-make/issue-28595/Makefile | 6 ++++ src/test/run-make/issue-28595/a.c | 11 ++++++ .../{issue-12446/foo.rs => issue-28595/a.rs} | 9 ++--- .../{issue-12446/bar.rs => issue-28595/b.c} | 12 +++---- src/test/run-make/issue-28595/b.rs | 21 +++++++++++ 9 files changed, 72 insertions(+), 49 deletions(-) delete mode 100644 src/test/run-make/issue-12446/Makefile delete mode 100644 src/test/run-make/issue-12446/foo.c create mode 100644 src/test/run-make/issue-28595/Makefile create mode 100644 src/test/run-make/issue-28595/a.c rename src/test/run-make/{issue-12446/foo.rs => issue-28595/a.rs} (76%) rename src/test/run-make/{issue-12446/bar.rs => issue-28595/b.c} (74%) create mode 100644 src/test/run-make/issue-28595/b.rs diff --git a/src/liballoc_jemalloc/lib.rs b/src/liballoc_jemalloc/lib.rs index 4179cbe8a7bac..80a831fd20775 100644 --- a/src/liballoc_jemalloc/lib.rs +++ b/src/liballoc_jemalloc/lib.rs @@ -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; @@ -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. diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 4e493e6779c66..2e18b50a45cfe 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -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 diff --git a/src/test/run-make/issue-12446/Makefile b/src/test/run-make/issue-12446/Makefile deleted file mode 100644 index c412b0479fb84..0000000000000 --- a/src/test/run-make/issue-12446/Makefile +++ /dev/null @@ -1,6 +0,0 @@ --include ../tools.mk - -all: $(call NATIVE_STATICLIB,foo) - $(RUSTC) foo.rs - $(RUSTC) bar.rs - $(call RUN,bar) diff --git a/src/test/run-make/issue-12446/foo.c b/src/test/run-make/issue-12446/foo.c deleted file mode 100644 index 186a0046e80ac..0000000000000 --- a/src/test/run-make/issue-12446/foo.c +++ /dev/null @@ -1,2 +0,0 @@ -// ignore-license -void some_c_symbol() {} diff --git a/src/test/run-make/issue-28595/Makefile b/src/test/run-make/issue-28595/Makefile new file mode 100644 index 0000000000000..61e9d0c654735 --- /dev/null +++ b/src/test/run-make/issue-28595/Makefile @@ -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) diff --git a/src/test/run-make/issue-28595/a.c b/src/test/run-make/issue-28595/a.c new file mode 100644 index 0000000000000..feacd7bc31377 --- /dev/null +++ b/src/test/run-make/issue-28595/a.c @@ -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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +void a(void) {} diff --git a/src/test/run-make/issue-12446/foo.rs b/src/test/run-make/issue-28595/a.rs similarity index 76% rename from src/test/run-make/issue-12446/foo.rs rename to src/test/run-make/issue-28595/a.rs index 11c61169de97b..7377a9f3416f8 100644 --- a/src/test/run-make/issue-12446/foo.rs +++ b/src/test/run-make/issue-28595/a.rs @@ -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. // @@ -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(); } diff --git a/src/test/run-make/issue-12446/bar.rs b/src/test/run-make/issue-28595/b.c similarity index 74% rename from src/test/run-make/issue-12446/bar.rs rename to src/test/run-make/issue-28595/b.c index cd41058744d5a..de81fbcaa6095 100644 --- a/src/test/run-make/issue-12446/bar.rs +++ b/src/test/run-make/issue-28595/b.c @@ -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. // @@ -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(); } + diff --git a/src/test/run-make/issue-28595/b.rs b/src/test/run-make/issue-28595/b.rs new file mode 100644 index 0000000000000..37ff346c3f3f3 --- /dev/null +++ b/src/test/run-make/issue-28595/b.rs @@ -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 or the MIT license +// , 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(); } +}