From fcd99aae0a6312ecd79ce3e814a54c6c1373f165 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 17 Jun 2015 17:38:03 -0700 Subject: [PATCH] rustc_driver: Frob the global PATH less 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. --- src/librustc_driver/driver.rs | 7 ------- src/librustc_trans/back/link.rs | 12 +++++++++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index eaac1eb6f2cb0..bda46bfe03c98 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -762,18 +762,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 e2c816bb84df6..82ba8b6df6f25 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -30,6 +30,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; @@ -796,7 +797,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);