Auto merge of rust-lang#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 rust-lang#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 rust-lang#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]");
+    });
+}