Skip to content

Commit 587be42

Browse files
borsbors
committed
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]
2 parents 031dd9c + 9502df5 commit 587be42

File tree

9 files changed

+72
-49
lines changed

9 files changed

+72
-49
lines changed

‎src/liballoc_jemalloc/lib.rs

+12-7
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,19 @@ extern crate libc;
2727

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

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

40-
// -lpthread needs to occur after -ljemalloc, the earlier argument isn't enough
41-
#[cfg(all(not(windows),
42-
not(target_os = "android"),
43-
not(target_env = "musl")))]
44-
#[link(name = "pthread")]
45-
extern {}
46-
4752
// The minimum alignment guaranteed by the architecture. This value is used to
4853
// add fast paths for low alignment values. In practice, the alignment is a
4954
// constant at the call site and the branch will be optimized out.

‎src/librustc_trans/back/link.rs

+14-21
Original file line numberDiff line numberDiff line change
@@ -984,31 +984,24 @@ fn link_args(cmd: &mut Linker,
984984
// such:
985985
//
986986
// 1. The local object that LLVM just generated
987-
// 2. Upstream rust libraries
988-
// 3. Local native libraries
987+
// 2. Local native libraries
988+
// 3. Upstream rust libraries
989989
// 4. Upstream native libraries
990990
//
991-
// This is generally fairly natural, but some may expect 2 and 3 to be
992-
// swapped. The reason that all native libraries are put last is that it's
993-
// not recommended for a native library to depend on a symbol from a rust
994-
// crate. If this is the case then a staticlib crate is recommended, solving
995-
// the problem.
991+
// The rationale behind this ordering is that those items lower down in the
992+
// list can't depend on items higher up in the list. For example nothing can
993+
// depend on what we just generated (e.g. that'd be a circular dependency).
994+
// Upstream rust libraries are not allowed to depend on our local native
995+
// libraries as that would violate the structure of the DAG, in that
996+
// scenario they are required to link to them as well in a shared fashion.
996997
//
997-
// Additionally, it is occasionally the case that upstream rust libraries
998-
// depend on a local native library. In the case of libraries such as
999-
// lua/glfw/etc the name of the library isn't the same across all platforms,
1000-
// so only the consumer crate of a library knows the actual name. This means
1001-
// that downstream crates will provide the #[link] attribute which upstream
1002-
// crates will depend on. Hence local native libraries are after out
1003-
// upstream rust crates.
1004-
//
1005-
// In theory this means that a symbol in an upstream native library will be
1006-
// shadowed by a local native library when it wouldn't have been before, but
1007-
// this kind of behavior is pretty platform specific and generally not
1008-
// recommended anyway, so I don't think we're shooting ourself in the foot
1009-
// much with that.
1010-
add_upstream_rust_crates(cmd, sess, dylib, tmpdir);
998+
// Note that upstream rust libraries may contain native dependencies as
999+
// well, but they also can't depend on what we just started to add to the
1000+
// link line. And finally upstream native libraries can't depend on anything
1001+
// in this DAG so far because they're only dylibs and dylibs can only depend
1002+
// on other dylibs (e.g. other native deps).
10111003
add_local_native_libraries(cmd, sess);
1004+
add_upstream_rust_crates(cmd, sess, dylib, tmpdir);
10121005
add_upstream_native_libraries(cmd, sess);
10131006

10141007
// # Telling the linker what we're doing

‎src/test/run-make/issue-12446/Makefile

-6
This file was deleted.

‎src/test/run-make/issue-12446/foo.c

-2
This file was deleted.

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

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-include ../tools.mk
2+
3+
all: $(call NATIVE_STATICLIB,a) $(call NATIVE_STATICLIB,b)
4+
$(RUSTC) a.rs
5+
$(RUSTC) b.rs
6+
$(call RUN,b)

‎src/test/run-make/issue-28595/a.c

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
void a(void) {}

‎src/test/run-make/issue-12446/foo.rs ‎src/test/run-make/issue-28595/a.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
22
// file at the top-level directory of this distribution and at
33
// http://rust-lang.org/COPYRIGHT.
44
//
@@ -10,10 +10,7 @@
1010

1111
#![crate_type = "rlib"]
1212

13+
#[link(name = "a", kind = "static")]
1314
extern {
14-
fn some_c_symbol();
15-
}
16-
17-
pub fn foo() {
18-
unsafe { some_c_symbol() }
15+
pub fn a();
1916
}

‎src/test/run-make/issue-12446/bar.rs ‎src/test/run-make/issue-28595/b.c

+5-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
22
// file at the top-level directory of this distribution and at
33
// http://rust-lang.org/COPYRIGHT.
44
//
@@ -8,11 +8,9 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
extern crate foo;
11+
extern void a(void);
1212

13-
#[link(name = "foo")]
14-
extern {}
15-
16-
fn main() {
17-
foo::foo();
13+
void b(void) {
14+
a();
1815
}
16+

‎src/test/run-make/issue-28595/b.rs

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
extern crate a;
12+
13+
#[link(name = "b", kind = "static")]
14+
extern {
15+
pub fn b();
16+
}
17+
18+
19+
fn main() {
20+
unsafe { b(); }
21+
}

0 commit comments

Comments
 (0)