Auto merge of #28605 - alexcrichton:link-native-first, r=brson

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
[breaking-change]

Author: bors

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