Skip to content

Commit f4a9d0b

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 f4a9d0b

File tree

3 files changed

+73
-17
lines changed

3 files changed

+73
-17
lines changed

cargo-miri/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cargo-miri/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ doctest = false # and no doc tests
1717
directories = "3"
1818
rustc_version = "0.4"
1919
serde_json = "1.0.40"
20+
libc = "0.2"
2021

2122
# A noop dependency that changes in the Rust repository, it's a bit of a hack.
2223
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`

cargo-miri/bin.rs

Lines changed: 71 additions & 17 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>,
@@ -251,26 +251,72 @@ fn xargo_check() -> Command {
251251

252252
/// Execute the command. If it fails, fail this process with the same exit code.
253253
/// 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() {
257-
std::process::exit(exit_status.code().unwrap_or(-1))
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");
259+
std::process::exit(exit_status.code().unwrap_or(-1));
260+
}
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")
258269
}
259270
}
260271

261272
/// Execute the command and pipe `input` into its stdin.
262273
/// If it fails, fail this process with the same exit code.
263274
/// Otherwise, continue.
264275
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");
276+
#[cfg(unix)]
267277
{
268-
let stdin = child.stdin.as_mut().expect("failed to open stdin");
269-
stdin.write_all(input).expect("failed to write out test source");
278+
use std::os::unix::io::FromRawFd;
279+
use std::process::Stdio;
280+
281+
let mut fds: [libc::c_int; 2] = [0, 0];
282+
// SAFETY: fds is an array of 2 libc::c_int
283+
let res = unsafe { libc::pipe(fds.as_mut_ptr()) };
284+
assert_eq!(res, 0, "failed to create pipe");
285+
286+
// We need to set close-on-exec, otherwise our pipe isn't readable after exec
287+
// SAFETY: fcntl has no preconditions
288+
unsafe {
289+
for fd in &fds {
290+
let res = libc::fcntl(
291+
*fd,
292+
libc::F_SETFD,
293+
libc::fcntl(*fd, libc::F_GETFD) | libc::FD_CLOEXEC,
294+
);
295+
assert_eq!(res, 0, "failed to set close-on-exec for pipe");
296+
}
297+
}
298+
299+
// SAFETY: Both elements of fds are open file descriptors, because pipe2 returned 0
300+
let dst = unsafe { Stdio::from_raw_fd(fds[0]) };
301+
let mut src = unsafe { File::from_raw_fd(fds[1]) };
302+
303+
src.write_all(input).expect("failed to write out test source");
304+
305+
cmd.stdin(dst);
306+
exec(cmd)
270307
}
271-
let exit_status = child.wait().expect("failed to run command");
272-
if exit_status.success().not() {
273-
std::process::exit(exit_status.code().unwrap_or(-1))
308+
#[cfg(not(unix))]
309+
{
310+
cmd.stdin(process::Stdio::piped());
311+
let mut child = cmd.spawn().expect("failed to spawn process");
312+
{
313+
let stdin = child.stdin.as_mut().expect("failed to open stdin");
314+
stdin.write_all(input).expect("failed to write out test source");
315+
}
316+
let exit_status = child.wait().expect("failed to run command");
317+
if exit_status.success().not() {
318+
std::process::exit(exit_status.code().unwrap_or(-1))
319+
}
274320
}
275321
}
276322

@@ -872,6 +918,8 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
872918
// and environment variables; this is used when cargo calls us again in the CARGO_TARGET_RUNNER phase.
873919
let env = CrateRunEnv::collect(args, inside_rustdoc);
874920

921+
store_json(CrateRunInfo::RunWith(env.clone()));
922+
875923
// Rustdoc expects us to exit with an error code if the test is marked as `compile_fail`,
876924
// just creating the JSON file is not enough: we need to detect syntax errors,
877925
// so we need to run Miri with `MIRI_BE_RUSTC` for a check-only build.
@@ -888,7 +936,14 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
888936
cmd.arg("--emit=metadata");
889937
}
890938

891-
cmd.args(&env.args);
939+
let mut args = env.args.clone();
940+
for i in 0..args.len() {
941+
if args[i] == "-o" {
942+
args[i + 1] = format!("{}_miri", args[i + 1]);
943+
}
944+
}
945+
946+
cmd.args(&args);
892947
cmd.env("MIRI_BE_RUSTC", "target");
893948

894949
if verbose > 0 {
@@ -902,8 +957,6 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
902957
exec_with_pipe(cmd, &env.stdin);
903958
}
904959

905-
store_json(CrateRunInfo::RunWith(env));
906-
907960
return;
908961
}
909962

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

9891040
// Create a stub .rlib file if "link" was requested by cargo.
9901041
// This is necessary to prevent cargo from doing rebuilds all the time.
@@ -999,6 +1050,9 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
9991050
File::create(out_filename("", ".dll")).expect("failed to create fake .dll file");
10001051
File::create(out_filename("", ".lib")).expect("failed to create fake .lib file");
10011052
}
1053+
1054+
debug_cmd("[cargo-miri rustc]", verbose, &cmd);
1055+
exec(cmd);
10021056
}
10031057

10041058
#[derive(Debug, Copy, Clone, PartialEq)]

0 commit comments

Comments
 (0)