diff --git a/src/librustc_trans/back/command.rs b/src/librustc_trans/back/command.rs index ea68e3b28b668..05489807fa614 100644 --- a/src/librustc_trans/back/command.rs +++ b/src/librustc_trans/back/command.rs @@ -14,22 +14,34 @@ use std::ffi::{OsStr, OsString}; use std::fmt; use std::io; +use std::mem; use std::process::{self, Output, Child}; +#[derive(Clone)] pub struct Command { - program: OsString, + program: Program, args: Vec, env: Vec<(OsString, OsString)>, } +#[derive(Clone)] +enum Program { + Normal(OsString), + CmdBatScript(OsString), +} + impl Command { pub fn new>(program: P) -> Command { - Command::_new(program.as_ref()) + Command::_new(Program::Normal(program.as_ref().to_owned())) + } + + pub fn bat_script>(program: P) -> Command { + Command::_new(Program::CmdBatScript(program.as_ref().to_owned())) } - fn _new(program: &OsStr) -> Command { + fn _new(program: Program) -> Command { Command { - program: program.to_owned(), + program, args: Vec::new(), env: Vec::new(), } @@ -86,7 +98,14 @@ impl Command { } pub fn command(&self) -> process::Command { - let mut ret = process::Command::new(&self.program); + let mut ret = match self.program { + Program::Normal(ref p) => process::Command::new(p), + Program::CmdBatScript(ref p) => { + let mut c = process::Command::new("cmd"); + c.arg("/c").arg(p); + c + } + }; ret.args(&self.args); ret.envs(self.env.clone()); return ret @@ -94,16 +113,45 @@ impl Command { // extensions - pub fn get_program(&self) -> &OsStr { - &self.program + pub fn take_args(&mut self) -> Vec { + mem::replace(&mut self.args, Vec::new()) } - pub fn get_args(&self) -> &[OsString] { - &self.args - } + /// Returns a `true` if we're pretty sure that this'll blow OS spawn limits, + /// or `false` if we should attempt to spawn and see what the OS says. + pub fn very_likely_to_exceed_some_spawn_limit(&self) -> bool { + // We mostly only care about Windows in this method, on Unix the limits + // can be gargantuan anyway so we're pretty unlikely to hit them + if cfg!(unix) { + return false + } - pub fn get_env(&self) -> &[(OsString, OsString)] { - &self.env + // Ok so on Windows to spawn a process is 32,768 characters in its + // command line [1]. Unfortunately we don't actually have access to that + // as it's calculated just before spawning. Instead we perform a + // poor-man's guess as to how long our command line will be. We're + // assuming here that we don't have to escape every character... + // + // Turns out though that `cmd.exe` has even smaller limits, 8192 + // characters [2]. Linkers can often be batch scripts (for example + // Emscripten, Gecko's current build system) which means that we're + // running through batch scripts. These linkers often just forward + // arguments elsewhere (and maybe tack on more), so if we blow 8192 + // bytes we'll typically cause them to blow as well. + // + // Basically as a result just perform an inflated estimate of what our + // command line will look like and test if it's > 8192 (we actually + // test against 6k to artificially inflate our estimate). If all else + // fails we'll fall back to the normal unix logic of testing the OS + // error code if we fail to spawn and automatically re-spawning the + // linker with smaller arguments. + // + // [1]: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx + // [2]: https://blogs.msdn.microsoft.com/oldnewthing/20031210-00/?p=41553 + + let estimated_command_line_len = + self.args.iter().map(|a| a.len()).sum::(); + estimated_command_line_len > 1024 * 6 } } diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index f53c5b3f58131..e9aaeac87b435 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -36,8 +36,8 @@ use std::char; use std::env; use std::ffi::OsString; use std::fmt; -use std::fs::{self, File}; -use std::io::{self, Write, BufWriter}; +use std::fs; +use std::io; use std::path::{Path, PathBuf}; use std::process::{Output, Stdio}; use std::str; @@ -71,9 +71,7 @@ pub fn get_linker(sess: &Session) -> (PathBuf, Command, Vec<(OsString, OsString) let cmd = |linker: &Path| { if let Some(linker) = linker.to_str() { if cfg!(windows) && linker.ends_with(".bat") { - let mut cmd = Command::new("cmd"); - cmd.arg("/c").arg(linker); - return cmd + return Command::bat_script(linker) } } Command::new(linker) @@ -758,26 +756,26 @@ fn exec_linker(sess: &Session, cmd: &mut Command, tmpdir: &Path) // that contains all the arguments. The theory is that this is then // accepted on all linkers and the linker will read all its options out of // there instead of looking at the command line. - match cmd.command().stdout(Stdio::piped()).stderr(Stdio::piped()).spawn() { - Ok(child) => return child.wait_with_output(), - Err(ref e) if command_line_too_big(e) => {} - Err(e) => return Err(e) + if !cmd.very_likely_to_exceed_some_spawn_limit() { + match cmd.command().stdout(Stdio::piped()).stderr(Stdio::piped()).spawn() { + Ok(child) => return child.wait_with_output(), + Err(ref e) if command_line_too_big(e) => {} + Err(e) => return Err(e) + } } - let file = tmpdir.join("linker-arguments"); - let mut cmd2 = Command::new(cmd.get_program()); - cmd2.arg(format!("@{}", file.display())); - for &(ref k, ref v) in cmd.get_env() { - cmd2.env(k, v); - } - let mut f = BufWriter::new(File::create(&file)?); - for arg in cmd.get_args() { - writeln!(f, "{}", Escape { + let mut cmd2 = cmd.clone(); + let mut args = String::new(); + for arg in cmd2.take_args() { + args.push_str(&Escape { arg: arg.to_str().unwrap(), is_like_msvc: sess.target.target.options.is_like_msvc, - })?; + }.to_string()); + args.push_str("\n"); } - f.into_inner()?; + let file = tmpdir.join("linker-arguments"); + fs::write(&file, args.as_bytes())?; + cmd2.arg(format!("@{}", file.display())); return cmd2.output(); #[cfg(unix)] diff --git a/src/test/run-make/long-linker-command-lines-cmd-exe/Makefile b/src/test/run-make/long-linker-command-lines-cmd-exe/Makefile new file mode 100644 index 0000000000000..debe9e93824bd --- /dev/null +++ b/src/test/run-make/long-linker-command-lines-cmd-exe/Makefile @@ -0,0 +1,6 @@ +-include ../tools.mk + +all: + $(RUSTC) foo.rs -g + cp foo.bat $(TMPDIR)/ + OUT_DIR="$(TMPDIR)" RUSTC="$(RUSTC_ORIGINAL)" $(call RUN,foo) diff --git a/src/test/run-make/long-linker-command-lines-cmd-exe/foo.bat b/src/test/run-make/long-linker-command-lines-cmd-exe/foo.bat new file mode 100644 index 0000000000000..a9350f12bbb6d --- /dev/null +++ b/src/test/run-make/long-linker-command-lines-cmd-exe/foo.bat @@ -0,0 +1 @@ +%MY_LINKER% %* diff --git a/src/test/run-make/long-linker-command-lines-cmd-exe/foo.rs b/src/test/run-make/long-linker-command-lines-cmd-exe/foo.rs new file mode 100644 index 0000000000000..f9168a82e2259 --- /dev/null +++ b/src/test/run-make/long-linker-command-lines-cmd-exe/foo.rs @@ -0,0 +1,96 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Like the `long-linker-command-lines` test this test attempts to blow +// a command line limit for running the linker. Unlike that test, however, +// this test is testing `cmd.exe` specifically rather than the OS. +// +// Unfortunately `cmd.exe` has a 8192 limit which is relatively small +// in the grand scheme of things and anyone sripting rustc's linker +// is probably using a `*.bat` script and is likely to hit this limit. +// +// This test uses a `foo.bat` script as the linker which just simply +// delegates back to this program. The compiler should use a lower +// limit for arguments before passing everything via `@`, which +// means that everything should still succeed here. + +use std::env; +use std::fs::{self, File}; +use std::io::{BufWriter, Write, Read}; +use std::path::PathBuf; +use std::process::Command; + +fn main() { + if !cfg!(windows) { + return + } + + let tmpdir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); + let ok = tmpdir.join("ok"); + let not_ok = tmpdir.join("not_ok"); + if env::var("YOU_ARE_A_LINKER").is_ok() { + match env::args().find(|a| a.contains("@")) { + Some(file) => { fs::copy(&file[1..], &ok).unwrap(); } + None => { File::create(¬_ok).unwrap(); } + } + return + } + + let rustc = env::var_os("RUSTC").unwrap_or("rustc".into()); + let me = env::current_exe().unwrap(); + let bat = me.parent() + .unwrap() + .join("foo.bat"); + let bat_linker = format!("linker={}", bat.display()); + for i in (1..).map(|i| i * 10) { + println!("attempt: {}", i); + + let file = tmpdir.join("bar.rs"); + let mut f = BufWriter::new(File::create(&file).unwrap()); + let mut lib_name = String::new(); + for _ in 0..i { + lib_name.push_str("foo"); + } + for j in 0..i { + writeln!(f, "#[link(name = \"{}{}\")]", lib_name, j).unwrap(); + } + writeln!(f, "extern {{}}\nfn main() {{}}").unwrap(); + f.into_inner().unwrap(); + + drop(fs::remove_file(&ok)); + drop(fs::remove_file(¬_ok)); + let status = Command::new(&rustc) + .arg(&file) + .arg("-C").arg(&bat_linker) + .arg("--out-dir").arg(&tmpdir) + .env("YOU_ARE_A_LINKER", "1") + .env("MY_LINKER", &me) + .status() + .unwrap(); + + if !status.success() { + panic!("rustc didn't succeed: {}", status); + } + + if !ok.exists() { + assert!(not_ok.exists()); + continue + } + + let mut contents = String::new(); + File::open(&ok).unwrap().read_to_string(&mut contents).unwrap(); + + for j in 0..i { + assert!(contents.contains(&format!("{}{}", lib_name, j))); + } + + break + } +}