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

Author: alexcrichton

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.

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

src/test/run-make/issue-12446/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)

src/test/run-make/issue-12446/foo.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 <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) {}

src/test/run-make/issue-28595/Makefile

@@ -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();
 }

src/test/run-make/issue-28595/a.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();
 }
+

src/test/run-make/issue-12446/foo.rs renamed to src/test/run-make/issue-28595/a.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 <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(); }
+}