Auto merge of #126094 - petrochenkov:libsearch, r=michaelwoerister

linker: Link dylib crates by path

Linkers seem to support linking dynamic libraries by path.
Not sure why the previous scheme with splitting the path into a directory (passed with `-L`) and a name (passed with `-l`) was used (upd: likely due to #126094 (comment)).

When we split a library path `some/dir/libfoo.so` into `-L some/dir` and `-l foo` we add `some/dir` to search directories for *all* libraries looked up by the linker, not just `foo`, and `foo` is also looked up in *all* search directories not just `some/dir`.
Technically we may find some unintended libraries this way.
Therefore linking dylibs via a full path is both simpler and more reliable.

It also makes the set of search directories more easily reproducible when we need to lookup some native library manually (like in #123436).

Author: bors

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -2817,6 +2817,15 @@ fn rehome_sysroot_lib_dir(sess: &Session, lib_dir: &Path) -> PathBuf {
     }
 }
 
+fn rehome_lib_path(sess: &Session, path: &Path) -> PathBuf {
+    if let Some(dir) = path.parent() {
+        let file_name = path.file_name().expect("library path has no file name component");
+        rehome_sysroot_lib_dir(sess, dir).join(file_name)
+    } else {
+        fix_windows_verbatim_for_gcc(path)
+    }
+}
+
 // Adds the static "rlib" versions of all crates to the command line.
 // There's a bit of magic which happens here specifically related to LTO,
 // namely that we remove upstream object files.
@@ -2847,15 +2856,8 @@ fn add_static_crate(
     let src = &codegen_results.crate_info.used_crate_source[&cnum];
     let cratepath = &src.rlib.as_ref().unwrap().0;
 
-    let mut link_upstream = |path: &Path| {
-        let rlib_path = if let Some(dir) = path.parent() {
-            let file_name = path.file_name().expect("rlib path has no file name path component");
-            rehome_sysroot_lib_dir(sess, dir).join(file_name)
-        } else {
-            fix_windows_verbatim_for_gcc(path)
-        };
-        cmd.link_staticlib_by_path(&rlib_path, false);
-    };
+    let mut link_upstream =
+        |path: &Path| cmd.link_staticlib_by_path(&rehome_lib_path(sess, path), false);
 
     if !are_upstream_rust_objects_already_included(sess)
         || ignored_for_lto(sess, &codegen_results.crate_info, cnum)
@@ -2919,27 +2921,7 @@ fn add_static_crate(
 
 // Same thing as above, but for dynamic crates instead of static crates.
 fn add_dynamic_crate(cmd: &mut dyn Linker, sess: &Session, cratepath: &Path) {
-    // Just need to tell the linker about where the library lives and
-    // what its name is
-    let parent = cratepath.parent();
-    // When producing a dll, the MSVC linker may not actually emit a
-    // `foo.lib` file if the dll doesn't actually export any symbols, so we
-    // check to see if the file is there and just omit linking to it if it's
-    // not present.
-    if sess.target.is_like_msvc && !cratepath.with_extension("dll.lib").exists() {
-        return;
-    }
-    if let Some(dir) = parent {
-        cmd.include_path(&rehome_sysroot_lib_dir(sess, dir));
-    }
-    // "<dir>/name.dll -> name.dll" on windows-msvc
-    // "<dir>/name.dll -> name" on windows-gnu
-    // "<dir>/libname.<ext> -> name" elsewhere
-    let stem = if sess.target.is_like_msvc { cratepath.file_name() } else { cratepath.file_stem() };
-    let stem = stem.unwrap().to_str().unwrap();
-    // Convert library file-stem into a cc -l argument.
-    let prefix = if stem.starts_with("lib") && !sess.target.is_like_windows { 3 } else { 0 };
-    cmd.link_dylib_by_name(&stem[prefix..], false, true);
+    cmd.link_dylib_by_path(&rehome_lib_path(sess, cratepath), true);
 }
 
 fn relevant_lib(sess: &Session, lib: &NativeLib) -> bool {

compiler/rustc_codegen_ssa/src/back/linker.rs

@@ -268,7 +268,12 @@ pub trait Linker {
         false
     }
     fn set_output_kind(&mut self, output_kind: LinkOutputKind, out_filename: &Path);
-    fn link_dylib_by_name(&mut self, name: &str, verbatim: bool, as_needed: bool);
+    fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) {
+        bug!("dylib linked with unsupported linker")
+    }
+    fn link_dylib_by_path(&mut self, _path: &Path, _as_needed: bool) {
+        bug!("dylib linked with unsupported linker")
+    }
     fn link_framework_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) {
         bug!("framework linked with unsupported linker")
     }
@@ -403,28 +408,53 @@ impl<'a> GccLinker<'a> {
             }
         } else {
             self.link_or_cc_arg("-shared");
-            if self.sess.target.is_like_windows {
-                // The output filename already contains `dll_suffix` so
-                // the resulting import library will have a name in the
-                // form of libfoo.dll.a
-                let implib_name =
-                    out_filename.file_name().and_then(|file| file.to_str()).map(|file| {
-                        format!(
-                            "{}{}{}",
-                            self.sess.target.staticlib_prefix,
-                            file,
-                            self.sess.target.staticlib_suffix
-                        )
-                    });
-                if let Some(implib_name) = implib_name {
-                    let implib = out_filename.parent().map(|dir| dir.join(&implib_name));
-                    if let Some(implib) = implib {
-                        self.link_arg(&format!("--out-implib={}", (*implib).to_str().unwrap()));
-                    }
+            if let Some(name) = out_filename.file_name() {
+                if self.sess.target.is_like_windows {
+                    // The output filename already contains `dll_suffix` so
+                    // the resulting import library will have a name in the
+                    // form of libfoo.dll.a
+                    let mut implib_name = OsString::from(&*self.sess.target.staticlib_prefix);
+                    implib_name.push(name);
+                    implib_name.push(&*self.sess.target.staticlib_suffix);
+                    let mut out_implib = OsString::from("--out-implib=");
+                    out_implib.push(out_filename.with_file_name(implib_name));
+                    self.link_arg(out_implib);
+                } else {
+                    // When dylibs are linked by a full path this value will get into `DT_NEEDED`
+                    // instead of the full path, so the library can be later found in some other
+                    // location than that specific path.
+                    let mut soname = OsString::from("-soname=");
+                    soname.push(name);
+                    self.link_arg(soname);
                 }
             }
         }
     }
+
+    fn with_as_needed(&mut self, as_needed: bool, f: impl FnOnce(&mut Self)) {
+        if !as_needed {
+            if self.sess.target.is_like_osx {
+                // FIXME(81490): ld64 doesn't support these flags but macOS 11
+                // has -needed-l{} / -needed_library {}
+                // but we have no way to detect that here.
+                self.sess.dcx().emit_warn(errors::Ld64UnimplementedModifier);
+            } else if self.is_gnu && !self.sess.target.is_like_windows {
+                self.link_arg("--no-as-needed");
+            } else {
+                self.sess.dcx().emit_warn(errors::LinkerUnsupportedModifier);
+            }
+        }
+
+        f(self);
+
+        if !as_needed {
+            if self.sess.target.is_like_osx {
+                // See above FIXME comment
+            } else if self.is_gnu && !self.sess.target.is_like_windows {
+                self.link_arg("--as-needed");
+            }
+        }
+    }
 }
 
 impl<'a> Linker for GccLinker<'a> {
@@ -506,27 +536,18 @@ impl<'a> Linker for GccLinker<'a> {
             // to the linker.
             return;
         }
-        if !as_needed {
-            if self.sess.target.is_like_osx {
-                // FIXME(81490): ld64 doesn't support these flags but macOS 11
-                // has -needed-l{} / -needed_library {}
-                // but we have no way to detect that here.
-                self.sess.dcx().emit_warn(errors::Ld64UnimplementedModifier);
-            } else if self.is_gnu && !self.sess.target.is_like_windows {
-                self.link_arg("--no-as-needed");
-            } else {
-                self.sess.dcx().emit_warn(errors::LinkerUnsupportedModifier);
-            }
-        }
         self.hint_dynamic();
-        self.link_or_cc_arg(format!("-l{}{name}", if verbatim && self.is_gnu { ":" } else { "" },));
-        if !as_needed {
-            if self.sess.target.is_like_osx {
-                // See above FIXME comment
-            } else if self.is_gnu && !self.sess.target.is_like_windows {
-                self.link_arg("--as-needed");
-            }
-        }
+        self.with_as_needed(as_needed, |this| {
+            let colon = if verbatim && this.is_gnu { ":" } else { "" };
+            this.link_or_cc_arg(format!("-l{colon}{name}"));
+        });
+    }
+
+    fn link_dylib_by_path(&mut self, path: &Path, as_needed: bool) {
+        self.hint_dynamic();
+        self.with_as_needed(as_needed, |this| {
+            this.link_or_cc_arg(path);
+        })
     }
 
     fn link_framework_by_name(&mut self, name: &str, _verbatim: bool, as_needed: bool) {
@@ -861,6 +882,15 @@ impl<'a> Linker for MsvcLinker<'a> {
         self.link_arg(format!("{}{}", name, if verbatim { "" } else { ".lib" }));
     }
 
+    fn link_dylib_by_path(&mut self, path: &Path, _as_needed: bool) {
+        // When producing a dll, MSVC linker may not emit an implib file if the dll doesn't export
+        // any symbols, so we skip linking if the implib file is not present.
+        let implib_path = path.with_extension("dll.lib");
+        if implib_path.exists() {
+            self.link_or_cc_arg(implib_path);
+        }
+    }
+
     fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool, whole_archive: bool) {
         let prefix = if whole_archive { "/WHOLEARCHIVE:" } else { "" };
         let suffix = if verbatim { "" } else { ".lib" };
@@ -1083,6 +1113,10 @@ impl<'a> Linker for EmLinker<'a> {
         self.link_or_cc_args(&["-l", name]);
     }
 
+    fn link_dylib_by_path(&mut self, path: &Path, _as_needed: bool) {
+        self.link_or_cc_arg(path);
+    }
+
     fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool, _whole_archive: bool) {
         self.link_or_cc_args(&["-l", name]);
     }
@@ -1240,6 +1274,10 @@ impl<'a> Linker for WasmLd<'a> {
         self.link_or_cc_args(&["-l", name]);
     }
 
+    fn link_dylib_by_path(&mut self, path: &Path, _as_needed: bool) {
+        self.link_or_cc_arg(path);
+    }
+
     fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool, whole_archive: bool) {
         if !whole_archive {
             self.link_or_cc_args(&["-l", name]);
@@ -1368,10 +1406,6 @@ impl<'a> Linker for L4Bender<'a> {
 
     fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {}
 
-    fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) {
-        bug!("dylibs are not supported on L4Re");
-    }
-
     fn link_staticlib_by_name(&mut self, name: &str, _verbatim: bool, whole_archive: bool) {
         self.hint_static();
         if !whole_archive {
@@ -1536,6 +1570,11 @@ impl<'a> Linker for AixLinker<'a> {
         self.link_or_cc_arg(format!("-l{name}"));
     }
 
+    fn link_dylib_by_path(&mut self, path: &Path, _as_needed: bool) {
+        self.hint_dynamic();
+        self.link_or_cc_arg(path);
+    }
+
     fn link_staticlib_by_name(&mut self, name: &str, verbatim: bool, whole_archive: bool) {
         self.hint_static();
         if !whole_archive {
@@ -1721,10 +1760,6 @@ impl<'a> Linker for PtxLinker<'a> {
 
     fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {}
 
-    fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) {
-        panic!("external dylibs not supported")
-    }
-
     fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool, _whole_archive: bool) {
         panic!("staticlibs not supported")
     }
@@ -1791,10 +1826,6 @@ impl<'a> Linker for LlbcLinker<'a> {
 
     fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {}
 
-    fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) {
-        panic!("external dylibs not supported")
-    }
-
     fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool, _whole_archive: bool) {
         panic!("staticlibs not supported")
     }
@@ -1866,10 +1897,6 @@ impl<'a> Linker for BpfLinker<'a> {
 
     fn set_output_kind(&mut self, _output_kind: LinkOutputKind, _out_filename: &Path) {}
 
-    fn link_dylib_by_name(&mut self, _name: &str, _verbatim: bool, _as_needed: bool) {
-        panic!("external dylibs not supported")
-    }
-
     fn link_staticlib_by_name(&mut self, _name: &str, _verbatim: bool, _whole_archive: bool) {
         panic!("staticlibs not supported")
     }

tests/run-make/dylib-soname/foo.rs

@@ -0,0 +1 @@
+pub fn something() {}

tests/run-make/dylib-soname/rmake.rs

@@ -0,0 +1,19 @@
+// Checks that produced dylibs have a relative SONAME set, so they don't put "unmovable" full paths
+// into DT_NEEDED when used by a full path.
+
+//@ only-linux
+//@ ignore-cross-compile
+
+use run_make_support::regex::Regex;
+use run_make_support::{cmd, run_in_tmpdir, rustc};
+
+fn main() {
+    run_in_tmpdir(|| {
+        rustc().crate_name("foo").crate_type("dylib").input("foo.rs").run();
+        cmd("readelf")
+            .arg("-d")
+            .arg("libfoo.so")
+            .run()
+            .assert_stdout_contains("Library soname: [libfoo.so]");
+    });
+}