Skip to content

Commit

Permalink
Auto merge of #26382 - alexcrichton:less-racy-path, r=brson
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Jun 20, 2015
2 parents a951569 + fcd99aa commit c057802
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
7 changes: 0 additions & 7 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 11 additions & 1 deletion src/librustc_trans/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit c057802

Please sign in to comment.