From d445e1ccaa18e501870dd4cf52f41980b7260c2c Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Thu, 20 Dec 2018 15:46:42 -0500 Subject: [PATCH 1/7] Keep last redundant linker flag, not first When a library (L1) is passed to the linker multiple times, this is sometimes purposeful: there might be several other libraries in the linker command (L2 and L3) that all depend on L1. You'd end up with a (simplified) linker command that looks like: -l2 -l1 -l3 -l1 With the previous behavior, when rustc encountered a redundant library, it would keep the first instance, and remove the later ones, resulting in: -l2 -l1 -l3 This can cause a linker error, because on some platforms (e.g. Linux), the linker will only include symbols from L1 that are needed *at the point it's referenced in the command line*. So if L3 depends on additional symbols from L1, which aren't needed by L2, the linker won't know to include them, and you'll end up with "undefined symbols" errors. A better behavior is to keep the *last* instance of the library: -l2 -l3 -l1 This ensures that all "downstream" libraries have been included in the linker command before the "upstream" library is referenced. Fixes rust-lang#47989 --- src/librustc_metadata/lib.rs | 1 + src/librustc_metadata/native_libs.rs | 51 ++++++++++++++-------------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/librustc_metadata/lib.rs b/src/librustc_metadata/lib.rs index ee99f7465b905..480c344f38123 100644 --- a/src/librustc_metadata/lib.rs +++ b/src/librustc_metadata/lib.rs @@ -13,6 +13,7 @@ html_root_url = "https://doc.rust-lang.org/nightly/")] #![feature(box_patterns)] +#![feature(drain_filter)] #![feature(libc)] #![feature(nll)] #![feature(proc_macro_internals)] diff --git a/src/librustc_metadata/native_libs.rs b/src/librustc_metadata/native_libs.rs index 6f85418b297ed..1ea4e4370eb1b 100644 --- a/src/librustc_metadata/native_libs.rs +++ b/src/librustc_metadata/native_libs.rs @@ -208,34 +208,31 @@ impl<'a, 'tcx> Collector<'a, 'tcx> { } // Update kind and, optionally, the name of all native libraries - // (there may be more than one) with the specified name. + // (there may be more than one) with the specified name. If any + // library is mentioned more than once, keep the latest mention + // of it, so that any possible dependent libraries appear before + // it. (This ensures that the linker is able to see symbols from + // all possible dependent libraries before linking in the library + // in question.) for &(ref name, ref new_name, kind) in &self.tcx.sess.opts.libs { - let mut found = false; - for lib in self.libs.iter_mut() { - let lib_name = match lib.name { - Some(n) => n, - None => continue, - }; - if lib_name == name as &str { - let mut changed = false; - if let Some(k) = kind { - lib.kind = k; - changed = true; - } - if let &Some(ref new_name) = new_name { - lib.name = Some(Symbol::intern(new_name)); - changed = true; - } - if !changed { - let msg = format!("redundant linker flag specified for \ - library `{}`", name); - self.tcx.sess.warn(&msg); + // If we've already added any native libraries with the same + // name, they will be pulled out into `moved`, so that we can + // move them to the end of the list below. + let mut existing = self.libs.drain_filter(|lib| { + if let Some(lib_name) = lib.name { + if lib_name == name as &str { + if let Some(k) = kind { + lib.kind = k; + } + if let &Some(ref new_name) = new_name { + lib.name = Some(Symbol::intern(new_name)); + } + return true; } - - found = true; } - } - if !found { + false + }).collect::>(); + if existing.is_empty() { // Add if not found let new_name = new_name.as_ref().map(|s| &**s); // &Option -> Option<&str> let lib = NativeLibrary { @@ -246,6 +243,10 @@ impl<'a, 'tcx> Collector<'a, 'tcx> { wasm_import_module: None, }; self.register_native_lib(None, lib); + } else { + // Move all existing libraries with the same name to the + // end of the command line. + self.libs.append(&mut existing); } } } From c56f12833837648d1a4ef40b714efea9c309f8c4 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Thu, 20 Dec 2018 16:19:55 -0500 Subject: [PATCH 2/7] Fix typo in comment --- src/librustc_metadata/native_libs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_metadata/native_libs.rs b/src/librustc_metadata/native_libs.rs index 1ea4e4370eb1b..2c6ef4baa55e5 100644 --- a/src/librustc_metadata/native_libs.rs +++ b/src/librustc_metadata/native_libs.rs @@ -216,8 +216,8 @@ impl<'a, 'tcx> Collector<'a, 'tcx> { // in question.) for &(ref name, ref new_name, kind) in &self.tcx.sess.opts.libs { // If we've already added any native libraries with the same - // name, they will be pulled out into `moved`, so that we can - // move them to the end of the list below. + // name, they will be pulled out into `existing`, so that we + // can move them to the end of the list below. let mut existing = self.libs.drain_filter(|lib| { if let Some(lib_name) = lib.name { if lib_name == name as &str { From c641a8c424e0d3fea16f41863d216c1ec0642c82 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Thu, 7 Mar 2019 15:12:48 -0500 Subject: [PATCH 3/7] Add run-make-fulldeps test case --- .../run-make-fulldeps/redundant-libs/Makefile | 11 +++++++ .../run-make-fulldeps/redundant-libs/bar.c | 12 ++++++++ .../run-make-fulldeps/redundant-libs/baz.c | 17 +++++++++++ .../run-make-fulldeps/redundant-libs/foo.c | 12 ++++++++ .../run-make-fulldeps/redundant-libs/main.rs | 30 +++++++++++++++++++ 5 files changed, 82 insertions(+) create mode 100644 src/test/run-make-fulldeps/redundant-libs/Makefile create mode 100644 src/test/run-make-fulldeps/redundant-libs/bar.c create mode 100644 src/test/run-make-fulldeps/redundant-libs/baz.c create mode 100644 src/test/run-make-fulldeps/redundant-libs/foo.c create mode 100644 src/test/run-make-fulldeps/redundant-libs/main.rs diff --git a/src/test/run-make-fulldeps/redundant-libs/Makefile b/src/test/run-make-fulldeps/redundant-libs/Makefile new file mode 100644 index 0000000000000..f756dc1659078 --- /dev/null +++ b/src/test/run-make-fulldeps/redundant-libs/Makefile @@ -0,0 +1,11 @@ +-include ../tools.mk +RUSTC_FLAGS = \ + -l static=bar \ + -l foo \ + -l static=baz \ + -l foo \ + -Z print-link-args + +all: $(call DYLIB,foo) $(call STATICLIB,bar) $(call STATICLIB,baz) + $(RUSTC) $(RUSTC_FLAGS) main.rs + $(call RUN,main) diff --git a/src/test/run-make-fulldeps/redundant-libs/bar.c b/src/test/run-make-fulldeps/redundant-libs/bar.c new file mode 100644 index 0000000000000..78ddeb454db8b --- /dev/null +++ b/src/test/run-make-fulldeps/redundant-libs/bar.c @@ -0,0 +1,12 @@ +// Copyright 2019 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 bar() { +} diff --git a/src/test/run-make-fulldeps/redundant-libs/baz.c b/src/test/run-make-fulldeps/redundant-libs/baz.c new file mode 100644 index 0000000000000..4ec0e9adc67c4 --- /dev/null +++ b/src/test/run-make-fulldeps/redundant-libs/baz.c @@ -0,0 +1,17 @@ +// Copyright 2019 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 void foo1(); +extern void foo2(); + +void baz() { + foo1(); + foo2(); +} diff --git a/src/test/run-make-fulldeps/redundant-libs/foo.c b/src/test/run-make-fulldeps/redundant-libs/foo.c new file mode 100644 index 0000000000000..d5b28210dff68 --- /dev/null +++ b/src/test/run-make-fulldeps/redundant-libs/foo.c @@ -0,0 +1,12 @@ +// Copyright 2019 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 foo1() {} +void foo2() {} diff --git a/src/test/run-make-fulldeps/redundant-libs/main.rs b/src/test/run-make-fulldeps/redundant-libs/main.rs new file mode 100644 index 0000000000000..796534d8116d7 --- /dev/null +++ b/src/test/run-make-fulldeps/redundant-libs/main.rs @@ -0,0 +1,30 @@ +// Copyright 2019 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. + +// rustc will remove one of the two redundant references to foo below. Depending on which one gets +// removed, we'll get a linker error on SOME platforms (like Linux). On these platforms, when a +// library is referenced, the linker will only pull in the symbols needed _at that point in time_. +// If a later library depends on additional symbols from the library, they will not have been +// pulled in, and you'll get undefined symbols errors. +// +// So in this example, we need to ensure that rustc keeps the _later_ reference to foo, and not the +// former one. + +extern "C" { + fn bar(); + fn baz(); +} + +fn main() { + unsafe { + bar(); + baz(); + } +} From 32969e9f57a7fc472addd40eb0d0fb7e7b00888f Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 12 Mar 2019 16:01:13 -0400 Subject: [PATCH 4/7] Move comment describing test case --- src/test/run-make-fulldeps/redundant-libs/Makefile | 10 ++++++++++ src/test/run-make-fulldeps/redundant-libs/main.rs | 9 --------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/test/run-make-fulldeps/redundant-libs/Makefile b/src/test/run-make-fulldeps/redundant-libs/Makefile index f756dc1659078..9a64f4ed07c0b 100644 --- a/src/test/run-make-fulldeps/redundant-libs/Makefile +++ b/src/test/run-make-fulldeps/redundant-libs/Makefile @@ -1,4 +1,14 @@ -include ../tools.mk + +# rustc will remove one of the two redundant references to foo below. Depending +# on which one gets removed, we'll get a linker error on SOME platforms (like +# Linux). On these platforms, when a library is referenced, the linker will +# only pull in the symbols needed _at that point in time_. If a later library +# depends on additional symbols from the library, they will not have been pulled +# in, and you'll get undefined symbols errors. +# +# So in this example, we need to ensure that rustc keeps the _later_ reference +# to foo, and not the former one. RUSTC_FLAGS = \ -l static=bar \ -l foo \ diff --git a/src/test/run-make-fulldeps/redundant-libs/main.rs b/src/test/run-make-fulldeps/redundant-libs/main.rs index 796534d8116d7..aeae38f096ef5 100644 --- a/src/test/run-make-fulldeps/redundant-libs/main.rs +++ b/src/test/run-make-fulldeps/redundant-libs/main.rs @@ -8,15 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// rustc will remove one of the two redundant references to foo below. Depending on which one gets -// removed, we'll get a linker error on SOME platforms (like Linux). On these platforms, when a -// library is referenced, the linker will only pull in the symbols needed _at that point in time_. -// If a later library depends on additional symbols from the library, they will not have been -// pulled in, and you'll get undefined symbols errors. -// -// So in this example, we need to ensure that rustc keeps the _later_ reference to foo, and not the -// former one. - extern "C" { fn bar(); fn baz(); From 8b6c566b9d405268312b32059f2f4a1cd3c24dfc Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 15 Mar 2019 10:49:36 -0400 Subject: [PATCH 5/7] Remove copyright notices --- src/test/run-make-fulldeps/redundant-libs/bar.c | 13 +------------ src/test/run-make-fulldeps/redundant-libs/baz.c | 10 ---------- src/test/run-make-fulldeps/redundant-libs/foo.c | 10 ---------- src/test/run-make-fulldeps/redundant-libs/main.rs | 10 ---------- 4 files changed, 1 insertion(+), 42 deletions(-) diff --git a/src/test/run-make-fulldeps/redundant-libs/bar.c b/src/test/run-make-fulldeps/redundant-libs/bar.c index 78ddeb454db8b..e42599986781f 100644 --- a/src/test/run-make-fulldeps/redundant-libs/bar.c +++ b/src/test/run-make-fulldeps/redundant-libs/bar.c @@ -1,12 +1 @@ -// Copyright 2019 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 bar() { -} +void bar() {} diff --git a/src/test/run-make-fulldeps/redundant-libs/baz.c b/src/test/run-make-fulldeps/redundant-libs/baz.c index 4ec0e9adc67c4..a4e2c2b717fdb 100644 --- a/src/test/run-make-fulldeps/redundant-libs/baz.c +++ b/src/test/run-make-fulldeps/redundant-libs/baz.c @@ -1,13 +1,3 @@ -// Copyright 2019 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 void foo1(); extern void foo2(); diff --git a/src/test/run-make-fulldeps/redundant-libs/foo.c b/src/test/run-make-fulldeps/redundant-libs/foo.c index d5b28210dff68..339ee86c99eae 100644 --- a/src/test/run-make-fulldeps/redundant-libs/foo.c +++ b/src/test/run-make-fulldeps/redundant-libs/foo.c @@ -1,12 +1,2 @@ -// Copyright 2019 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 foo1() {} void foo2() {} diff --git a/src/test/run-make-fulldeps/redundant-libs/main.rs b/src/test/run-make-fulldeps/redundant-libs/main.rs index aeae38f096ef5..90d185ff51dbd 100644 --- a/src/test/run-make-fulldeps/redundant-libs/main.rs +++ b/src/test/run-make-fulldeps/redundant-libs/main.rs @@ -1,13 +1,3 @@ -// Copyright 2019 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 "C" { fn bar(); fn baz(); From b58e19db30906212eafb457243d832eebf4630df Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 19 Mar 2019 11:56:32 -0400 Subject: [PATCH 6/7] Explicitly prefer dynamic linking in test case --- src/test/run-make-fulldeps/redundant-libs/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/run-make-fulldeps/redundant-libs/Makefile b/src/test/run-make-fulldeps/redundant-libs/Makefile index 9a64f4ed07c0b..25643e50df692 100644 --- a/src/test/run-make-fulldeps/redundant-libs/Makefile +++ b/src/test/run-make-fulldeps/redundant-libs/Makefile @@ -10,6 +10,7 @@ # So in this example, we need to ensure that rustc keeps the _later_ reference # to foo, and not the former one. RUSTC_FLAGS = \ + -C prefer-dynamic \ -l static=bar \ -l foo \ -l static=baz \ From 32d99efa403a5c3ba93d3389110cd9f4226591d2 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 19 Mar 2019 14:53:19 -0400 Subject: [PATCH 7/7] Ignore test on Windows --- src/test/run-make-fulldeps/redundant-libs/Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/run-make-fulldeps/redundant-libs/Makefile b/src/test/run-make-fulldeps/redundant-libs/Makefile index 25643e50df692..9486e07d21bf7 100644 --- a/src/test/run-make-fulldeps/redundant-libs/Makefile +++ b/src/test/run-make-fulldeps/redundant-libs/Makefile @@ -1,5 +1,9 @@ -include ../tools.mk +ifdef IS_WINDOWS +all: +else + # rustc will remove one of the two redundant references to foo below. Depending # on which one gets removed, we'll get a linker error on SOME platforms (like # Linux). On these platforms, when a library is referenced, the linker will @@ -10,7 +14,6 @@ # So in this example, we need to ensure that rustc keeps the _later_ reference # to foo, and not the former one. RUSTC_FLAGS = \ - -C prefer-dynamic \ -l static=bar \ -l foo \ -l static=baz \ @@ -20,3 +23,5 @@ RUSTC_FLAGS = \ all: $(call DYLIB,foo) $(call STATICLIB,bar) $(call STATICLIB,baz) $(RUSTC) $(RUSTC_FLAGS) main.rs $(call RUN,main) + +endif