Purge manual use of -Wl and -Xlinker

tamird · tamird · commit c30d8f2f1f2d · 2017-11-25T11:15:41.000-05:00
...outside of target specifications - that'll come next. Updates rust-lang#11937.
diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs
@@ -206,31 +206,35 @@ fn main() {
         // Cargo will be very different than the runtime directory structure.
         //
         // All that's a really long winded way of saying that if we use
-        // `-Crpath` then the executables generated have the wrong rpath of
+        // `-C rpath` then the executables generated have the wrong rpath of
         // something like `$ORIGIN/deps` when in fact the way we distribute
         // rustc requires the rpath to be `$ORIGIN/../lib`.
         //
         // So, all in all, to set up the correct rpath we pass the linker
-        // argument manually via `-C link-args=-Wl,-rpath,...`. Plus isn't it
+        // argument manually via `-C link-args=-rpath,...`. Plus isn't it
         // fun to pass a flag to a tool to pass a flag to pass a flag to a tool
         // to change a flag in a binary?
         if env::var("RUSTC_RPATH") == Ok("true".to_string()) {
-            let rpath = if target.contains("apple") {
-
+            let rpath_flags = if target.contains("apple") {
                 // Note that we need to take one extra step on macOS to also pass
-                // `-Wl,-instal_name,@rpath/...` to get things to work right. To
+                // `-install_name,@rpath/...` to get things to work right. To
                 // do that we pass a weird flag to the compiler to get it to do
                 // so. Note that this is definitely a hack, and we should likely
                 // flesh out rpath support more fully in the future.
                 cmd.arg("-Z").arg("osx-rpath-install-name");
-                Some("-Wl,-rpath,@loader_path/../lib")
+                Some(&["-rpath", "@loader_path/../lib"])
             } else if !target.contains("windows") {
-                Some("-Wl,-rpath,$ORIGIN/../lib")
+                Some(&["-rpath", "$ORIGIN/../lib"])
             } else {
                 None
             };
-            if let Some(rpath) = rpath {
-                cmd.arg("-C").arg(format!("link-args={}", rpath));
+            if let Some(rpath_flags) = rpath_flags {
+                for rpath_flag in rpath_flags {
+                    if stage == "0" {
+                        cmd.arg("-C").arg("link-arg=-Xlinker");
+                    }
+                    cmd.arg("-C").arg(format!("link-arg={}", rpath_flag));
+                }
             }
         }
 
diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs
@@ -1108,9 +1108,9 @@ fn add_upstream_rust_crates(cmd: &mut Linker,
         }
     }
 
-    // We must link the sanitizer runtime using -Wl,--whole-archive but since
-    // it's packed in a .rlib, it contains stuff that are not objects that will
-    // make the linker error. So we must remove those bits from the .rlib before
+    // We must link the sanitizer runtime using --whole-archive but since it's
+    // packed in a .rlib, it contains stuff that are not objects that will make
+    // the linker error. So we must remove those bits from the .rlib before
     // linking it.
     fn link_sanitizer_runtime(cmd: &mut Linker,
                               sess: &Session,
@@ -1129,7 +1129,7 @@ fn add_upstream_rust_crates(cmd: &mut Linker,
             // FIXME: Remove this logic into librustc_*san once Cargo supports it
             let rpath = cratepath.parent().unwrap();
             let rpath = rpath.to_str().expect("non-utf8 component in path");
-            cmd.args(&["-Wl,-rpath".into(), "-Xlinker".into(), rpath.into()]);
+            cmd.args(&["-rpath".into(), rpath.into()]);
         }
 
         let dst = tmpdir.join(cratepath.file_name().unwrap());
diff --git a/src/librustc_trans/back/linker.rs b/src/librustc_trans/back/linker.rs
@@ -131,17 +131,22 @@ pub struct GccLinker<'a> {
 impl<'a> GccLinker<'a> {
     /// Argument that must be passed *directly* to the linker
     ///
-    /// These arguments need to be prepended with '-Wl,' when a gcc-style linker is used
+    /// These arguments need to be preceded by '-Xlinker' when a gcc-style
+    /// linker is used.
     fn linker_arg<S>(&mut self, arg: S) -> &mut Self
         where S: AsRef<OsStr>
     {
         if !self.is_ld {
-            let mut os = OsString::from("-Wl,");
-            os.push(arg.as_ref());
-            self.cmd.arg(os);
-        } else {
-            self.cmd.arg(arg);
+            self.cmd.arg("-Xlinker");
         }
+        self.cmd.arg(arg);
+        self
+    }
+
+    fn add_lib<S: AsRef<OsStr>>(&mut self, lib: S) -> &mut Self {
+        let mut os = OsString::from("-l");
+        os.push(lib.as_ref());
+        self.cmd.arg(os);
         self
     }
 
@@ -171,8 +176,8 @@ impl<'a> GccLinker<'a> {
 }
 
 impl<'a> Linker for GccLinker<'a> {
-    fn link_dylib(&mut self, lib: &str) { self.hint_dynamic(); self.cmd.arg("-l").arg(lib); }
-    fn link_staticlib(&mut self, lib: &str) { self.hint_static(); self.cmd.arg("-l").arg(lib); }
+    fn link_dylib(&mut self, lib: &str) { self.hint_dynamic(); self.add_lib(lib); }
+    fn link_staticlib(&mut self, lib: &str) { self.hint_static(); self.add_lib(lib); }
     fn link_rlib(&mut self, lib: &Path) { self.hint_static(); self.cmd.arg(lib); }
     fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); }
     fn framework_path(&mut self, path: &Path) { self.cmd.arg("-F").arg(path); }
@@ -182,11 +187,15 @@ impl<'a> Linker for GccLinker<'a> {
     fn partial_relro(&mut self) { self.linker_arg("-z,relro"); }
     fn full_relro(&mut self) { self.linker_arg("-z,relro,-z,now"); }
     fn build_static_executable(&mut self) { self.cmd.arg("-static"); }
-    fn args(&mut self, args: &[String]) { self.cmd.args(args); }
+    fn args(&mut self, args: &[String]) {
+        for arg in args {
+            self.linker_arg(arg);
+        }
+    }
 
     fn link_rust_dylib(&mut self, lib: &str, _path: &Path) {
         self.hint_dynamic();
-        self.cmd.arg("-l").arg(lib);
+        self.add_lib(lib);
     }
 
     fn link_framework(&mut self, framework: &str) {
@@ -202,25 +211,22 @@ impl<'a> Linker for GccLinker<'a> {
     // functions, etc.
     fn link_whole_staticlib(&mut self, lib: &str, search_path: &[PathBuf]) {
         self.hint_static();
-        let target = &self.sess.target.target;
-        if !target.options.is_like_osx {
-            self.linker_arg("--whole-archive").cmd.arg("-l").arg(lib);
-            self.linker_arg("--no-whole-archive");
+        if self.sess.target.target.options.is_like_osx {
+            self.linker_arg("-force_load");
+            // let-binding needed to appease borrowck.
+            let lib = archive::find_library(lib, search_path, &self.sess);
+            self.linker_arg(lib);
         } else {
-            // -force_load is the macOS equivalent of --whole-archive, but it
-            // involves passing the full path to the library to link.
-            let mut v = OsString::from("-force_load,");
-            v.push(&archive::find_library(lib, search_path, &self.sess));
-            self.linker_arg(&v);
+            self.linker_arg("--whole-archive").add_lib(lib);
+            self.linker_arg("--no-whole-archive");
         }
     }
 
     fn link_whole_rlib(&mut self, lib: &Path) {
         self.hint_static();
         if self.sess.target.target.options.is_like_osx {
-            let mut v = OsString::from("-force_load,");
-            v.push(lib);
-            self.linker_arg(&v);
+            self.linker_arg("-force_load");
+            self.linker_arg(lib);
         } else {
             self.linker_arg("--whole-archive").cmd.arg(lib);
             self.linker_arg("--no-whole-archive");
@@ -282,16 +288,16 @@ impl<'a> Linker for GccLinker<'a> {
     fn build_dylib(&mut self, out_filename: &Path) {
         // On mac we need to tell the linker to let this library be rpathed
         if self.sess.target.target.options.is_like_osx {
-            self.cmd.arg("-dynamiclib");
             self.linker_arg("-dylib");
 
             // Note that the `osx_rpath_install_name` option here is a hack
             // purely to support rustbuild right now, we should get a more
             // principled solution at some point to force the compiler to pass
-            // the right `-Wl,-install_name` with an `@rpath` in it.
+            // the right `-install_name` with an `@rpath` in it.
             if self.sess.opts.cg.rpath ||
                self.sess.opts.debugging_opts.osx_rpath_install_name {
-                let mut v = OsString::from("-install_name,@rpath/");
+                self.linker_arg("-install_name");
+                let mut v = OsString::from("@rpath/");
                 v.push(out_filename.file_name().unwrap());
                 self.linker_arg(&v);
             }
@@ -313,7 +319,6 @@ impl<'a> Linker for GccLinker<'a> {
             return
         }
 
-        let mut arg = OsString::new();
         let path = tmpdir.join("list");
 
         debug!("EXPORTED SYMBOLS:");
@@ -349,24 +354,16 @@ impl<'a> Linker for GccLinker<'a> {
         }
 
         if self.sess.target.target.options.is_like_osx {
-            if !self.is_ld {
-                arg.push("-Wl,")
-            }
-            arg.push("-exported_symbols_list,");
+            self.linker_arg("-exported_symbols_list");
+            self.linker_arg(&path);
         } else if self.sess.target.target.options.is_like_solaris {
-            if !self.is_ld {
-                arg.push("-Wl,")
-            }
-            arg.push("-M,");
+            self.linker_arg("-M");
+            self.linker_arg(&path);
         } else {
-            if !self.is_ld {
-                arg.push("-Wl,")
-            }
-            arg.push("--version-script=");
+            let mut arg = OsString::from("--version-script=");
+            arg.push(&path);
+            self.linker_arg(&arg);
         }
-
-        arg.push(&path);
-        self.cmd.arg(arg);
     }
 
     fn subsystem(&mut self, subsystem: &str) {
diff --git a/src/librustc_trans/back/rpath.rs b/src/librustc_trans/back/rpath.rs
@@ -26,41 +26,30 @@ pub struct RPathConfig<'a> {
 }
 
 pub fn get_rpath_flags(config: &mut RPathConfig) -> Vec<String> {
-    // No rpath on windows
-    if !config.has_rpath {
-        return Vec::new();
-    }
-
     let mut flags = Vec::new();
 
-    debug!("preparing the RPATH!");
+    // No rpath on windows
+    if config.has_rpath {
+        debug!("preparing the RPATH!");
+
+        let libs = config.used_crates.iter().filter_map(|&(_, ref l)| {
+            l.option()
+        }).collect::<Vec<_>>();
 
-    let libs = config.used_crates.clone();
-    let libs = libs.iter().filter_map(|&(_, ref l)| l.option()).collect::<Vec<_>>();
-    let rpaths = get_rpaths(config, &libs);
-    flags.extend_from_slice(&rpaths_to_flags(&rpaths));
+        for rpath in get_rpaths(config, &libs) {
+            flags.push("-rpath".into());
+            flags.push(rpath);
+        }
 
-    // Use DT_RUNPATH instead of DT_RPATH if available
-    if config.linker_is_gnu {
-        flags.push("-Wl,--enable-new-dtags".to_string());
+        // Use DT_RUNPATH instead of DT_RPATH if available
+        if config.linker_is_gnu {
+            flags.push("--enable-new-dtags".into());
+        }
     }
 
     flags
 }
 
-fn rpaths_to_flags(rpaths: &[String]) -> Vec<String> {
-    let mut ret = Vec::new();
-    for rpath in rpaths {
-        if rpath.contains(',') {
-            ret.push("-Wl,-rpath".into());
-            ret.push("-Xlinker".into());
-            ret.push(rpath.clone());
-        } else {
-            ret.push(format!("-Wl,-rpath,{}", &(*rpath)));
-        }
-    }
-    return ret;
-}
 
 fn get_rpaths(config: &mut RPathConfig, libs: &[PathBuf]) -> Vec<String> {
     debug!("output: {:?}", config.out_filename.display());
@@ -91,8 +80,7 @@ fn get_rpaths(config: &mut RPathConfig, libs: &[PathBuf]) -> Vec<String> {
     rpaths.extend_from_slice(&fallback_rpaths);
 
     // Remove duplicates
-    let rpaths = minimize_rpaths(&rpaths);
-    return rpaths;
+    minimize_rpaths(&rpaths)
 }
 
 fn get_rpaths_relative_to_output(config: &mut RPathConfig,
@@ -187,20 +175,9 @@ fn minimize_rpaths(rpaths: &[String]) -> Vec<String> {
 #[cfg(all(unix, test))]
 mod tests {
     use super::{RPathConfig};
-    use super::{minimize_rpaths, rpaths_to_flags, get_rpath_relative_to_output};
+    use super::{minimize_rpaths, get_rpath_relative_to_output};
     use std::path::{Path, PathBuf};
 
-    #[test]
-    fn test_rpaths_to_flags() {
-        let flags = rpaths_to_flags(&[
-            "path1".to_string(),
-            "path2".to_string()
-        ]);
-        assert_eq!(flags,
-                   ["-Wl,-rpath,path1",
-                    "-Wl,-rpath,path2"]);
-    }
-
     #[test]
     fn test_minimize1() {
         let res = minimize_rpaths(&[
@@ -264,19 +241,4 @@ mod tests {
             assert_eq!(res, "$ORIGIN/../lib");
         }
     }
-
-    #[test]
-    fn test_xlinker() {
-        let args = rpaths_to_flags(&[
-            "a/normal/path".to_string(),
-            "a,comma,path".to_string()
-        ]);
-
-        assert_eq!(args, vec![
-            "-Wl,-rpath,a/normal/path".to_string(),
-            "-Wl,-rpath".to_string(),
-            "-Xlinker".to_string(),
-            "a,comma,path".to_string()
-        ]);
-    }
 }
diff --git a/src/test/run-make/fpic/Makefile b/src/test/run-make/fpic/Makefile
@@ -7,7 +7,10 @@ ifeq ($(UNAME),Darwin)
 all:
 else ifdef IS_WINDOWS
 all:
-else
+else ifeq ($(findstring stage0,$(RUST_BUILD_STAGE)),stage0)
 all:
 	$(RUSTC) hello.rs -C link-args=-Wl,-z,text
+else
+all:
+	$(RUSTC) hello.rs -C link-args='-z text'
 endif

Original file line number	Diff line number	Diff line change
`@@ -1108,9 +1108,9 @@ fn add_upstream_rust_crates(cmd: &mut Linker,`
`1108`	`1108`	`}`
`1109`	`1109`	`}`
`1110`	`1110`
`1111`		`- // We must link the sanitizer runtime using -Wl,--whole-archive but since`
`1112`		`- // it's packed in a .rlib, it contains stuff that are not objects that will`
`1113`		`- // make the linker error. So we must remove those bits from the .rlib before`
	`1111`	`+ // We must link the sanitizer runtime using --whole-archive but since it's`
	`1112`	`+ // packed in a .rlib, it contains stuff that are not objects that will make`
	`1113`	`+ // the linker error. So we must remove those bits from the .rlib before`
`1114`	`1114`	`// linking it.`
`1115`	`1115`	`fn link_sanitizer_runtime(cmd: &mut Linker,`
`1116`	`1116`	`sess: &Session,`
`@@ -1129,7 +1129,7 @@ fn add_upstream_rust_crates(cmd: &mut Linker,`
`1129`	`1129`	`// FIXME: Remove this logic into librustc_*san once Cargo supports it`
`1130`	`1130`	`let rpath = cratepath.parent().unwrap();`
`1131`	`1131`	`let rpath = rpath.to_str().expect("non-utf8 component in path");`
`1132`		`- cmd.args(&["-Wl,-rpath".into(), "-Xlinker".into(), rpath.into()]);`
	`1132`	`+ cmd.args(&["-rpath".into(), rpath.into()]);`
`1133`	`1133`	`}`
`1134`	`1134`
`1135`	`1135`	`let dst = tmpdir.join(cratepath.file_name().unwrap());`