Auto merge of #57018 - dcreager:redundant-linker, r=alexcrichton

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

7 files changed, 75 insertions, 25 deletions

diff --git a/src/librustc_metadata/lib.rs b/src/librustc_metadata/lib.rs
index b4f68399d9f..14416b5ce07 100644
--- a/src/librustc_metadata/lib.rs
+++ b/src/librustc_metadata/lib.rs
@@ -1,6 +1,7 @@
 #![doc(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 80786992cd9..314c95a42be 100644
--- a/src/librustc_metadata/native_libs.rs
+++ b/src/librustc_metadata/native_libs.rs
@@ -199,34 +199,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 `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 {
+                        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::<Vec<_>>();
+            if existing.is_empty() {
                 // Add if not found
                 let new_name = new_name.as_ref().map(|s| &**s); // &Option<String> -> Option<&str>
                 let lib = NativeLibrary {
@@ -237,6 +234,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);
             }
         }
     }
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 00000000000..9486e07d21b
--- /dev/null
+++ b/src/test/run-make-fulldeps/redundant-libs/Makefile
@@ -0,0 +1,27 @@
+-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
+# 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 \
+    -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)
+
+endif
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 00000000000..e4259998678
--- /dev/null
+++ b/src/test/run-make-fulldeps/redundant-libs/bar.c
@@ -0,0 +1 @@
+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 00000000000..a4e2c2b717f
--- /dev/null
+++ b/src/test/run-make-fulldeps/redundant-libs/baz.c
@@ -0,0 +1,7 @@
+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 00000000000..339ee86c99e
--- /dev/null
+++ b/src/test/run-make-fulldeps/redundant-libs/foo.c
@@ -0,0 +1,2 @@
+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 00000000000..90d185ff51d
--- /dev/null
+++ b/src/test/run-make-fulldeps/redundant-libs/main.rs
@@ -0,0 +1,11 @@
+extern "C" {
+    fn bar();
+    fn baz();
+}
+
+fn main() {
+    unsafe {
+        bar();
+        baz();
+    }
+}