Auto merge of #26382 - alexcrichton:less-racy-path, r=brson

Environment variables are global state so this can lead to surprising results if
the driver is called in a multithreaded environment (e.g. doctests). There
shouldn't be any memory corruption that's possible, but a lot of the bots have
been failing because they can't find `cc` or `gcc` in the path during doctests,
and I highly suspect that it is due to the compiler modifying `PATH` in a
multithreaded fashion.

This commit moves the logic for appending to `PATH` to only affect the child
process instead of also affecting the parent, at least for the linking stage.
When loading dynamic libraries the compiler still modifies `PATH` on Windows,
but this may be more difficult to fix than spawning off a new process.

2 files changed, 11 insertions, 8 deletions

diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
index a834ed4cb5f..f2ff0060e55 100644
--- a/src/librustc_driver/driver.rs
+++ b/src/librustc_driver/driver.rs
@@ -778,18 +778,11 @@ pub fn phase_5_run_llvm_passes(sess: &Session,
 pub fn phase_6_link_output(sess: &Session,
                            trans: &trans::CrateTranslation,
                            outputs: &OutputFilenames) {
-    let old_path = env::var_os("PATH").unwrap_or(OsString::new());
-    let mut new_path = sess.host_filesearch(PathKind::All).get_tools_search_paths();
-    new_path.extend(env::split_paths(&old_path));
-    env::set_var("PATH", &env::join_paths(&new_path).unwrap());
-
     time(sess.time_passes(), "linking", (), |_|
          link::link_binary(sess,
                            trans,
                            outputs,
                            &trans.link.crate_name));
-
-    env::set_var("PATH", &old_path);
 }
 
 fn escape_dep_filename(filename: &str) -> String {
diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs
index f6f9afa0221..c12bc07ce0f 100644
--- a/src/librustc_trans/back/link.rs
+++ b/src/librustc_trans/back/link.rs
@@ -29,6 +29,7 @@ use util::sha2::{Digest, Sha256};
 use util::fs::fix_windows_verbatim_for_gcc;
 use rustc_back::tempdir::TempDir;
 
+use std::env;
 use std::fs::{self, PathExt};
 use std::io::{self, Read, Write};
 use std::mem;
@@ -794,7 +795,16 @@ fn link_natively(sess: &Session, trans: &CrateTranslation, dylib: bool,
 
     // The invocations of cc share some flags across platforms
     let pname = get_cc_prog(sess);
-    let mut cmd = Command::new(&pname[..]);
+    let mut cmd = Command::new(&pname);
+
+    // The compiler's sysroot often has some bundled tools, so add it to the
+    // PATH for the child.
+    let mut new_path = sess.host_filesearch(PathKind::All)
+                           .get_tools_search_paths();
+    if let Some(path) = env::var_os("PATH") {
+        new_path.extend(env::split_paths(&path));
+    }
+    cmd.env("PATH", env::join_paths(new_path).unwrap());
 
     let root = sess.target_filesearch(PathKind::Native).get_lib_path();
     cmd.args(&sess.target.target.options.pre_link_args);