Skip to content

Commit 0acd11e

Browse files
committed
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 rust-lang#28595, and this change also invalidates the test case that was added for rust-lang#12446 which is now considered a bug because the downstream dependency would need to declare that it depends on the native library somehow. Closes rust-lang#28595
1 parent b2f379c commit 0acd11e

File tree

8 files changed

+60
-42
lines changed

8 files changed

+60
-42
lines changed

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.
+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) {}
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
}
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)