Skip to content

Commit c92423b

Browse files
committed
Use real exec on cfg(unix) targets
When cargo-miri is executed as a cargo test runner or rustdoc runtool, external tools expect what they launch as the runner/runtool to be the process actually running the test. But in the implementation, we launch the Miri interpreter as a subprocess using std::process::Command. This tends to confuse other tools (like nextest) and users (like the author). What we really want is to call POSIX exec so that the cargo-miri process becomes the interpreter. So this implements just that; we call execve via a cfg(unix) extension trait. Windows has no such mechanism, but it also doesn't have POSIX signals, which is the primary tripping hazard this change fixes.
1 parent a591e9f commit c92423b

File tree

1 file changed

+62
-23
lines changed

1 file changed

+62
-23
lines changed

cargo-miri/bin.rs

Lines changed: 62 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ enum MiriCommand {
5151
}
5252

5353
/// The information to run a crate with the given environment.
54-
#[derive(Serialize, Deserialize)]
54+
#[derive(Clone, Serialize, Deserialize)]
5555
struct CrateRunEnv {
5656
/// The command-line arguments.
5757
args: Vec<String>,
@@ -249,27 +249,57 @@ fn xargo_check() -> Command {
249249
Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check")))
250250
}
251251

252-
/// Execute the command. If it fails, fail this process with the same exit code.
253-
/// Otherwise, continue.
254-
fn exec(mut cmd: Command) {
255-
let exit_status = cmd.status().expect("failed to run command");
256-
if exit_status.success().not() {
252+
/// Execute the `Command`, where possible by replacing the current process with a new process
253+
/// described by the `Command`. Then exit this process with the exit code of the new process.
254+
fn exec(mut cmd: Command) -> ! {
255+
// On non-Unix imitate POSIX exec as closely as we can
256+
#[cfg(not(unix))]
257+
{
258+
let exit_status = cmd.status().expect("failed to run command");
257259
std::process::exit(exit_status.code().unwrap_or(-1))
258260
}
261+
// On Unix targets, actually exec
262+
// If exec returns, process setup has failed. This is the same error condition as the expect in
263+
// the non-Unix case.
264+
#[cfg(unix)]
265+
{
266+
use std::os::unix::process::CommandExt;
267+
let error = cmd.exec();
268+
Err(error).expect("failed to run command")
269+
}
259270
}
260271

261-
/// Execute the command and pipe `input` into its stdin.
262-
/// If it fails, fail this process with the same exit code.
263-
/// Otherwise, continue.
264-
fn exec_with_pipe(mut cmd: Command, input: &[u8]) {
265-
cmd.stdin(process::Stdio::piped());
266-
let mut child = cmd.spawn().expect("failed to spawn process");
272+
/// Execute the `Command`, where possible by replacing the current process with a new process
273+
/// described by the `Command`. Then exit this process with the exit code of the new process.
274+
/// `input` is also piped to the new process's stdin, on cfg(unix) platforms by writing its
275+
/// contents to `path` first, then setting stdin to that file.
276+
fn exec_with_pipe<P>(mut cmd: Command, input: &[u8], path: P) -> !
277+
where
278+
P: AsRef<Path>,
279+
{
280+
#[cfg(unix)]
267281
{
268-
let stdin = child.stdin.as_mut().expect("failed to open stdin");
269-
stdin.write_all(input).expect("failed to write out test source");
282+
// Write the bytes we want to send to stdin out to a file
283+
std::fs::write(&path, input).unwrap();
284+
285+
// Open the file for reading, and set our new stdin to it
286+
let stdin = File::open(&path).unwrap();
287+
cmd.stdin(stdin);
288+
289+
// Unlink the file so that it is fully cleaned up as soon as the new process exits
290+
std::fs::remove_file(&path).unwrap();
291+
exec(cmd)
270292
}
271-
let exit_status = child.wait().expect("failed to run command");
272-
if exit_status.success().not() {
293+
#[cfg(not(unix))]
294+
{
295+
drop(path); // We don't need the path, we can pipe the bytes directly
296+
cmd.stdin(process::Stdio::piped());
297+
let mut child = cmd.spawn().expect("failed to spawn process");
298+
{
299+
let stdin = child.stdin.as_mut().expect("failed to open stdin");
300+
stdin.write_all(input).expect("failed to write out test source");
301+
}
302+
let exit_status = child.wait().expect("failed to run command");
273303
std::process::exit(exit_status.code().unwrap_or(-1))
274304
}
275305
}
@@ -872,6 +902,8 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
872902
// and environment variables; this is used when cargo calls us again in the CARGO_TARGET_RUNNER phase.
873903
let env = CrateRunEnv::collect(args, inside_rustdoc);
874904

905+
store_json(CrateRunInfo::RunWith(env.clone()));
906+
875907
// Rustdoc expects us to exit with an error code if the test is marked as `compile_fail`,
876908
// just creating the JSON file is not enough: we need to detect syntax errors,
877909
// so we need to run Miri with `MIRI_BE_RUSTC` for a check-only build.
@@ -888,7 +920,15 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
888920
cmd.arg("--emit=metadata");
889921
}
890922

891-
cmd.args(&env.args);
923+
// Alter the `-o` parameter so that it does not overwrite the JSON file we stored above.
924+
let mut args = env.args.clone();
925+
for i in 0..args.len() {
926+
if args[i] == "-o" {
927+
args[i + 1].push_str("_miri");
928+
}
929+
}
930+
931+
cmd.args(&args);
892932
cmd.env("MIRI_BE_RUSTC", "target");
893933

894934
if verbose > 0 {
@@ -899,11 +939,9 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
899939
eprintln!("[cargo-miri rustc inside rustdoc] going to run:\n{:?}", cmd);
900940
}
901941

902-
exec_with_pipe(cmd, &env.stdin);
942+
exec_with_pipe(cmd, &env.stdin, format!("{}.stdin", out_filename("", "").display()));
903943
}
904944

905-
store_json(CrateRunInfo::RunWith(env));
906-
907945
return;
908946
}
909947

@@ -983,8 +1021,6 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
9831021
"[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate} print={print}"
9841022
);
9851023
}
986-
debug_cmd("[cargo-miri rustc]", verbose, &cmd);
987-
exec(cmd);
9881024

9891025
// Create a stub .rlib file if "link" was requested by cargo.
9901026
// This is necessary to prevent cargo from doing rebuilds all the time.
@@ -999,6 +1035,9 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
9991035
File::create(out_filename("", ".dll")).expect("failed to create fake .dll file");
10001036
File::create(out_filename("", ".lib")).expect("failed to create fake .lib file");
10011037
}
1038+
1039+
debug_cmd("[cargo-miri rustc]", verbose, &cmd);
1040+
exec(cmd);
10021041
}
10031042

10041043
#[derive(Debug, Copy, Clone, PartialEq)]
@@ -1100,7 +1139,7 @@ fn phase_runner(mut binary_args: impl Iterator<Item = String>, phase: RunnerPhas
11001139
// Run it.
11011140
debug_cmd("[cargo-miri runner]", verbose, &cmd);
11021141
match phase {
1103-
RunnerPhase::Rustdoc => exec_with_pipe(cmd, &info.stdin),
1142+
RunnerPhase::Rustdoc => exec_with_pipe(cmd, &info.stdin, format!("{}.stdin", binary)),
11041143
RunnerPhase::Cargo => exec(cmd),
11051144
}
11061145
}

0 commit comments

Comments
 (0)