Skip to content

Commit 9525719

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 3ee5698 commit 9525719

File tree

3 files changed

+74
-17
lines changed

3 files changed

+74
-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: 72 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,73 @@ 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+
exec(cmd);
259+
// Our imitation of exec only returns upon success
260+
std::process::exit(0)
261+
}
262+
// On Unix targets, actually exec
263+
// If exec returns, process setup has failed. This is the same error condition as the expect in
264+
// the non-Unix case.
265+
#[cfg(unix)]
266+
{
267+
use std::os::unix::process::CommandExt;
268+
let error = cmd.exec();
269+
Err(error).expect("failed to run command")
258270
}
259271
}
260272

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

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

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

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

894950
if verbose > 0 {
@@ -902,8 +958,6 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
902958
exec_with_pipe(cmd, &env.stdin);
903959
}
904960

905-
store_json(CrateRunInfo::RunWith(env));
906-
907961
return;
908962
}
909963

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

9891041
// Create a stub .rlib file if "link" was requested by cargo.
9901042
// This is necessary to prevent cargo from doing rebuilds all the time.
@@ -999,6 +1051,9 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
9991051
File::create(out_filename("", ".dll")).expect("failed to create fake .dll file");
10001052
File::create(out_filename("", ".lib")).expect("failed to create fake .lib file");
10011053
}
1054+
1055+
debug_cmd("[cargo-miri rustc]", verbose, &cmd);
1056+
exec(cmd);
10021057
}
10031058

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

0 commit comments

Comments
 (0)