From f7bfbe7c772bc8edea0e22618746f1b0547a0b71 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Tue, 12 Jan 2021 22:16:48 +0100 Subject: [PATCH 01/10] Prototype: make cargo-miri run doc-tests --- cargo-miri/bin.rs | 108 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 91 insertions(+), 17 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index b413a0b222..5c2f89f6fb 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -1,7 +1,7 @@ use std::env; use std::ffi::OsString; use std::fs::{self, File}; -use std::io::{self, BufRead, BufReader, BufWriter, Write}; +use std::io::{self, BufRead, BufReader, BufWriter, Read, Write}; use std::ops::Not; use std::path::{Path, PathBuf}; use std::process::Command; @@ -45,6 +45,8 @@ struct CrateRunInfo { env: Vec<(OsString, OsString)>, /// The current working directory. current_dir: OsString, + /// The contents passed via standard input. + stdin: Vec, } impl CrateRunInfo { @@ -53,7 +55,13 @@ impl CrateRunInfo { let args = args.collect(); let env = env::vars_os().collect(); let current_dir = env::current_dir().unwrap().into_os_string(); - CrateRunInfo { args, env, current_dir } + + let mut stdin = Vec::new(); + if env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some() { + std::io::stdin().lock().read_to_end(&mut stdin).expect("cannot read stdin"); + } + + CrateRunInfo { args, env, current_dir, stdin } } fn store(&self, filename: &Path) { @@ -539,17 +547,22 @@ fn phase_cargo_rustc(args: env::Args) { } fn out_filename(prefix: &str, suffix: &str) -> PathBuf { - let mut path = PathBuf::from(get_arg_flag_value("--out-dir").unwrap()); - path.push(format!( - "{}{}{}{}", - prefix, - get_arg_flag_value("--crate-name").unwrap(), - // This is technically a `-C` flag but the prefix seems unique enough... - // (and cargo passes this before the filename so it should be unique) - get_arg_flag_value("extra-filename").unwrap_or(String::new()), - suffix, - )); - path + if let Some(out_dir) = get_arg_flag_value("--out-dir") { + let mut path = PathBuf::from(out_dir); + path.push(format!( + "{}{}{}{}", + prefix, + get_arg_flag_value("--crate-name").unwrap(), + // This is technically a `-C` flag but the prefix seems unique enough... + // (and cargo passes this before the filename so it should be unique) + get_arg_flag_value("extra-filename").unwrap_or(String::new()), + suffix, + )); + path + } else { + let out_file = get_arg_flag_value("-o").unwrap(); + PathBuf::from(out_file) + } } let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); @@ -734,6 +747,44 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { if verbose { eprintln!("[cargo-miri runner] {:?}", cmd); } + + cmd.stdin(std::process::Stdio::piped()); + let mut child = cmd.spawn().expect("failed to spawn miri process"); + { + let stdin = child.stdin.as_mut().expect("failed to open stdin"); + stdin.write_all(&info.stdin).expect("failed to write out test source"); + } + let exit_status = child.wait().expect("failed to run command"); + if exit_status.success().not() { + std::process::exit(exit_status.code().unwrap_or(-1)) + } +} + +fn phase_cargo_rustdoc(fst_arg: &str, args: env::Args) { + let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); + + // phase_cargo_miri sets the RUSTDOC env var to ourselves, so we can't use that here; + // just default to a straight-forward invocation for now: + let mut cmd = Command::new(OsString::from("rustdoc")); + + // just pass everything through until we find a reason not to do that: + cmd.arg(fst_arg); + cmd.args(args); + + cmd.arg("-Z").arg("unstable-options"); + + let cargo_miri_path = std::env::current_exe().expect("current executable path invalid"); + cmd.arg("--test-builder").arg(&cargo_miri_path); + cmd.arg("--runtool").arg(&cargo_miri_path); + + // rustdoc passes generated code to rustc via stdin, rather than a temporary file, + // so we need to let the coming invocations know to expect that + cmd.env("MIRI_CALLED_FROM_RUSTDOC", "1"); + + if verbose { + eprintln!("[cargo-miri rustdoc] {:?}", cmd); + } + exec(cmd) } @@ -750,6 +801,30 @@ fn main() { return; } + // The way rustdoc invokes rustc is indistuingishable from the way cargo invokes rustdoc + // by the arguments alone, and we can't take from the args iterator in this case. + // phase_cargo_rustdoc sets this environment variable to let us disambiguate here + let invoked_as_rustc_from_rustdoc = env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some(); + if invoked_as_rustc_from_rustdoc { + // ...however, we then also see this variable when rustdoc invokes us as the testrunner! + // The runner is invoked as `$runtool ($runtool-arg)* output_file; + // since we don't specify any runtool-args, and rustdoc supplies multiple arguments to + // the test-builder unconditionally, we can just check the number of remaining arguments: + if args.len() == 1 { + let arg = args.next().unwrap(); + let binary = Path::new(&arg); + if binary.exists() { + phase_cargo_runner(binary, args); + } else { + show_error(format!("`cargo-miri` called with non-existing path argument `{}`; please invoke this binary through `cargo miri`", arg)); + } + } else { + phase_cargo_rustc(args); + } + + return; + } + // Dispatch to `cargo-miri` phase. There are three phases: // - When we are called via `cargo miri`, we run as the frontend and invoke the underlying // cargo. We set RUSTC_WRAPPER and CARGO_TARGET_RUNNER to ourselves. @@ -762,16 +837,15 @@ fn main() { Some("miri") => phase_cargo_miri(args), Some("rustc") => phase_cargo_rustc(args), Some(arg) => { - // We have to distinguish the "runner" and "rustfmt" cases. + // We have to distinguish the "runner" and "rustdoc" cases. // As runner, the first argument is the binary (a file that should exist, with an absolute path); - // as rustfmt, the first argument is a flag (`--something`). + // as rustdoc, the first argument is a flag (`--something`). let binary = Path::new(arg); if binary.exists() { assert!(!arg.starts_with("--")); // not a flag phase_cargo_runner(binary, args); } else if arg.starts_with("--") { - // We are rustdoc. - eprintln!("Running doctests is not currently supported by Miri.") + phase_cargo_rustdoc(arg, args); } else { show_error(format!("`cargo-miri` called with unexpected first argument `{}`; please only invoke this binary through `cargo miri`", arg)); } From 46e9dbcaffe67a8c40f672fd6a15209ccab7bd82 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Thu, 14 Jan 2021 18:41:21 +0100 Subject: [PATCH 02/10] Add check builds to support compile_fail doc-tests --- cargo-miri/bin.rs | 81 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 14 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 5c2f89f6fb..350795a051 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -157,6 +157,22 @@ fn exec(mut cmd: Command) { } } +/// Execute the command and pipe `input` into its stdin. +/// If it fails, fail this process with the same exit code. +/// Otherwise, continue. +fn exec_with_pipe(mut cmd: Command, input: &[u8]) { + cmd.stdin(std::process::Stdio::piped()); + let mut child = cmd.spawn().expect("failed to spawn process"); + { + let stdin = child.stdin.as_mut().expect("failed to open stdin"); + stdin.write_all(input).expect("failed to write out test source"); + } + let exit_status = child.wait().expect("failed to run command"); + if exit_status.success().not() { + std::process::exit(exit_status.code().unwrap_or(-1)) + } +} + fn xargo_version() -> Option<(u32, u32, u32)> { let out = xargo_check().arg("--version").output().ok()?; if !out.status.success() { @@ -598,6 +614,34 @@ fn phase_cargo_rustc(args: env::Args) { // (Need to do this here as cargo moves that "binary" to a different place before running it.) info.store(&out_filename("", ".exe")); + // Rustdoc expects us to exit with an error code if the test is marked as `compile_fail`, + // so we need to run Miri with `MIRI_BE_RUSTC` for a check-only build. + if std::env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some() { + let mut cmd = miri(); + + // use our own sysroot + if !has_arg_flag("--sysroot") { + let sysroot = env::var_os("MIRI_SYSROOT") + .expect("the wrapper should have set MIRI_SYSROOT"); + cmd.arg("--sysroot").arg(sysroot); + } + + // don't go into "code generation" (i.e. validation) + if info.args.iter().position(|arg| arg.starts_with("--emit=")).is_none() { + cmd.arg("--emit=dep-info,metadata"); + } + + cmd.args(info.args); + cmd.env("MIRI_BE_RUSTC", "1"); + + if verbose { + eprintln!("[cargo-miri rustc] captured input:\n{}", std::str::from_utf8(&info.stdin).unwrap()); + eprintln!("[cargo-miri rustc] {:?}", cmd); + } + + exec_with_pipe(cmd, &info.stdin); + } + return; } @@ -748,31 +792,40 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { eprintln!("[cargo-miri runner] {:?}", cmd); } - cmd.stdin(std::process::Stdio::piped()); - let mut child = cmd.spawn().expect("failed to spawn miri process"); - { - let stdin = child.stdin.as_mut().expect("failed to open stdin"); - stdin.write_all(&info.stdin).expect("failed to write out test source"); - } - let exit_status = child.wait().expect("failed to run command"); - if exit_status.success().not() { - std::process::exit(exit_status.code().unwrap_or(-1)) - } + exec_with_pipe(cmd, &info.stdin) } -fn phase_cargo_rustdoc(fst_arg: &str, args: env::Args) { +fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); // phase_cargo_miri sets the RUSTDOC env var to ourselves, so we can't use that here; // just default to a straight-forward invocation for now: let mut cmd = Command::new(OsString::from("rustdoc")); - // just pass everything through until we find a reason not to do that: + let extern_flag = "--extern"; + assert!(fst_arg != extern_flag); cmd.arg(fst_arg); - cmd.args(args); - cmd.arg("-Z").arg("unstable-options"); + // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. + while let Some(arg) = args.next() { + if arg == extern_flag { + cmd.arg(extern_flag); // always forward flag, but adjust filename + // `--extern` is always passed as a separate argument by cargo. + let next_arg = args.next().expect("`--extern` should be followed by a filename"); + if let Some(next_lib) = next_arg.strip_suffix(".rlib") { + // If this is an rlib, make it an rmeta. + cmd.arg(format!("{}.rmeta", next_lib)); + } else { + // Some other extern file (e.g., a `.so`). Forward unchanged. + cmd.arg(next_arg); + } + } else { + cmd.arg(arg); + } + } + cmd.arg("-Z").arg("unstable-options"); + let cargo_miri_path = std::env::current_exe().expect("current executable path invalid"); cmd.arg("--test-builder").arg(&cargo_miri_path); cmd.arg("--runtool").arg(&cargo_miri_path); From 11c3029e12c3ae6b185947324e76594006bba30e Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Sat, 16 Jan 2021 21:42:38 +0100 Subject: [PATCH 03/10] Play back stdin only when called from rustdoc --- cargo-miri/bin.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 350795a051..68fffa4aad 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -792,7 +792,11 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { eprintln!("[cargo-miri runner] {:?}", cmd); } - exec_with_pipe(cmd, &info.stdin) + if std::env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some() { + exec_with_pipe(cmd, &info.stdin) + } else { + exec(cmd) + } } fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { From 75053bee072372a331e34b2556ee93f824a4ee99 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Sat, 16 Jan 2021 21:43:39 +0100 Subject: [PATCH 04/10] Update test-cargo-miri --- test-cargo-miri/run-test.py | 7 +++++-- test-cargo-miri/test.default.stdout.ref | 6 ++++++ test-cargo-miri/test.filter.stdout.ref | 5 +++++ test-cargo-miri/test.stderr-rustdoc.ref | 1 - 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 60924d4f8d..ee561e7893 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -5,7 +5,7 @@ and the working directory to contain the cargo-miri-test project. ''' -import sys, subprocess, os +import sys, subprocess, os, re CGREEN = '\33[32m' CBOLD = '\33[1m' @@ -21,6 +21,9 @@ def cargo_miri(cmd): args += ["--target", os.environ['MIRI_TEST_TARGET']] return args +def scrub_timing_info(str): + return re.sub("finished in \d+\.\d\ds", "", str) + def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): print("Testing {}...".format(name)) ## Call `cargo miri`, capture all output @@ -36,7 +39,7 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): (stdout, stderr) = p.communicate(input=stdin) stdout = stdout.decode("UTF-8") stderr = stderr.decode("UTF-8") - if p.returncode == 0 and stdout == open(stdout_ref).read() and stderr == open(stderr_ref).read(): + if p.returncode == 0 and scrub_timing_info(stdout) == scrub_timing_info(open(stdout_ref).read()) and stderr == open(stderr_ref).read(): # All good! return # Show output diff --git a/test-cargo-miri/test.default.stdout.ref b/test-cargo-miri/test.default.stdout.ref index 7079798e42..c89a8d858b 100644 --- a/test-cargo-miri/test.default.stdout.ref +++ b/test-cargo-miri/test.default.stdout.ref @@ -8,3 +8,9 @@ running 7 tests ..i.... test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out + +running 1 test +test src/lib.rs - make_true (line 2) ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s + diff --git a/test-cargo-miri/test.filter.stdout.ref b/test-cargo-miri/test.filter.stdout.ref index 37efb8c3ee..b1409f0abd 100644 --- a/test-cargo-miri/test.filter.stdout.ref +++ b/test-cargo-miri/test.filter.stdout.ref @@ -9,3 +9,8 @@ test simple1 ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 6 filtered out + +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 0.00s + diff --git a/test-cargo-miri/test.stderr-rustdoc.ref b/test-cargo-miri/test.stderr-rustdoc.ref index a310169e30..e69de29bb2 100644 --- a/test-cargo-miri/test.stderr-rustdoc.ref +++ b/test-cargo-miri/test.stderr-rustdoc.ref @@ -1 +0,0 @@ -Running doctests is not currently supported by Miri. From b10a1b024d4688014c43261f53efa842557692d0 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Sat, 23 Jan 2021 18:02:47 +0100 Subject: [PATCH 05/10] Update README, add more comments, cleanup test-cargo-miri --- README.md | 4 ++ cargo-miri/bin.rs | 75 ++++++++++++++----------- test-cargo-miri/run-test.py | 24 ++++---- test-cargo-miri/test.default.stdout.ref | 2 +- test-cargo-miri/test.filter.stdout.ref | 2 +- test-cargo-miri/test.stderr-rustdoc.ref | 0 6 files changed, 60 insertions(+), 47 deletions(-) delete mode 100644 test-cargo-miri/test.stderr-rustdoc.ref diff --git a/README.md b/README.md index f4a762937e..590cafce8f 100644 --- a/README.md +++ b/README.md @@ -282,6 +282,10 @@ different Miri binaries, and as such worth documenting: that the compiled `rlib`s are compatible with Miri. When set while running `cargo-miri`, it indicates that we are part of a sysroot build (for which some crates need special treatment). +* `MIRI_CALLED_FROM_RUSTDOC` when set to any value tells `cargo-miri` that it is + running as a child process of `rustdoc`, which invokes it twice for each doc-test + and requires special treatment, most notably a check-only build before validation. + This is set by `cargo-miri` itself when running as a `rustdoc`-wrapper. * `MIRI_CWD` when set to any value tells the Miri driver to change to the given directory after loading all the source files, but before commencing interpretation. This is useful if the interpreted program wants a different diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 68fffa4aad..bf3b4b6b7b 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -615,6 +615,7 @@ fn phase_cargo_rustc(args: env::Args) { info.store(&out_filename("", ".exe")); // Rustdoc expects us to exit with an error code if the test is marked as `compile_fail`, + // just creating the JSON file is not enough: we need to detect syntax errors, // so we need to run Miri with `MIRI_BE_RUSTC` for a check-only build. if std::env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some() { let mut cmd = miri(); @@ -626,8 +627,12 @@ fn phase_cargo_rustc(args: env::Args) { cmd.arg("--sysroot").arg(sysroot); } - // don't go into "code generation" (i.e. validation) - if info.args.iter().position(|arg| arg.starts_with("--emit=")).is_none() { + // ensure --emit argument for a check-only build is present + if let Some(i) = info.args.iter().position(|arg| arg.starts_with("--emit=")) { + // We need to make sure we're not producing a binary that overwrites the JSON file. + // rustdoc should only ever pass an --emit=metadata argument for tests marked as `no_run`: + assert_eq!(info.args[i], "--emit=metadata"); + } else { cmd.arg("--emit=dep-info,metadata"); } @@ -703,6 +708,18 @@ fn phase_cargo_rustc(args: env::Args) { } } +fn try_forward_patched_extern_arg(args: &mut impl Iterator, cmd: &mut Command) { + cmd.arg("--extern"); // always forward flag, but adjust filename: + let path = args.next().expect("`--extern` should be followed by a filename"); + if let Some(lib) = path.strip_suffix(".rlib") { + // If this is an rlib, make it an rmeta. + cmd.arg(format!("{}.rmeta", lib)); + } else { + // Some other extern file (e.g. a `.so`). Forward unchanged. + cmd.arg(path); + } +} + fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); @@ -740,16 +757,7 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { let json_flag = "--json"; while let Some(arg) = args.next() { if arg == extern_flag { - cmd.arg(extern_flag); // always forward flag, but adjust filename - // `--extern` is always passed as a separate argument by cargo. - let next_arg = args.next().expect("`--extern` should be followed by a filename"); - if let Some(next_lib) = next_arg.strip_suffix(".rlib") { - // If this is an rlib, make it an rmeta. - cmd.arg(format!("{}.rmeta", next_lib)); - } else { - // Some other extern file (e.g., a `.so`). Forward unchanged. - cmd.arg(next_arg); - } + try_forward_patched_extern_arg(&mut args, &mut cmd); } else if arg.starts_with(error_format_flag) { let suffix = &arg[error_format_flag.len()..]; assert!(suffix.starts_with('=')); @@ -806,6 +814,10 @@ fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { // just default to a straight-forward invocation for now: let mut cmd = Command::new(OsString::from("rustdoc")); + // Because of the way the main function is structured, we have to take the first argument spearately + // from the rest; to simplify the following argument patching loop, we'll just skip that one. + // This is fine for now, because cargo will never pass an --extern argument in the first position, + // but we should defensively assert that this will work. let extern_flag = "--extern"; assert!(fst_arg != extern_flag); cmd.arg(fst_arg); @@ -813,31 +825,30 @@ fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. while let Some(arg) = args.next() { if arg == extern_flag { - cmd.arg(extern_flag); // always forward flag, but adjust filename - // `--extern` is always passed as a separate argument by cargo. - let next_arg = args.next().expect("`--extern` should be followed by a filename"); - if let Some(next_lib) = next_arg.strip_suffix(".rlib") { - // If this is an rlib, make it an rmeta. - cmd.arg(format!("{}.rmeta", next_lib)); - } else { - // Some other extern file (e.g., a `.so`). Forward unchanged. - cmd.arg(next_arg); - } + try_forward_patched_extern_arg(&mut args, &mut cmd); } else { cmd.arg(arg); } } + // For each doc-test, rustdoc starts two child processes: first the test is compiled, + // then the produced executable is invoked. We want to reroute both of these to cargo-miri, + // such that the first time we'll enter phase_cargo_rustc, and phase_cargo_runner second. + // + // rustdoc invokes the test-builder by forwarding most of its own arguments, which makes + // it difficult to determine when phase_cargo_rustc should run instead of phase_cargo_rustdoc. + // Furthermore, the test code is passed via stdin, rather than a temporary file, so we need + // to let phase_cargo_rustc know to expect that. We'll use this environment variable as a flag: + cmd.env("MIRI_CALLED_FROM_RUSTDOC", "1"); + + // The `--test-builder` and `--runtool` arguments are unstable rustdoc features, + // which are disabled by default. We first need to enable them explicitly: cmd.arg("-Z").arg("unstable-options"); let cargo_miri_path = std::env::current_exe().expect("current executable path invalid"); - cmd.arg("--test-builder").arg(&cargo_miri_path); - cmd.arg("--runtool").arg(&cargo_miri_path); + cmd.arg("--test-builder").arg(&cargo_miri_path); // invoked by forwarding most arguments + cmd.arg("--runtool").arg(&cargo_miri_path); // invoked with just a single path argument - // rustdoc passes generated code to rustc via stdin, rather than a temporary file, - // so we need to let the coming invocations know to expect that - cmd.env("MIRI_CALLED_FROM_RUSTDOC", "1"); - if verbose { eprintln!("[cargo-miri rustdoc] {:?}", cmd); } @@ -861,10 +872,10 @@ fn main() { // The way rustdoc invokes rustc is indistuingishable from the way cargo invokes rustdoc // by the arguments alone, and we can't take from the args iterator in this case. // phase_cargo_rustdoc sets this environment variable to let us disambiguate here - let invoked_as_rustc_from_rustdoc = env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some(); - if invoked_as_rustc_from_rustdoc { + let invoked_by_rustdoc = env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some(); + if invoked_by_rustdoc { // ...however, we then also see this variable when rustdoc invokes us as the testrunner! - // The runner is invoked as `$runtool ($runtool-arg)* output_file; + // The runner is invoked as `$runtool ($runtool-arg)* output_file`; // since we don't specify any runtool-args, and rustdoc supplies multiple arguments to // the test-builder unconditionally, we can just check the number of remaining arguments: if args.len() == 1 { @@ -873,7 +884,7 @@ fn main() { if binary.exists() { phase_cargo_runner(binary, args); } else { - show_error(format!("`cargo-miri` called with non-existing path argument `{}`; please invoke this binary through `cargo miri`", arg)); + show_error(format!("`cargo-miri` called with non-existing path argument `{}` in rustdoc mode; please invoke this binary through `cargo miri`", arg)); } } else { phase_cargo_rustc(args); diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index ee561e7893..f1c6db4a99 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -21,8 +21,8 @@ def cargo_miri(cmd): args += ["--target", os.environ['MIRI_TEST_TARGET']] return args -def scrub_timing_info(str): - return re.sub("finished in \d+\.\d\ds", "", str) +def normalize_stdout(str): + return re.sub("finished in \d+\.\d\ds", "finished in $TIME", str) def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): print("Testing {}...".format(name)) @@ -39,7 +39,7 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): (stdout, stderr) = p.communicate(input=stdin) stdout = stdout.decode("UTF-8") stderr = stderr.decode("UTF-8") - if p.returncode == 0 and scrub_timing_info(stdout) == scrub_timing_info(open(stdout_ref).read()) and stderr == open(stderr_ref).read(): + if p.returncode == 0 and normalize_stdout(stdout) == open(stdout_ref).read() and stderr == open(stderr_ref).read(): # All good! return # Show output @@ -72,40 +72,38 @@ def test_cargo_miri_run(): ) def test_cargo_miri_test(): - # rustdoc is not run on foreign targets - is_foreign = 'MIRI_TEST_TARGET' in os.environ - rustdoc_ref = "test.stderr-empty.ref" if is_foreign else "test.stderr-rustdoc.ref" + empty_ref = "test.stderr-empty.ref" test("`cargo miri test`", cargo_miri("test"), - "test.default.stdout.ref", rustdoc_ref, + "test.default.stdout.ref", empty_ref, env={'MIRIFLAGS': "-Zmiri-seed=feed"}, ) test("`cargo miri test` (no isolation)", cargo_miri("test"), - "test.default.stdout.ref", rustdoc_ref, + "test.default.stdout.ref", empty_ref, env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) test("`cargo miri test` (raw-ptr tracking)", cargo_miri("test"), - "test.default.stdout.ref", rustdoc_ref, + "test.default.stdout.ref", empty_ref, env={'MIRIFLAGS': "-Zmiri-track-raw-pointers"}, ) test("`cargo miri test` (with filter)", cargo_miri("test") + ["--", "--format=pretty", "le1"], - "test.filter.stdout.ref", rustdoc_ref, + "test.filter.stdout.ref", empty_ref, ) test("`cargo miri test` (test target)", cargo_miri("test") + ["--test", "test", "--", "--format=pretty"], - "test.test-target.stdout.ref", "test.stderr-empty.ref", + "test.test-target.stdout.ref", empty_ref, ) test("`cargo miri test` (bin target)", cargo_miri("test") + ["--bin", "cargo-miri-test", "--", "--format=pretty"], - "test.bin-target.stdout.ref", "test.stderr-empty.ref", + "test.bin-target.stdout.ref", empty_ref, ) test("`cargo miri test` (subcrate, no isolation)", cargo_miri("test") + ["-p", "subcrate"], - "test.subcrate.stdout.ref", "test.stderr-empty.ref", + "test.subcrate.stdout.ref", empty_ref, env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) diff --git a/test-cargo-miri/test.default.stdout.ref b/test-cargo-miri/test.default.stdout.ref index c89a8d858b..7ed0b2dbae 100644 --- a/test-cargo-miri/test.default.stdout.ref +++ b/test-cargo-miri/test.default.stdout.ref @@ -12,5 +12,5 @@ test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out running 1 test test src/lib.rs - make_true (line 2) ... ok -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME diff --git a/test-cargo-miri/test.filter.stdout.ref b/test-cargo-miri/test.filter.stdout.ref index b1409f0abd..18174cadd2 100644 --- a/test-cargo-miri/test.filter.stdout.ref +++ b/test-cargo-miri/test.filter.stdout.ref @@ -12,5 +12,5 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 6 filtered out running 0 tests -test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 0.00s +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in $TIME diff --git a/test-cargo-miri/test.stderr-rustdoc.ref b/test-cargo-miri/test.stderr-rustdoc.ref deleted file mode 100644 index e69de29bb2..0000000000 From 10115cd2a6756ce2d3bbc929bb43e5cb9741a9ef Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Mon, 25 Jan 2021 13:56:46 +0100 Subject: [PATCH 06/10] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 590cafce8f..8b28674886 100644 --- a/README.md +++ b/README.md @@ -284,7 +284,7 @@ different Miri binaries, and as such worth documenting: build (for which some crates need special treatment). * `MIRI_CALLED_FROM_RUSTDOC` when set to any value tells `cargo-miri` that it is running as a child process of `rustdoc`, which invokes it twice for each doc-test - and requires special treatment, most notably a check-only build before validation. + and requires special treatment, most notably a check-only build before interpretation. This is set by `cargo-miri` itself when running as a `rustdoc`-wrapper. * `MIRI_CWD` when set to any value tells the Miri driver to change to the given directory after loading all the source files, but before commencing From 8b29e5787dc9079da52a6e42d3a19056dcdfc9b5 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Mon, 25 Jan 2021 14:26:08 +0100 Subject: [PATCH 07/10] Enable doc-tests in cross-mode tests --- cargo-miri/bin.rs | 6 +++++- test-cargo-miri/run-test.py | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index bf3b4b6b7b..bf02bbab32 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -819,13 +819,17 @@ fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { // This is fine for now, because cargo will never pass an --extern argument in the first position, // but we should defensively assert that this will work. let extern_flag = "--extern"; + let runtool_flag = "--runtool"; assert!(fst_arg != extern_flag); cmd.arg(fst_arg); - // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. while let Some(arg) = args.next() { if arg == extern_flag { + // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. try_forward_patched_extern_arg(&mut args, &mut cmd); + } else if arg == runtool_flag { + // Do not forward an existing --runtool argument, since we will set this ourselves + let _ = args.next().expect("`--runtool` should be followed by an executable name"); } else { cmd.arg(arg); } diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index f1c6db4a99..9e20673ea8 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -19,6 +19,7 @@ def cargo_miri(cmd): args = ["cargo", "miri", cmd, "-q"] if 'MIRI_TEST_TARGET' in os.environ: args += ["--target", os.environ['MIRI_TEST_TARGET']] + args += ["-Zdoctest-xcompile"] return args def normalize_stdout(str): From f41d72c4f42f91707f7429d1c909044185ab1b26 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Mon, 25 Jan 2021 15:22:41 +0100 Subject: [PATCH 08/10] Rename helper function --- cargo-miri/bin.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index bf02bbab32..e3fff2570f 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -708,7 +708,7 @@ fn phase_cargo_rustc(args: env::Args) { } } -fn try_forward_patched_extern_arg(args: &mut impl Iterator, cmd: &mut Command) { +fn forward_patched_extern_arg(args: &mut impl Iterator, cmd: &mut Command) { cmd.arg("--extern"); // always forward flag, but adjust filename: let path = args.next().expect("`--extern` should be followed by a filename"); if let Some(lib) = path.strip_suffix(".rlib") { @@ -757,7 +757,7 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { let json_flag = "--json"; while let Some(arg) = args.next() { if arg == extern_flag { - try_forward_patched_extern_arg(&mut args, &mut cmd); + forward_patched_extern_arg(&mut args, &mut cmd); } else if arg.starts_with(error_format_flag) { let suffix = &arg[error_format_flag.len()..]; assert!(suffix.starts_with('=')); @@ -826,7 +826,7 @@ fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { while let Some(arg) = args.next() { if arg == extern_flag { // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. - try_forward_patched_extern_arg(&mut args, &mut cmd); + forward_patched_extern_arg(&mut args, &mut cmd); } else if arg == runtool_flag { // Do not forward an existing --runtool argument, since we will set this ourselves let _ = args.next().expect("`--runtool` should be followed by an executable name"); From a815a42e515c83c0c60149547d93717c7e751d89 Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Mon, 25 Jan 2021 15:33:26 +0100 Subject: [PATCH 09/10] Update assertion --- cargo-miri/bin.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index e3fff2570f..656382a6f4 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -816,11 +816,12 @@ fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { // Because of the way the main function is structured, we have to take the first argument spearately // from the rest; to simplify the following argument patching loop, we'll just skip that one. - // This is fine for now, because cargo will never pass an --extern argument in the first position, + // This is fine for now, because cargo will never pass the relevant arguments in the first position, // but we should defensively assert that this will work. let extern_flag = "--extern"; let runtool_flag = "--runtool"; assert!(fst_arg != extern_flag); + assert!(fst_arg != runtool_flag); cmd.arg(fst_arg); while let Some(arg) = args.next() { From 10345ff1597981762894a66df56a7abe73da26be Mon Sep 17 00:00:00 2001 From: Tristan Dannenberg Date: Thu, 28 Jan 2021 18:39:21 +0100 Subject: [PATCH 10/10] Detect cross-mode and skip doc-tests --- cargo-miri/bin.rs | 20 +++++++++++++------ test-cargo-miri/run-test.py | 17 ++++++++++------ test-cargo-miri/test.cross-target.stdout.ref | 10 ++++++++++ .../test.filter.cross-target.stdout.ref | 11 ++++++++++ 4 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 test-cargo-miri/test.cross-target.stdout.ref create mode 100644 test-cargo-miri/test.filter.cross-target.stdout.ref diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 480dd63859..39f8dcf17e 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -882,26 +882,34 @@ fn phase_cargo_rustdoc(fst_arg: &str, mut args: env::Args) { // Because of the way the main function is structured, we have to take the first argument spearately // from the rest; to simplify the following argument patching loop, we'll just skip that one. - // This is fine for now, because cargo will never pass the relevant arguments in the first position, + // This is fine for now, because cargo will never pass --extern arguments in the first position, // but we should defensively assert that this will work. let extern_flag = "--extern"; - let runtool_flag = "--runtool"; assert!(fst_arg != extern_flag); - assert!(fst_arg != runtool_flag); cmd.arg(fst_arg); - + + let runtool_flag = "--runtool"; + let mut crossmode = fst_arg == runtool_flag; while let Some(arg) = args.next() { if arg == extern_flag { // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. forward_patched_extern_arg(&mut args, &mut cmd); } else if arg == runtool_flag { - // Do not forward an existing --runtool argument, since we will set this ourselves - let _ = args.next().expect("`--runtool` should be followed by an executable name"); + // An existing --runtool flag indicates cargo is running in cross-target mode, which we don't support. + // Note that this is only passed when cargo is run with the unstable -Zdoctest-xcompile flag; + // otherwise, we won't be called as rustdoc at all. + crossmode = true; + break; } else { cmd.arg(arg); } } + if crossmode { + eprintln!("Cross-interpreting doc-tests is not currently supported by Miri."); + return; + } + // For each doc-test, rustdoc starts two child processes: first the test is compiled, // then the produced executable is invoked. We want to reroute both of these to cargo-miri, // such that the first time we'll enter phase_cargo_rustc, and phase_cargo_runner second. diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index d32afa6a8a..df0b736028 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -16,13 +16,13 @@ def fail(msg): sys.exit(1) def cargo_miri(cmd): - args = ["cargo", "+nightly", "miri", cmd, "-q"] + args = ["cargo", "miri", cmd, "-q"] if 'MIRI_TEST_TARGET' in os.environ: - args += ["-Zdoctest-xcompile"] args += ["--target", os.environ['MIRI_TEST_TARGET']] return args def normalize_stdout(str): + str = str.replace("src\\", "src/") # normalize paths across platforms return re.sub("finished in \d+\.\d\ds", "finished in $TIME", str) def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): @@ -73,24 +73,29 @@ def test_cargo_miri_run(): ) def test_cargo_miri_test(): + # rustdoc is not run on foreign targets + is_foreign = 'MIRI_TEST_TARGET' in os.environ + default_ref = "test.cross-target.stdout.ref" if is_foreign else "test.default.stdout.ref" + filter_ref = "test.filter.cross-target.stdout.ref" if is_foreign else "test.filter.stdout.ref" + test("`cargo miri test`", cargo_miri("test"), - "test.default.stdout.ref", "test.stderr-empty.ref", + default_ref, "test.stderr-empty.ref", env={'MIRIFLAGS': "-Zmiri-seed=feed"}, ) test("`cargo miri test` (no isolation)", cargo_miri("test"), - "test.default.stdout.ref", "test.stderr-empty.ref", + default_ref, "test.stderr-empty.ref", env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) test("`cargo miri test` (raw-ptr tracking)", cargo_miri("test"), - "test.default.stdout.ref", "test.stderr-empty.ref", + default_ref, "test.stderr-empty.ref", env={'MIRIFLAGS': "-Zmiri-track-raw-pointers"}, ) test("`cargo miri test` (with filter)", cargo_miri("test") + ["--", "--format=pretty", "le1"], - "test.filter.stdout.ref", "test.stderr-empty.ref", + filter_ref, "test.stderr-empty.ref", ) test("`cargo miri test` (test target)", cargo_miri("test") + ["--test", "test", "--", "--format=pretty"], diff --git a/test-cargo-miri/test.cross-target.stdout.ref b/test-cargo-miri/test.cross-target.stdout.ref new file mode 100644 index 0000000000..7079798e42 --- /dev/null +++ b/test-cargo-miri/test.cross-target.stdout.ref @@ -0,0 +1,10 @@ + +running 1 test +. +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out + + +running 7 tests +..i.... +test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out + diff --git a/test-cargo-miri/test.filter.cross-target.stdout.ref b/test-cargo-miri/test.filter.cross-target.stdout.ref new file mode 100644 index 0000000000..37efb8c3ee --- /dev/null +++ b/test-cargo-miri/test.filter.cross-target.stdout.ref @@ -0,0 +1,11 @@ + +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out + + +running 1 test +test simple1 ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 6 filtered out +