From 33438c937d622ac981c0df6c736c9d4213386bf4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Sep 2020 19:28:29 +0200 Subject: [PATCH 01/29] canonicalize miri's directory --- miri | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/miri b/miri index 37e87ec798..f53c21ff11 100755 --- a/miri +++ b/miri @@ -39,6 +39,7 @@ EOF TARGET=$(rustc --version --verbose | grep "^host:" | cut -d ' ' -f 2) SYSROOT=$(rustc --print sysroot) LIBDIR=$SYSROOT/lib/rustlib/$TARGET/lib +MIRIDIR=$(readlink -e "$(dirname "$0")") if ! test -d "$LIBDIR"; then echo "Something went wrong determining the library dir." echo "I got $LIBDIR but that does not exist." @@ -51,7 +52,7 @@ if [ -z "$CARGO_INCREMENTAL" ]; then fi if [ -z "$CARGO_TARGET_DIR" ]; then # Share target dir between `miri` and `cargo-miri`. - export CARGO_TARGET_DIR="$(dirname "$0")"/target + export CARGO_TARGET_DIR="$MIRIDIR/target" fi # We set the rpath so that Miri finds the private rustc libraries it needs. # We enable debug-assertions to get tracing. @@ -63,9 +64,9 @@ export RUSTFLAGS="-C link-args=-Wl,-rpath,$LIBDIR -C debug-assertions -C debugin # Build a sysroot and set MIRI_SYSROOT to use it. Arguments are passed to `cargo miri setup`. build_sysroot() { # Build once, for the user to see. - cargo run $CARGO_BUILD_FLAGS --manifest-path "$(dirname "$0")"/cargo-miri/Cargo.toml -- miri setup "$@" + cargo run $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -- miri setup "$@" # Call again, to just set env var. - export MIRI_SYSROOT="$(cargo run $CARGO_BUILD_FLAGS --manifest-path "$(dirname "$0")"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")" + export MIRI_SYSROOT="$(cargo run $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")" } # Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account @@ -108,18 +109,18 @@ case "$COMMAND" in install|install-debug) # "--locked" to respect the Cargo.lock file if it exists, # "--offline" to avoid querying the registry (for yanked packages). - cargo install $CARGO_INSTALL_FLAGS --path "$(dirname "$0")" --force --locked --offline "$@" - cargo install $CARGO_INSTALL_FLAGS --path "$(dirname "$0")"/cargo-miri --force --locked --offline "$@" + cargo install $CARGO_INSTALL_FLAGS --path "$MIRIDIR" --force --locked --offline "$@" + cargo install $CARGO_INSTALL_FLAGS --path "$MIRIDIR"/cargo-miri --force --locked --offline "$@" ;; check|check-debug) # Check, and let caller control flags. - cargo check $CARGO_BUILD_FLAGS --manifest-path "$(dirname "$0")"/Cargo.toml "$@" - cargo check $CARGO_BUILD_FLAGS --manifest-path "$(dirname "$0")"/cargo-miri/Cargo.toml "$@" + cargo check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@" + cargo check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" ;; build|build-debug) # Build, and let caller control flags. - cargo build $CARGO_BUILD_FLAGS --manifest-path "$(dirname "$0")"/Cargo.toml "$@" - cargo build $CARGO_BUILD_FLAGS --manifest-path "$(dirname "$0")"/cargo-miri/Cargo.toml "$@" + cargo build $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@" + cargo build $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" ;; test|test-debug) # First build and get a sysroot. From 92aeb552ba3fa73d21716e37d9f2723501d990a0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Sep 2020 19:28:58 +0200 Subject: [PATCH 02/29] towards letting cargo do binary selection: wrappers and runners set up --- cargo-miri/Cargo.lock | 24 +-- cargo-miri/Cargo.toml | 1 - cargo-miri/bin.rs | 332 +++++++++++++----------------------------- 3 files changed, 103 insertions(+), 254 deletions(-) diff --git a/cargo-miri/Cargo.lock b/cargo-miri/Cargo.lock index 0052bfa183..bb3b05db03 100644 --- a/cargo-miri/Cargo.lock +++ b/cargo-miri/Cargo.lock @@ -45,7 +45,6 @@ dependencies = [ name = "cargo-miri" version = "0.1.0" dependencies = [ - "cargo_metadata", "directories", "rustc-workspace-hack", "rustc_version", @@ -54,17 +53,6 @@ dependencies = [ "vergen", ] -[[package]] -name = "cargo_metadata" -version = "0.11.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89fec17b16f1ac67908af82e47d0a90a7afd0e1827b181cd77504323d3263d35" -dependencies = [ - "semver 0.10.0", - "serde", - "serde_json", -] - [[package]] name = "cfg-if" version = "0.1.10" @@ -228,7 +216,7 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver 0.9.0", + "semver", ] [[package]] @@ -246,16 +234,6 @@ dependencies = [ "semver-parser", ] -[[package]] -name = "semver" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "394cec28fa623e00903caf7ba4fa6fb9a0e260280bb8cdbbba029611108a0190" -dependencies = [ - "semver-parser", - "serde", -] - [[package]] name = "semver-parser" version = "0.7.0" diff --git a/cargo-miri/Cargo.toml b/cargo-miri/Cargo.toml index 91c4783694..2de581c1c2 100644 --- a/cargo-miri/Cargo.toml +++ b/cargo-miri/Cargo.toml @@ -14,7 +14,6 @@ test = false # we have no unit tests doctest = false # and no doc tests [dependencies] -cargo_metadata = "0.11" directories = "2.0" rustc_version = "0.2.3" serde_json = "1.0.40" diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 98304d247f..caf021e911 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -116,50 +116,6 @@ fn xargo_check() -> Command { Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check"))) } -fn list_targets() -> impl Iterator { - // We need to get the manifest, and then the metadata, to enumerate targets. - let manifest_path = - get_arg_flag_value("--manifest-path").map(|m| Path::new(&m).canonicalize().unwrap()); - - let mut cmd = cargo_metadata::MetadataCommand::new(); - if let Some(manifest_path) = &manifest_path { - cmd.manifest_path(manifest_path); - } - let mut metadata = if let Ok(metadata) = cmd.exec() { - metadata - } else { - show_error(format!("Could not obtain Cargo metadata; likely an ill-formed manifest")); - }; - - let current_dir = std::env::current_dir(); - - let package_index = metadata - .packages - .iter() - .position(|package| { - let package_manifest_path = Path::new(&package.manifest_path); - if let Some(manifest_path) = &manifest_path { - package_manifest_path == manifest_path - } else { - let current_dir = current_dir.as_ref().expect("could not read current directory"); - let package_manifest_directory = package_manifest_path - .parent() - .expect("could not find parent directory of package manifest"); - package_manifest_directory == current_dir - } - }) - .unwrap_or_else(|| { - show_error(format!( - "this seems to be a workspace, which is not supported by `cargo miri`.\n\ - Try to `cd` into the crate you want to test, and re-run `cargo miri` there." - )) - }); - let package = metadata.packages.remove(package_index); - - // Finally we got the list of targets to build - package.targets.into_iter() -} - fn xargo_version() -> Option<(u32, u32, u32)> { let out = xargo_check().arg("--version").output().ok()?; if !out.status.success() { @@ -378,173 +334,77 @@ path = "lib.rs" } } -enum CargoTargets { - All, - Filtered { lib: bool, bin: Vec, test: Vec }, -} - -impl CargoTargets { - fn matches(&self, kind: &str, name: &str) -> bool { - match self { - CargoTargets::All => true, - CargoTargets::Filtered { lib, bin, test } => match kind { - "lib" => *lib, - "bin" => bin.iter().any(|n| n == name), - "test" => test.iter().any(|n| n == name), - _ => false, - }, - } - } -} - -fn parse_cargo_miri_args( - mut args: impl Iterator, -) -> (CargoTargets, Vec, Vec) { - let mut lib_present = false; - let mut bin_targets = Vec::new(); - let mut test_targets = Vec::new(); - let mut additional_args = Vec::new(); - while let Some(arg) = args.next() { - match arg { - arg if arg == "--" => { - // Miri arguments begin after the first "--". - break; - } - arg if arg == "--lib" => lib_present = true, - arg if arg == "--bin" => { - if let Some(binary) = args.next() { - if binary == "--" { - show_error(format!("\"--bin\" takes one argument.")); - } else { - bin_targets.push(binary) - } - } else { - show_error(format!("\"--bin\" takes one argument.")); - } - } - arg if arg.starts_with("--bin=") => bin_targets.push((&arg["--bin=".len()..]).to_string()), - arg if arg == "--test" => { - if let Some(test) = args.next() { - if test == "--" { - show_error(format!("\"--test\" takes one argument.")); - } else { - test_targets.push(test) - } - } else { - show_error(format!("\"--test\" takes one argument.")); - } - } - arg if arg.starts_with("--test=") => test_targets.push((&arg["--test=".len()..]).to_string()), - other => additional_args.push(other), - } - } - let targets = if !lib_present && bin_targets.len() == 0 && test_targets.len() == 0 { - CargoTargets::All - } else { - CargoTargets::Filtered { lib: lib_present, bin: bin_targets, test: test_targets } - }; - (targets, additional_args, args.collect()) -} - -fn in_cargo_miri() { - let (subcommand, skip) = match std::env::args().nth(2).as_deref() { - Some("test") => (MiriCommand::Test, 3), - Some("run") => (MiriCommand::Run, 3), - Some("setup") => (MiriCommand::Setup, 3), - // Default command, if there is an option or nothing. - Some(s) if s.starts_with("-") => (MiriCommand::Run, 2), - None => (MiriCommand::Run, 2), +fn phase_cargo_miri(mut args: env::Args) { + // Require a subcommand before any flags. + // We cannot know which of those flags take arguments and which do not, + // so we cannot detect subcommands later. + let subcommand = match args.next().as_deref() { + Some("test") => MiriCommand::Test, + Some("run") => MiriCommand::Run, + Some("setup") => MiriCommand::Setup, // Invalid command. - Some(s) => show_error(format!("Unknown command `{}`", s)), + None => show_error(format!("`cargo miri` must be immediately followed by `test`, `run`, or `setup`.")), + Some(s) => show_error(format!("unknown command `{}`", s)), }; let verbose = has_arg_flag("-v"); // We always setup. setup(subcommand); - if subcommand == MiriCommand::Setup { - // Stop here. - return; - } - // FIXME: this accepts --test, --lib, and multiple --bin for `cargo miri run`. - let (target_filters, cargo_args, miri_args) = - parse_cargo_miri_args(std::env::args().skip(skip)); - - // Now run the command. - for target in list_targets() { - let kind = target - .kind - .get(0) - .expect("badly formatted cargo metadata: target::kind is an empty array"); - if !target_filters.matches(kind, &target.name) { - continue; - } - // Now we run `cargo check $FLAGS $ARGS`, giving the user the - // change to add additional arguments. `FLAGS` is set to identify - // this target. The user gets to control what gets actually passed to Miri. - let mut cmd = cargo(); - cmd.arg("check"); - match (subcommand, kind.as_str()) { - (MiriCommand::Run, "bin") => { - // FIXME: we default to running all binaries here. - cmd.arg("--bin").arg(target.name); - } - (MiriCommand::Test, "test") => { - cmd.arg("--test").arg(target.name); - } - (MiriCommand::Test, "lib") => { - // There can be only one lib. - cmd.arg("--lib").arg("--profile").arg("test"); - } - (MiriCommand::Test, "bin") => { - cmd.arg("--bin").arg(target.name).arg("--profile").arg("test"); - } - // The remaining targets we do not even want to build. - _ => continue, - } - // Forward further `cargo` args. - for arg in cargo_args.iter() { - cmd.arg(arg); - } - // We want to always run `cargo` with `--target`. This later helps us detect - // which crates are proc-macro/build-script (host crates) and which crates are - // needed for the program itself. - if get_arg_flag_value("--target").is_none() { - // When no `--target` is given, default to the host. - cmd.arg("--target"); - cmd.arg(version_info().host); - } + // Invoke actual cargo for the job, but with different flags. + let miri_path = std::env::current_exe().expect("current executable path invalid"); + let cargo_cmd = match subcommand { + MiriCommand::Test => "test", + MiriCommand::Run => "run", + MiriCommand::Setup => return, // `cargo miri setup` stops here. + }; + let mut cmd = cargo(); + cmd.arg(cargo_cmd); + + // Make sure we know the build target, and cargo does, too. + // This is needed to make the `CARGO_TARGET_*_RUNNER` env var do something, + // and it later helps us detect which crates are proc-macro/build-script + // (host crates) and which crates are needed for the program itself. + let target = if let Some(target) = get_arg_flag_value("--target") { + target + } else { + // No target given. Pick default and tell cargo about it. + let host = version_info().host; + cmd.arg("--target"); + cmd.arg(&host); + host + }; - // Serialize the remaining args into a special environemt variable. - // This will be read by `inside_cargo_rustc` when we go to invoke - // our actual target crate (the binary or the test we are running). - // Since we're using "cargo check", we have no other way of passing - // these arguments. - cmd.env("MIRI_ARGS", serde_json::to_string(&miri_args).expect("failed to serialize args")); - - // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, - // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish - // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) - if env::var_os("RUSTC_WRAPPER").is_some() { - println!("WARNING: Ignoring existing `RUSTC_WRAPPER` environment variable, Miri does not support wrapping."); - } - let path = std::env::current_exe().expect("current executable path invalid"); - cmd.env("RUSTC_WRAPPER", path); - if verbose { - cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. - eprintln!("+ {:?}", cmd); - } + // Forward all further arguments. + cmd.args(args); - let exit_status = - cmd.spawn().expect("could not run cargo").wait().expect("failed to wait for cargo?"); + // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, + // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish + // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) + if env::var_os("RUSTC_WRAPPER").is_some() { + println!("WARNING: Ignoring `RUSTC_WRAPPER` environment variable, Miri does not support wrapping."); + } + cmd.env("RUSTC_WRAPPER", &miri_path); + if verbose { + eprintln!("+ RUSTC_WRAPPER={:?}", miri_path); + } - if !exit_status.success() { - std::process::exit(exit_status.code().unwrap_or(-1)) - } + // Set the runner for the current target to us as well, so we can interpret the binaries. + let runner_env_name = format!("CARGO_TARGET_{}_RUNNER", target.to_uppercase().replace('-', "_")); + cmd.env(runner_env_name, &miri_path); + + // Run cargo. + if verbose { + cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. + eprintln!("+ {:?}", cmd); } + let exit_status = + cmd.spawn().expect("could not run cargo").wait().expect("failed to wait for cargo?"); + + std::process::exit(exit_status.code().unwrap_or(-1)) } -fn inside_cargo_rustc() { +fn phase_cargo_rustc(mut args: env::Args) { /// Determines if we are being invoked (as rustc) to build a crate for /// the "target" architecture, in contrast to the "host" architecture. /// Host crates are for build scripts and proc macros and still need to @@ -566,15 +426,35 @@ fn inside_cargo_rustc() { fn is_runnable_crate() -> bool { let is_bin = get_arg_flag_value("--crate-type").as_deref() == Some("bin"); let is_test = has_arg_flag("--test"); - is_bin || is_test + let print = get_arg_flag_value("--print").is_some(); + (is_bin || is_test) && !print } let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let target_crate = is_target_crate(); + if target_crate && is_runnable_crate() { + // This is the binary or test crate that we want to interpret under Miri. + // But we cannot run it here, as cargo invoked us as a compiler -- our stdin and stdout are not + // like we want them. + // Instead of compiling, we write JSON into the output file with all the relevant command-line flags + // and environment variables; this is sued alter when cargo calls us again in the CARGO_TARGET_RUNNER phase. + let filename = format!( + "{}/{}{}", + get_arg_flag_value("--out-dir").unwrap(), + 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()), + ); + eprintln!("Miri is supposed to run {}", filename); + return; + } + let mut cmd = miri(); // Forward arguments. - cmd.args(std::env::args().skip(2)); // skip `cargo-miri rustc` + cmd.args(args); + // FIXME: Make the build check-only! // We make sure to only specify our custom Xargo sysroot for target crates - that is, // crates which are needed for interpretation by Miri. proc-macros and build scripts @@ -586,23 +466,9 @@ fn inside_cargo_rustc() { cmd.arg(sysroot); } - // If this is a runnable target crate, we want Miri to start interpretation; - // otherwise we want Miri to behave like rustc and build the crate as usual. - if target_crate && is_runnable_crate() { - // This is the binary or test crate that we want to interpret under Miri. - // (Testing `target_crate` is needed to exclude build scripts.) - // We deserialize the arguments that are meant for Miri from the special environment - // variable "MIRI_ARGS", and feed them to the 'miri' binary. - // - // `env::var` is okay here, well-formed JSON is always UTF-8. - let magic = std::env::var("MIRI_ARGS").expect("missing MIRI_ARGS"); - let miri_args: Vec = - serde_json::from_str(&magic).expect("failed to deserialize MIRI_ARGS"); - cmd.args(miri_args); - } else { - // We want to compile, not interpret. - cmd.env("MIRI_BE_RUSTC", "1"); - }; + // We want to compile, not interpret. We still use Miri to make sure the compiler version etc + // are the exact same as what is used for interpretation. + cmd.env("MIRI_BE_RUSTC", "1"); // Run it. if verbose { @@ -617,6 +483,10 @@ fn inside_cargo_rustc() { } } +fn phase_cargo_runner(binary: &str, args: env::Args) { + eprintln!("Asked to execute {}, args: {:?}", binary, args.collect::>()); +} + fn main() { // Check for version and help flags even when invoked as `cargo-miri`. if has_arg_flag("--help") || has_arg_flag("-h") { @@ -628,18 +498,20 @@ fn main() { return; } - if let Some("miri") = std::env::args().nth(1).as_deref() { - // This arm is for when `cargo miri` is called. We call `cargo check` for each applicable target, - // but with the `RUSTC` env var set to the `cargo-miri` binary so that we come back in the other branch, - // and dispatch the invocations to `rustc` and `miri`, respectively. - in_cargo_miri(); - } else if let Some("rustc") = std::env::args().nth(1).as_deref() { - // This arm is executed when `cargo-miri` runs `cargo check` with the `RUSTC_WRAPPER` env var set to itself: - // dependencies get dispatched to `rustc`, the final test/binary to `miri`. - inside_cargo_rustc(); - } else { - show_error(format!( - "`cargo-miri` must be called with either `miri` or `rustc` as first argument." - )) + let mut args = std::env::args(); + // Skip binary name. + args.next().unwrap(); + + // 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. + // - When we are executed due to RUSTC_WRAPPER, we build crates or store the flags of + // binary crates for later interpretation. + // - When we are executed due to CARGO_TARGET_RUNNER, we start interpretation based on the + // flags that were stored earlier. + match &*args.next().unwrap() { + "miri" => phase_cargo_miri(args), + "rustc" => phase_cargo_rustc(args), + binary => phase_cargo_runner(binary, args), } } From 4c80a7649c55d71920398f11ac5b73dc43d1764a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 7 Sep 2020 20:19:45 +0200 Subject: [PATCH 03/29] stub JSON information flow from cargo-build-time to run-time --- cargo-miri/bin.rs | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index caf021e911..5692041371 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -1,10 +1,13 @@ use std::env; use std::ffi::OsString; use std::fs::{self, File}; -use std::io::{self, BufRead, Write}; +use std::io::{self, BufRead, BufReader, BufWriter, Write}; use std::ops::Not; use std::path::{Path, PathBuf}; use std::process::Command; +use std::collections::HashMap; + +use serde::{Deserialize, Serialize}; use rustc_version::VersionMeta; @@ -41,6 +44,15 @@ enum MiriCommand { Setup, } +/// The inforamtion Miri needs to run a crate. Stored as JSON when the crate is "compiled". +#[derive(Serialize, Deserialize)] +struct CrateRunInfo { + /// The command-line arguments. + args: Vec, + /// The environment. + env: HashMap, +} + fn show_help() { println!("{}", CARGO_MIRI_HELP); } @@ -439,15 +451,24 @@ fn phase_cargo_rustc(mut args: env::Args) { // like we want them. // Instead of compiling, we write JSON into the output file with all the relevant command-line flags // and environment variables; this is sued alter when cargo calls us again in the CARGO_TARGET_RUNNER phase. - let filename = format!( - "{}/{}{}", - get_arg_flag_value("--out-dir").unwrap(), + let info = CrateRunInfo { args: Vec::new(), env: HashMap::new() }; + + let mut path = PathBuf::from(get_arg_flag_value("--out-dir").unwrap()); + path.push(format!( + "{}{}", 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()), - ); - eprintln!("Miri is supposed to run {}", filename); + )); + eprintln!("Miri is supposed to run {}", path.display()); + + let file = File::create(&path) + .unwrap_or_else(|_| show_error(format!("Cannot create {}", path.display()))); + let file = BufWriter::new(file); + serde_json::ser::to_writer(file, &info) + .unwrap_or_else(|_| show_error(format!("Cannot write to {}", path.display()))); + return; } @@ -485,6 +506,13 @@ fn phase_cargo_rustc(mut args: env::Args) { fn phase_cargo_runner(binary: &str, args: env::Args) { eprintln!("Asked to execute {}, args: {:?}", binary, args.collect::>()); + + let file = File::open(binary) + .unwrap_or_else(|_| show_error(format!("File {:?} not found, or cargo-miri invoked incorrectly", binary))); + let file = BufReader::new(file); + let info: CrateRunInfo = serde_json::from_reader(file) + .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); + // FIXME: remove the file. } fn main() { From bf265f449f8648325e90349415fd2109b4274e81 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 7 Sep 2020 21:12:51 +0200 Subject: [PATCH 04/29] it actually runs tests now! --- cargo-miri/bin.rs | 70 ++++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 5692041371..5005d225ec 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -5,7 +5,6 @@ use std::io::{self, BufRead, BufReader, BufWriter, Write}; use std::ops::Not; use std::path::{Path, PathBuf}; use std::process::Command; -use std::collections::HashMap; use serde::{Deserialize, Serialize}; @@ -50,7 +49,16 @@ struct CrateRunInfo { /// The command-line arguments. args: Vec, /// The environment. - env: HashMap, + env: Vec<(OsString, OsString)>, +} + +impl CrateRunInfo { + /// Gather all the information we need. + fn collect(args: env::ArgsOs) -> Self { + let args = args.collect(); + let env = env::vars_os().collect(); + CrateRunInfo { args, env } + } } fn show_help() { @@ -128,6 +136,11 @@ fn xargo_check() -> Command { Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check"))) } +fn exec(mut cmd: Command) -> ! { + let exit_status = cmd.status().expect("failed to run command"); + 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() { @@ -346,17 +359,16 @@ path = "lib.rs" } } -fn phase_cargo_miri(mut args: env::Args) { +fn phase_cargo_miri(mut args: env::ArgsOs) { // Require a subcommand before any flags. // We cannot know which of those flags take arguments and which do not, // so we cannot detect subcommands later. - let subcommand = match args.next().as_deref() { + let subcommand = match args.next().as_deref().and_then(|s| s.to_str()) { Some("test") => MiriCommand::Test, Some("run") => MiriCommand::Run, Some("setup") => MiriCommand::Setup, // Invalid command. - None => show_error(format!("`cargo miri` must be immediately followed by `test`, `run`, or `setup`.")), - Some(s) => show_error(format!("unknown command `{}`", s)), + _ => show_error(format!("`cargo miri` must be immediately followed by `test`, `run`, or `setup`.")), }; let verbose = has_arg_flag("-v"); @@ -410,13 +422,10 @@ fn phase_cargo_miri(mut args: env::Args) { cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. eprintln!("+ {:?}", cmd); } - let exit_status = - cmd.spawn().expect("could not run cargo").wait().expect("failed to wait for cargo?"); - - std::process::exit(exit_status.code().unwrap_or(-1)) + exec(cmd) } -fn phase_cargo_rustc(mut args: env::Args) { +fn phase_cargo_rustc(args: env::ArgsOs) { /// Determines if we are being invoked (as rustc) to build a crate for /// the "target" architecture, in contrast to the "host" architecture. /// Host crates are for build scripts and proc macros and still need to @@ -451,7 +460,7 @@ fn phase_cargo_rustc(mut args: env::Args) { // like we want them. // Instead of compiling, we write JSON into the output file with all the relevant command-line flags // and environment variables; this is sued alter when cargo calls us again in the CARGO_TARGET_RUNNER phase. - let info = CrateRunInfo { args: Vec::new(), env: HashMap::new() }; + let info = CrateRunInfo::collect(args); let mut path = PathBuf::from(get_arg_flag_value("--out-dir").unwrap()); path.push(format!( @@ -461,14 +470,12 @@ fn phase_cargo_rustc(mut args: env::Args) { // (and cargo passes this before the filename so it should be unique) get_arg_flag_value("extra-filename").unwrap_or(String::new()), )); - eprintln!("Miri is supposed to run {}", path.display()); let file = File::create(&path) .unwrap_or_else(|_| show_error(format!("Cannot create {}", path.display()))); let file = BufWriter::new(file); serde_json::ser::to_writer(file, &info) .unwrap_or_else(|_| show_error(format!("Cannot write to {}", path.display()))); - return; } @@ -495,17 +502,11 @@ fn phase_cargo_rustc(mut args: env::Args) { if verbose { eprintln!("+ {:?}", cmd); } - match cmd.status() { - Ok(exit) => - if !exit.success() { - std::process::exit(exit.code().unwrap_or(42)); - }, - Err(e) => panic!("error running {:?}:\n{:?}", cmd, e), - } + exec(cmd) } -fn phase_cargo_runner(binary: &str, args: env::Args) { - eprintln!("Asked to execute {}, args: {:?}", binary, args.collect::>()); +fn phase_cargo_runner(binary: &str, args: env::ArgsOs) { + let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let file = File::open(binary) .unwrap_or_else(|_| show_error(format!("File {:?} not found, or cargo-miri invoked incorrectly", binary))); @@ -513,6 +514,24 @@ fn phase_cargo_runner(binary: &str, args: env::Args) { let info: CrateRunInfo = serde_json::from_reader(file) .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); // FIXME: remove the file. + + let mut cmd = miri(); + // Forward rustc arguments,with our sysroot. + cmd.args(info.args); + let sysroot = + env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); + cmd.arg("--sysroot"); + cmd.arg(sysroot); + + // Then pass binary arguments. + cmd.arg("--"); + cmd.args(args); + + // Run it. + if verbose { + eprintln!("+ {:?}", cmd); + } + exec(cmd) } fn main() { @@ -526,7 +545,7 @@ fn main() { return; } - let mut args = std::env::args(); + let mut args = std::env::args_os(); // Skip binary name. args.next().unwrap(); @@ -537,7 +556,8 @@ fn main() { // binary crates for later interpretation. // - When we are executed due to CARGO_TARGET_RUNNER, we start interpretation based on the // flags that were stored earlier. - match &*args.next().unwrap() { + // FIXME: report errors for these unwraps. + match &*args.next().unwrap().to_str().unwrap() { "miri" => phase_cargo_miri(args), "rustc" => phase_cargo_rustc(args), binary => phase_cargo_runner(binary, args), From c2f1b5679189857a7029ddfa69e61cd9831bd99a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 08:58:29 +0200 Subject: [PATCH 05/29] patch --extern and --emit; test suite passes now! --- cargo-miri/bin.rs | 98 ++++++++++++++++++++++++--------- test-cargo-miri/run-test.py | 25 +++++---- test-cargo-miri/test.stdout.ref | 12 +--- 3 files changed, 90 insertions(+), 45 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 5005d225ec..2d107a6cce 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -47,14 +47,14 @@ enum MiriCommand { #[derive(Serialize, Deserialize)] struct CrateRunInfo { /// The command-line arguments. - args: Vec, + args: Vec, /// The environment. env: Vec<(OsString, OsString)>, } impl CrateRunInfo { /// Gather all the information we need. - fn collect(args: env::ArgsOs) -> Self { + fn collect(args: env::Args) -> Self { let args = args.collect(); let env = env::vars_os().collect(); CrateRunInfo { args, env } @@ -359,11 +359,11 @@ path = "lib.rs" } } -fn phase_cargo_miri(mut args: env::ArgsOs) { +fn phase_cargo_miri(mut args: env::Args) { // Require a subcommand before any flags. // We cannot know which of those flags take arguments and which do not, // so we cannot detect subcommands later. - let subcommand = match args.next().as_deref().and_then(|s| s.to_str()) { + let subcommand = match args.next().as_deref() { Some("test") => MiriCommand::Test, Some("run") => MiriCommand::Run, Some("setup") => MiriCommand::Setup, @@ -420,12 +420,12 @@ fn phase_cargo_miri(mut args: env::ArgsOs) { // Run cargo. if verbose { cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. - eprintln!("+ {:?}", cmd); + eprintln!("[cargo-miri miri] {:?}", cmd); } exec(cmd) } -fn phase_cargo_rustc(args: env::ArgsOs) { +fn phase_cargo_rustc(args: env::Args) { /// Determines if we are being invoked (as rustc) to build a crate for /// the "target" architecture, in contrast to the "host" architecture. /// Host crates are for build scripts and proc macros and still need to @@ -470,19 +470,41 @@ fn phase_cargo_rustc(args: env::ArgsOs) { // (and cargo passes this before the filename so it should be unique) get_arg_flag_value("extra-filename").unwrap_or(String::new()), )); + if verbose { + eprintln!("[cargo-miri rustc] writing run info to {:?}", path.display()); + } let file = File::create(&path) - .unwrap_or_else(|_| show_error(format!("Cannot create {}", path.display()))); + .unwrap_or_else(|_| show_error(format!("Cannot create {:?}", path.display()))); let file = BufWriter::new(file); serde_json::ser::to_writer(file, &info) - .unwrap_or_else(|_| show_error(format!("Cannot write to {}", path.display()))); + .unwrap_or_else(|_| show_error(format!("Cannot write to {:?}", path.display()))); return; } let mut cmd = miri(); - // Forward arguments. - cmd.args(args); - // FIXME: Make the build check-only! + // Forward arguments, but (only for target crates!) remove "link" from "--emit" to make this a check-only build. + let emit_flag = "--emit"; + for arg in args { + if target_crate && arg.starts_with(emit_flag) { + // Patch this argument. First, extract its value. + let val = &arg[emit_flag.len()..]; + assert!(val.starts_with("="), "`cargo` should pass `--emit=X` as one argument"); + let val = &val[1..]; + let mut val: Vec<_> = val.split(',').collect(); + // Now make sure "link" is not in there, but "metadata" is. + if let Some(i) = val.iter().position(|&s| s == "link") { + val.remove(i); + if !val.iter().any(|&s| s == "metadata") { + val.push("metadata"); + } + } + cmd.arg(format!("{}={}", emit_flag, val.join(","))); + // FIXME: due to this, the `.rlib` file does not get created and cargo re-triggers the build each time. + } else { + cmd.arg(arg); + } + } // We make sure to only specify our custom Xargo sysroot for target crates - that is, // crates which are needed for interpretation by Miri. proc-macros and build scripts @@ -500,36 +522,60 @@ fn phase_cargo_rustc(args: env::ArgsOs) { // Run it. if verbose { - eprintln!("+ {:?}", cmd); + eprintln!("[cargo-miri rustc] {:?}", cmd); } exec(cmd) } -fn phase_cargo_runner(binary: &str, args: env::ArgsOs) { +fn phase_cargo_runner(binary: &str, binary_args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let file = File::open(binary) - .unwrap_or_else(|_| show_error(format!("File {:?} not found, or cargo-miri invoked incorrectly", binary))); + .unwrap_or_else(|_| show_error(format!("File {:?} not found or `cargo-miri` invoked incorrectly; please only invoke this binary through `cargo miri`", binary))); let file = BufReader::new(file); let info: CrateRunInfo = serde_json::from_reader(file) .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); - // FIXME: remove the file. + fs::remove_file(binary) + .unwrap_or_else(|_| show_error(format!("Unable to remove file {:?}", binary))); let mut cmd = miri(); - // Forward rustc arguments,with our sysroot. - cmd.args(info.args); + // Forward rustc arguments. We need to patch "--extern" filenames because + // we forced a check-only build without cargo knowing about that: replace `.rlib` suffix by `.rmeta`. + let mut args = info.args.into_iter(); + let extern_flag = "--extern"; + while let Some(arg) = args.next() { + if arg == extern_flag { + let next_arg = args.next().expect("`--extern` should be followed by a filename"); + let next_arg = next_arg.strip_suffix(".rlib").expect("all extern filenames should end in `.rlib`"); + cmd.arg(extern_flag); + cmd.arg(format!("{}.rmeta", next_arg)); + } else { + cmd.arg(arg); + } + } + // Set sysroot. let sysroot = env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); cmd.arg("--sysroot"); cmd.arg(sysroot); + // Respect `MIRIFLAGS`. + if let Ok(a) = env::var("MIRIFLAGS") { + // This code is taken from `RUSTFLAGS` handling in cargo. + let args = a + .split(' ') + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string); + cmd.args(args); + } // Then pass binary arguments. cmd.arg("--"); - cmd.args(args); + cmd.args(binary_args); // Run it. if verbose { - eprintln!("+ {:?}", cmd); + eprintln!("[cargo-miri runner] {:?}", cmd); } exec(cmd) } @@ -545,7 +591,9 @@ fn main() { return; } - let mut args = std::env::args_os(); + // Rustc does not support non-UTF-8 arguments so we make no attempt either. + // (We do support non-UTF-8 environment variables though.) + let mut args = std::env::args(); // Skip binary name. args.next().unwrap(); @@ -556,10 +604,10 @@ fn main() { // binary crates for later interpretation. // - When we are executed due to CARGO_TARGET_RUNNER, we start interpretation based on the // flags that were stored earlier. - // FIXME: report errors for these unwraps. - match &*args.next().unwrap().to_str().unwrap() { - "miri" => phase_cargo_miri(args), - "rustc" => phase_cargo_rustc(args), - binary => phase_cargo_runner(binary, args), + match args.next().as_deref() { + Some("miri") => phase_cargo_miri(args), + Some("rustc") => phase_cargo_rustc(args), + Some(binary) => phase_cargo_runner(binary, args), + _ => show_error(format!("`cargo-miri` called without first argument; please only invoke this binary through `cargo miri`")), } } diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index a258c7f73c..6a28f1b403 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -21,13 +21,16 @@ def cargo_miri(cmd): args += ["--target", os.environ['MIRI_TEST_TARGET']] return args -def test(name, cmd, stdout_ref, stderr_ref): +def test(name, cmd, stdout_ref, stderr_ref, env={}): print("==> Testing `{}` <==".format(name)) ## Call `cargo miri`, capture all output + p_env = os.environ.copy() + p_env.update(env) p = subprocess.Popen( cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, + env=p_env, ) (stdout, stderr) = p.communicate() stdout = stdout.decode("UTF-8") @@ -55,29 +58,31 @@ def test_cargo_miri_run(): "stdout.ref", "stderr.ref" ) test("cargo miri run (with arguments)", - cargo_miri("run") + ["--", "--", "hello world", '"hello world"'], + cargo_miri("run") + ["--", "hello world", '"hello world"'], "stdout.ref", "stderr.ref2" ) def test_cargo_miri_test(): test("cargo miri test", - cargo_miri("test") + ["--", "-Zmiri-seed=feed"], - "test.stdout.ref", "test.stderr.ref" + cargo_miri("test"), + "test.stdout.ref", "test.stderr.ref", + env={'MIRIFLAGS': "-Zmiri-seed=feed"}, ) test("cargo miri test (with filter)", - cargo_miri("test") + ["--", "--", "le1"], + cargo_miri("test") + ["--", "--format=pretty", "le1"], "test.stdout.ref2", "test.stderr.ref" ) test("cargo miri test (without isolation)", - cargo_miri("test") + ["--", "-Zmiri-disable-isolation", "--", "num_cpus"], - "test.stdout.ref3", "test.stderr.ref" + cargo_miri("test") + ["--", "--format=pretty", "num_cpus"], + "test.stdout.ref3", "test.stderr.ref", + env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) test("cargo miri test (test target)", - cargo_miri("test") + ["--test", "test"], + cargo_miri("test") + ["--test", "test", "--", "--format=pretty"], "test.stdout.ref4", "test.stderr.ref" ) test("cargo miri test (bin target)", - cargo_miri("test") + ["--bin", "cargo-miri-test"], + cargo_miri("test") + ["--bin", "cargo-miri-test", "--", "--format=pretty"], "test.stdout.ref5", "test.stderr.ref" ) diff --git a/test-cargo-miri/test.stdout.ref b/test-cargo-miri/test.stdout.ref index 4260f5b3cb..fa78cd3548 100644 --- a/test-cargo-miri/test.stdout.ref +++ b/test-cargo-miri/test.stdout.ref @@ -1,18 +1,10 @@ running 1 test -test test::rng ... ok - +. test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out running 7 tests -test do_panic ... ok -test does_not_work_on_miri ... ignored -test entropy_rng ... ok -test fail_index_check ... ok -test num_cpus ... ok -test simple1 ... ok -test simple2 ... ok - +.i..... test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out From 44d1d96fe6d3a82bf16413cc7f09356b71df237b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 8 Sep 2020 09:56:21 +0200 Subject: [PATCH 06/29] test respecting 'test=false', and what happens with doctests --- test-cargo-miri/Cargo.toml | 4 ++++ test-cargo-miri/src/lib.rs | 7 +++++++ 2 files changed, 11 insertions(+) create mode 100644 test-cargo-miri/src/lib.rs diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 3abb437049..68970d7d16 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -10,3 +10,7 @@ byteorder = "1.0" [dev-dependencies] rand = { version = "0.7", features = ["small_rng"] } num_cpus = "1.10.1" + +[lib] +test = false +doctest = false # FIXME: doctests should be skipped automatically until we can run them... diff --git a/test-cargo-miri/src/lib.rs b/test-cargo-miri/src/lib.rs new file mode 100644 index 0000000000..064954ba98 --- /dev/null +++ b/test-cargo-miri/src/lib.rs @@ -0,0 +1,7 @@ +/// Doc-test test +/// ```rust +/// assert!(true); +/// ``` +pub fn make_true() -> bool { + true +} From cfa8eb25870a864b1f8f70377b938cd017874bd3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 8 Sep 2020 10:01:18 +0200 Subject: [PATCH 07/29] update docs, and also use MIRIFLAGS for the test suite --- README.md | 16 ++++++++-------- ci.sh | 2 +- tests/compiletest.rs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index d847486870..67161974b1 100644 --- a/README.md +++ b/README.md @@ -83,11 +83,10 @@ Now you can run your project in Miri: The first time you run Miri, it will perform some extra setup and install some dependencies. It will ask you for confirmation before installing anything. -You can pass arguments to Miri after the first `--`, and pass arguments to the -interpreted program or test suite after the second `--`. For example, `cargo -miri run -- -Zmiri-disable-stacked-borrows` runs the program without checking -the aliasing of references. To filter the tests being run, use `cargo miri test --- -- filter`. +`cargo miri run/test` supports the exact same flags as `cargo run/test`. You +can pass arguments to Miri via `MIRIFLAGS`. For example, +`MIRIFLAGS="-Zmiri-disable-stacked-borrows" cargo miri run` runs the program +without checking the aliasing of references. Miri supports cross-execution: if you want to run the program as if it was a Linux program, you can do `cargo miri run --target x86_64-unknown-linux-gnu`. @@ -163,7 +162,8 @@ up the sysroot. If you are using `miri` (the Miri driver) directly, see the ## Miri `-Z` flags and environment variables [miri-flags]: #miri--z-flags-and-environment-variables -Miri adds its own set of `-Z` flags: +Miri adds its own set of `-Z` flags, which are usually set via the `MIRIFLAGS` +environment variable: * `-Zmiri-disable-alignment-check` disables checking pointer alignment, so you can focus on other failures, but it means Miri can miss bugs in your program. @@ -229,14 +229,14 @@ Moreover, Miri recognizes some environment variables: * `MIRI_LOG`, `MIRI_BACKTRACE` control logging and backtrace printing during Miri executions, also [see above][testing-miri]. +* `MIRIFLAGS` (recognized by `cargo miri` and the test suite) defines extra + flags to be passed to Miri. * `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the sysroot to use. To do the same thing with `miri` directly, use the `--sysroot` flag. * `MIRI_TEST_TARGET` (recognized by the test suite) indicates which target architecture to test against. `miri` and `cargo miri` accept the `--target` flag for the same purpose. -* `MIRI_TEST_FLAGS` (recognized by the test suite) defines extra flags to be - passed to Miri. The following environment variables are internal, but used to communicate between different Miri binaries, and as such worth documenting: diff --git a/ci.sh b/ci.sh index 915a4cf2fd..12683a2fcc 100755 --- a/ci.sh +++ b/ci.sh @@ -26,7 +26,7 @@ function run_tests { if ! [ -n "${MIRI_TEST_TARGET+exists}" ]; then # Only for host architecture: tests with MIR optimizations # FIXME: only testing level 2 because of . - MIRI_TEST_FLAGS="-Z mir-opt-level=2" ./miri test --locked + MIRIFLAGS="-Z mir-opt-level=2" ./miri test --locked fi # "miri test" has built the sysroot for us, now this should pass without # any interactive questions. diff --git a/tests/compiletest.rs b/tests/compiletest.rs index a64f0edb94..35c1de3399 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -27,7 +27,7 @@ fn run_tests(mode: &str, path: &str, target: &str) { if let Ok(sysroot) = std::env::var("MIRI_SYSROOT") { flags.push(format!("--sysroot {}", sysroot)); } - if let Ok(extra_flags) = std::env::var("MIRI_TEST_FLAGS") { + if let Ok(extra_flags) = std::env::var("MIRIFLAGS") { flags.push(extra_flags); } From 2cc39937d07c94a5ba73d69728f968f00bd4deb1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 8 Sep 2020 11:08:46 +0200 Subject: [PATCH 08/29] fix typo Co-authored-by: Oli Scherer --- cargo-miri/bin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 2d107a6cce..0ef971a376 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -459,7 +459,7 @@ fn phase_cargo_rustc(args: env::Args) { // But we cannot run it here, as cargo invoked us as a compiler -- our stdin and stdout are not // like we want them. // Instead of compiling, we write JSON into the output file with all the relevant command-line flags - // and environment variables; this is sued alter when cargo calls us again in the CARGO_TARGET_RUNNER phase. + // and environment variables; this is used when cargo calls us again in the CARGO_TARGET_RUNNER phase. let info = CrateRunInfo::collect(args); let mut path = PathBuf::from(get_arg_flag_value("--out-dir").unwrap()); From 1fd1ad55d297238e89b78eb0e382cb647d73fbcf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 8 Sep 2020 20:08:11 +0200 Subject: [PATCH 09/29] even when not linking, create stub .rlib files to fool cargo --- cargo-miri/bin.rs | 99 +++++++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 38 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 0ef971a376..a788745468 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -136,9 +136,13 @@ fn xargo_check() -> Command { Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check"))) } -fn exec(mut cmd: Command) -> ! { +/// Execute the command if it fails, fail this process with the same exit code. +/// Otherwise, continue. +fn exec(mut cmd: Command) { let exit_status = cmd.status().expect("failed to run command"); - std::process::exit(exit_status.code().unwrap_or(-1)) + if exit_status.success().not() { + std::process::exit(exit_status.code().unwrap_or(-1)) + } } fn xargo_version() -> Option<(u32, u32, u32)> { @@ -451,6 +455,20 @@ fn phase_cargo_rustc(args: env::Args) { (is_bin || is_test) && !print } + 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 + } + let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let target_crate = is_target_crate(); @@ -461,59 +479,57 @@ fn phase_cargo_rustc(args: env::Args) { // Instead of compiling, we write JSON into the output file with all the relevant command-line flags // and environment variables; this is used when cargo calls us again in the CARGO_TARGET_RUNNER phase. let info = CrateRunInfo::collect(args); + // FIXME: Windows might need a ".exe" suffix. + let filename = out_filename("", ""); - let mut path = PathBuf::from(get_arg_flag_value("--out-dir").unwrap()); - path.push(format!( - "{}{}", - 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()), - )); if verbose { - eprintln!("[cargo-miri rustc] writing run info to {:?}", path.display()); + eprintln!("[cargo-miri rustc] writing run info to {:?}", filename.display()); } - let file = File::create(&path) - .unwrap_or_else(|_| show_error(format!("Cannot create {:?}", path.display()))); + let file = File::create(&filename) + .unwrap_or_else(|_| show_error(format!("Cannot create {:?}", filename.display()))); let file = BufWriter::new(file); serde_json::ser::to_writer(file, &info) - .unwrap_or_else(|_| show_error(format!("Cannot write to {:?}", path.display()))); + .unwrap_or_else(|_| show_error(format!("Cannot write to {:?}", filename.display()))); return; } let mut cmd = miri(); - // Forward arguments, but (only for target crates!) remove "link" from "--emit" to make this a check-only build. - let emit_flag = "--emit"; - for arg in args { - if target_crate && arg.starts_with(emit_flag) { - // Patch this argument. First, extract its value. - let val = &arg[emit_flag.len()..]; - assert!(val.starts_with("="), "`cargo` should pass `--emit=X` as one argument"); - let val = &val[1..]; - let mut val: Vec<_> = val.split(',').collect(); - // Now make sure "link" is not in there, but "metadata" is. - if let Some(i) = val.iter().position(|&s| s == "link") { - val.remove(i); - if !val.iter().any(|&s| s == "metadata") { - val.push("metadata"); + let mut emit_link_hack = false; + // Arguments are treated very differently depending on whether this crate is + // for interpretation by Miri, or for use by a build script / proc macro. + if target_crate { + // Forward arguments, butremove "link" from "--emit" to make this a check-only build. + let emit_flag = "--emit"; + for arg in args { + if arg.starts_with(emit_flag) { + // Patch this argument. First, extract its value. + let val = &arg[emit_flag.len()..]; + assert!(val.starts_with("="), "`cargo` should pass `--emit=X` as one argument"); + let val = &val[1..]; + let mut val: Vec<_> = val.split(',').collect(); + // Now make sure "link" is not in there, but "metadata" is. + if let Some(i) = val.iter().position(|&s| s == "link") { + emit_link_hack = true; + val.remove(i); + if !val.iter().any(|&s| s == "metadata") { + val.push("metadata"); + } } + cmd.arg(format!("{}={}", emit_flag, val.join(","))); + } else { + cmd.arg(arg); } - cmd.arg(format!("{}={}", emit_flag, val.join(","))); - // FIXME: due to this, the `.rlib` file does not get created and cargo re-triggers the build each time. - } else { - cmd.arg(arg); } - } - // We make sure to only specify our custom Xargo sysroot for target crates - that is, - // crates which are needed for interpretation by Miri. proc-macros and build scripts - // should use the default sysroot. - if target_crate { + // Use our custom sysroot. let sysroot = env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); cmd.arg("--sysroot"); cmd.arg(sysroot); + } else { + // For host crates, just forward everything. + cmd.args(args); } // We want to compile, not interpret. We still use Miri to make sure the compiler version etc @@ -524,7 +540,14 @@ fn phase_cargo_rustc(args: env::Args) { if verbose { eprintln!("[cargo-miri rustc] {:?}", cmd); } - exec(cmd) + exec(cmd); + + // Create a stub .rlib file if "link" was requested by cargo. + if emit_link_hack { + // FIXME: is "lib" always right? + let filename = out_filename("lib", ".rlib"); + File::create(filename).expect("Failed to create rlib file"); + } } fn phase_cargo_runner(binary: &str, binary_args: env::Args) { From 15e1baff3becf29012377e14857d6b92339da93c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 8 Sep 2020 20:18:08 +0200 Subject: [PATCH 10/29] make our filename handling work better across platforms --- cargo-miri/bin.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index a788745468..9fdd763a07 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -483,14 +483,14 @@ fn phase_cargo_rustc(args: env::Args) { let filename = out_filename("", ""); if verbose { - eprintln!("[cargo-miri rustc] writing run info to {:?}", filename.display()); + eprintln!("[cargo-miri rustc] writing run info to `{}`", filename.display()); } let file = File::create(&filename) - .unwrap_or_else(|_| show_error(format!("Cannot create {:?}", filename.display()))); + .unwrap_or_else(|_| show_error(format!("Cannot create `{}`", filename.display()))); let file = BufWriter::new(file); serde_json::ser::to_writer(file, &info) - .unwrap_or_else(|_| show_error(format!("Cannot write to {:?}", filename.display()))); + .unwrap_or_else(|_| show_error(format!("Cannot write to `{}`", filename.display()))); return; } @@ -499,7 +499,7 @@ fn phase_cargo_rustc(args: env::Args) { // Arguments are treated very differently depending on whether this crate is // for interpretation by Miri, or for use by a build script / proc macro. if target_crate { - // Forward arguments, butremove "link" from "--emit" to make this a check-only build. + // Forward arguments, but remove "link" from "--emit" to make this a check-only build. let emit_flag = "--emit"; for arg in args { if arg.starts_with(emit_flag) { @@ -544,21 +544,26 @@ fn phase_cargo_rustc(args: env::Args) { // Create a stub .rlib file if "link" was requested by cargo. if emit_link_hack { - // FIXME: is "lib" always right? + // Some platforms prepend "lib", some do not... let's just create both files. let filename = out_filename("lib", ".rlib"); File::create(filename).expect("Failed to create rlib file"); + let filename = out_filename("", ".rlib"); + File::create(filename).expect("Failed to create rlib file"); } } fn phase_cargo_runner(binary: &str, binary_args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); - let file = File::open(binary) + // Strip extension from binary name (Windows adds ".exe"). + let mut filename = PathBuf::from(binary); + filename.set_extension(""); + let file = File::open(&filename) .unwrap_or_else(|_| show_error(format!("File {:?} not found or `cargo-miri` invoked incorrectly; please only invoke this binary through `cargo miri`", binary))); let file = BufReader::new(file); let info: CrateRunInfo = serde_json::from_reader(file) .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); - fs::remove_file(binary) + fs::remove_file(&filename) .unwrap_or_else(|_| show_error(format!("Unable to remove file {:?}", binary))); let mut cmd = miri(); From 531211cd86219be9153765eb0138cbd464036808 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 01:00:09 +0200 Subject: [PATCH 11/29] handle binary suffices (for Windows); stop deleting fake binary --- cargo-miri/bin.rs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 9fdd763a07..f02777b70d 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -59,6 +59,14 @@ impl CrateRunInfo { let env = env::vars_os().collect(); CrateRunInfo { args, env } } + + fn store(&self, filename: &Path) { + let file = File::create(filename) + .unwrap_or_else(|_| show_error(format!("Cannot create `{}`", filename.display()))); + let file = BufWriter::new(file); + serde_json::ser::to_writer(file, self) + .unwrap_or_else(|_| show_error(format!("Cannot write to `{}`", filename.display()))); + } } fn show_help() { @@ -479,18 +487,16 @@ fn phase_cargo_rustc(args: env::Args) { // Instead of compiling, we write JSON into the output file with all the relevant command-line flags // and environment variables; this is used when cargo calls us again in the CARGO_TARGET_RUNNER phase. let info = CrateRunInfo::collect(args); - // FIXME: Windows might need a ".exe" suffix. let filename = out_filename("", ""); - if verbose { eprintln!("[cargo-miri rustc] writing run info to `{}`", filename.display()); } - let file = File::create(&filename) - .unwrap_or_else(|_| show_error(format!("Cannot create `{}`", filename.display()))); - let file = BufWriter::new(file); - serde_json::ser::to_writer(file, &info) - .unwrap_or_else(|_| show_error(format!("Cannot write to `{}`", filename.display()))); + info.store(&filename); + // For Windows, do the same thing again with `.exe` appended to the filename. + // (Need to do this here as cargo moves that "binary" to a different place before running it.) + info.store(&out_filename("", ".exe")); + return; } @@ -555,16 +561,11 @@ fn phase_cargo_rustc(args: env::Args) { fn phase_cargo_runner(binary: &str, binary_args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); - // Strip extension from binary name (Windows adds ".exe"). - let mut filename = PathBuf::from(binary); - filename.set_extension(""); - let file = File::open(&filename) + let file = File::open(&binary) .unwrap_or_else(|_| show_error(format!("File {:?} not found or `cargo-miri` invoked incorrectly; please only invoke this binary through `cargo miri`", binary))); let file = BufReader::new(file); let info: CrateRunInfo = serde_json::from_reader(file) .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); - fs::remove_file(&filename) - .unwrap_or_else(|_| show_error(format!("Unable to remove file {:?}", binary))); let mut cmd = miri(); // Forward rustc arguments. We need to patch "--extern" filenames because @@ -590,10 +591,10 @@ fn phase_cargo_runner(binary: &str, binary_args: env::Args) { if let Ok(a) = env::var("MIRIFLAGS") { // This code is taken from `RUSTFLAGS` handling in cargo. let args = a - .split(' ') - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(str::to_string); + .split(' ') + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string); cmd.args(args); } From f56cd06068b2d099841ae502b2ddb300b2c5098f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 08:40:06 +0200 Subject: [PATCH 12/29] fix Miri script on macOS --- miri | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/miri b/miri index f53c21ff11..aef61b6dd6 100755 --- a/miri +++ b/miri @@ -39,7 +39,11 @@ EOF TARGET=$(rustc --version --verbose | grep "^host:" | cut -d ' ' -f 2) SYSROOT=$(rustc --print sysroot) LIBDIR=$SYSROOT/lib/rustlib/$TARGET/lib -MIRIDIR=$(readlink -e "$(dirname "$0")") +MIRIDIR=$(dirname "$0") +if readlink -e . >/dev/null; then + # This platform supports `readlink -e`. + MIRIDIR=$(readlink -e "$MIRIDIR") +fi if ! test -d "$LIBDIR"; then echo "Something went wrong determining the library dir." echo "I got $LIBDIR but that does not exist." From 1f42b4b35be1dc4f3cd0435387a6495bc1220bec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 08:51:41 +0200 Subject: [PATCH 13/29] forward build-time env vars to binary, and test that we do --- cargo-miri/bin.rs | 27 +++++++++++++++++---------- test-cargo-miri/test.stdout.ref | 6 +++--- test-cargo-miri/test.stdout.ref2 | 2 +- test-cargo-miri/test.stdout.ref3 | 2 +- test-cargo-miri/test.stdout.ref4 | 5 +++-- test-cargo-miri/tests/test.rs | 5 +++++ 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index f02777b70d..1edeaade09 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -372,6 +372,16 @@ path = "lib.rs" } fn phase_cargo_miri(mut args: env::Args) { + // Check for version and help flags even when invoked as `cargo-miri`. + if has_arg_flag("--help") || has_arg_flag("-h") { + show_help(); + return; + } + if has_arg_flag("--version") || has_arg_flag("-V") { + show_version(); + return; + } + // Require a subcommand before any flags. // We cannot know which of those flags take arguments and which do not, // so we cannot detect subcommands later. @@ -567,6 +577,13 @@ fn phase_cargo_runner(binary: &str, binary_args: env::Args) { let info: CrateRunInfo = serde_json::from_reader(file) .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); + // Set missing env vars. + for (name, val) in info.env { + if env::var_os(&name).is_none() { + env::set_var(name, val); + } + } + let mut cmd = miri(); // Forward rustc arguments. We need to patch "--extern" filenames because // we forced a check-only build without cargo knowing about that: replace `.rlib` suffix by `.rmeta`. @@ -610,16 +627,6 @@ fn phase_cargo_runner(binary: &str, binary_args: env::Args) { } fn main() { - // Check for version and help flags even when invoked as `cargo-miri`. - if has_arg_flag("--help") || has_arg_flag("-h") { - show_help(); - return; - } - if has_arg_flag("--version") || has_arg_flag("-V") { - show_version(); - return; - } - // Rustc does not support non-UTF-8 arguments so we make no attempt either. // (We do support non-UTF-8 environment variables though.) let mut args = std::env::args(); diff --git a/test-cargo-miri/test.stdout.ref b/test-cargo-miri/test.stdout.ref index fa78cd3548..1eb18fe887 100644 --- a/test-cargo-miri/test.stdout.ref +++ b/test-cargo-miri/test.stdout.ref @@ -4,7 +4,7 @@ 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 +running 8 tests +..i..... +test result: ok. 7 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out diff --git a/test-cargo-miri/test.stdout.ref2 b/test-cargo-miri/test.stdout.ref2 index 37efb8c3ee..d426bdf6db 100644 --- a/test-cargo-miri/test.stdout.ref2 +++ b/test-cargo-miri/test.stdout.ref2 @@ -7,5 +7,5 @@ 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 +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 7 filtered out diff --git a/test-cargo-miri/test.stdout.ref3 b/test-cargo-miri/test.stdout.ref3 index c5c39de109..bc4a7c47e9 100644 --- a/test-cargo-miri/test.stdout.ref3 +++ b/test-cargo-miri/test.stdout.ref3 @@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out running 1 test test num_cpus ... ok -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 6 filtered out +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 7 filtered out diff --git a/test-cargo-miri/test.stdout.ref4 b/test-cargo-miri/test.stdout.ref4 index b6403bf6c0..32bbcf9bf2 100644 --- a/test-cargo-miri/test.stdout.ref4 +++ b/test-cargo-miri/test.stdout.ref4 @@ -1,5 +1,6 @@ -running 7 tests +running 8 tests +test cargo_env ... ok test do_panic ... ok test does_not_work_on_miri ... ignored test entropy_rng ... ok @@ -8,5 +9,5 @@ test num_cpus ... ok test simple1 ... ok test simple2 ... ok -test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out +test result: ok. 7 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out diff --git a/test-cargo-miri/tests/test.rs b/test-cargo-miri/tests/test.rs index 68d5426802..915312dfff 100644 --- a/test-cargo-miri/tests/test.rs +++ b/test-cargo-miri/tests/test.rs @@ -42,6 +42,11 @@ fn num_cpus() { assert_eq!(num_cpus::get(), 1); } +#[test] +fn cargo_env() { + assert_eq!(env!("CARGO_PKG_NAME"), "cargo-miri-test"); + env!("CARGO_BIN_EXE_cargo-miri-test"); // Asserts that this exists. +} // FIXME: Remove this `cfg` once we fix https://github.com/rust-lang/miri/issues/1059. // We cfg-gate the `should_panic` attribute and the `panic!` itself, so that the test From 4acab8037e7b505b74f21ae39f05155f8f1b4f6f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 09:12:26 +0200 Subject: [PATCH 14/29] patch away --error-format and --json so that errors are rendered properly --- cargo-miri/bin.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 1edeaade09..0101ac87f0 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -585,16 +585,32 @@ fn phase_cargo_runner(binary: &str, binary_args: env::Args) { } let mut cmd = miri(); - // Forward rustc arguments. We need to patch "--extern" filenames because - // we forced a check-only build without cargo knowing about that: replace `.rlib` suffix by `.rmeta`. + // Forward rustc arguments. + // We need to patch "--extern" filenames because we forced a check-only + // build without cargo knowing about that: replace `.rlib` suffix by + // `.rmeta`. + // We also need to remove `--error-format` as cargo specifies that to be JSON, + // but when we run here, cargo does not interpret the JSON any more. `--json` + // then also nees to be dropped. let mut args = info.args.into_iter(); let extern_flag = "--extern"; + let error_format_flag = "--error-format"; + let json_flag = "--json"; while let Some(arg) = args.next() { if arg == extern_flag { + // `--extern` is always passed as a separate argument by cargo. let next_arg = args.next().expect("`--extern` should be followed by a filename"); let next_arg = next_arg.strip_suffix(".rlib").expect("all extern filenames should end in `.rlib`"); cmd.arg(extern_flag); cmd.arg(format!("{}.rmeta", next_arg)); + } else if arg.starts_with(error_format_flag) { + let suffix = &arg[error_format_flag.len()..]; + assert!(suffix.starts_with('=')); + // Drop this argument. + } else if arg.starts_with(json_flag) { + let suffix = &arg[json_flag.len()..]; + assert!(suffix.starts_with('=')); + // Drop this argument. } else { cmd.arg(arg); } From 495aa932fcdd731cda0c3c8a12b0e8d83939e4e3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 09:18:10 +0200 Subject: [PATCH 15/29] test reading from stdin --- test-cargo-miri/run-test.py | 19 +++++++++---------- test-cargo-miri/src/main.rs | 18 ++++++++++++++++++ test-cargo-miri/stdout.ref | 2 ++ test-cargo-miri/stdout.ref2 | 1 + 4 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 test-cargo-miri/stdout.ref2 diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 6a28f1b403..877a2a5706 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -21,18 +21,19 @@ def cargo_miri(cmd): args += ["--target", os.environ['MIRI_TEST_TARGET']] return args -def test(name, cmd, stdout_ref, stderr_ref, env={}): +def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): print("==> Testing `{}` <==".format(name)) ## Call `cargo miri`, capture all output p_env = os.environ.copy() p_env.update(env) p = subprocess.Popen( cmd, + stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=p_env, ) - (stdout, stderr) = p.communicate() + (stdout, stderr) = p.communicate(input=stdin) stdout = stdout.decode("UTF-8") stderr = stderr.decode("UTF-8") # Show output @@ -51,15 +52,13 @@ def test(name, cmd, stdout_ref, stderr_ref, env={}): def test_cargo_miri_run(): test("cargo miri run", cargo_miri("run"), - "stdout.ref", "stderr.ref" - ) - test("cargo miri run (with target)", - cargo_miri("run") + ["--bin", "cargo-miri-test"], - "stdout.ref", "stderr.ref" + "stdout.ref", "stderr.ref", + stdin=b'12\n21\n', + env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) - test("cargo miri run (with arguments)", - cargo_miri("run") + ["--", "hello world", '"hello world"'], - "stdout.ref", "stderr.ref2" + test("cargo miri run (with arguments and target)", + cargo_miri("run") + ["--bin", "cargo-miri-test", "--", "hello world", '"hello world"'], + "stdout.ref2", "stderr.ref2" ) def test_cargo_miri_test(): diff --git a/test-cargo-miri/src/main.rs b/test-cargo-miri/src/main.rs index d3663ec849..0079328ff6 100644 --- a/test-cargo-miri/src/main.rs +++ b/test-cargo-miri/src/main.rs @@ -1,4 +1,6 @@ use byteorder::{BigEndian, ByteOrder}; +#[cfg(unix)] +use std::io::{self, BufRead}; fn main() { // Exercise external crate, printing to stdout. @@ -11,6 +13,22 @@ fn main() { for arg in std::env::args() { eprintln!("{}", arg); } + + // If there were no arguments, access stdin. + if std::env::args().len() <= 1 { + #[cfg(unix)] + for line in io::stdin().lock().lines() { + let num: i32 = line.unwrap().parse().unwrap(); + println!("{}", 2*num); + } + // On non-Unix, reading from stdin is not support. So we hard-code the right answer. + #[cfg(not(unix))] + { + println!("24"); + println!("42"); + } + } + } #[cfg(test)] diff --git a/test-cargo-miri/stdout.ref b/test-cargo-miri/stdout.ref index 6710f307cb..2eab8df967 100644 --- a/test-cargo-miri/stdout.ref +++ b/test-cargo-miri/stdout.ref @@ -1 +1,3 @@ 0x01020304 +24 +42 diff --git a/test-cargo-miri/stdout.ref2 b/test-cargo-miri/stdout.ref2 new file mode 100644 index 0000000000..6710f307cb --- /dev/null +++ b/test-cargo-miri/stdout.ref2 @@ -0,0 +1 @@ +0x01020304 From 74db22842042f46ee58c63975a6443125825a07c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 9 Sep 2020 11:48:44 +0200 Subject: [PATCH 16/29] update comment Co-authored-by: Oli Scherer --- cargo-miri/bin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 0101ac87f0..d0f098c91d 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -441,7 +441,7 @@ fn phase_cargo_miri(mut args: env::Args) { // Run cargo. if verbose { - cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. + cmd.env("MIRI_VERBOSE", ""); // This makes the other phases verbose. eprintln!("[cargo-miri miri] {:?}", cmd); } exec(cmd) From 907de3ab27914f3b3e844f2f7dcece3263f1c904 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 11 Sep 2020 15:05:05 +0200 Subject: [PATCH 17/29] show proper warning about not running doctests --- cargo-miri/bin.rs | 22 ++++++++++++++++++++-- test-cargo-miri/Cargo.toml | 3 +-- test-cargo-miri/run-test.py | 4 ++-- test-cargo-miri/test.stderr.ref | 1 + test-cargo-miri/test.stderr.ref2 | 0 5 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 test-cargo-miri/test.stderr.ref2 diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index d0f098c91d..f23933a695 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -439,6 +439,9 @@ fn phase_cargo_miri(mut args: env::Args) { let runner_env_name = format!("CARGO_TARGET_{}_RUNNER", target.to_uppercase().replace('-', "_")); cmd.env(runner_env_name, &miri_path); + // Set rustdoc to us as well, so we can make it do nothing (see issue #584). + cmd.env("RUSTDOC", &miri_path); + // Run cargo. if verbose { cmd.env("MIRI_VERBOSE", ""); // This makes the other phases verbose. @@ -568,7 +571,7 @@ fn phase_cargo_rustc(args: env::Args) { } } -fn phase_cargo_runner(binary: &str, binary_args: env::Args) { +fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let file = File::open(&binary) @@ -656,10 +659,25 @@ fn main() { // binary crates for later interpretation. // - When we are executed due to CARGO_TARGET_RUNNER, we start interpretation based on the // flags that were stored earlier. + // On top of that, we are also called as RUSTDOC, but that is just a stub currently. match args.next().as_deref() { Some("miri") => phase_cargo_miri(args), Some("rustc") => phase_cargo_rustc(args), - Some(binary) => phase_cargo_runner(binary, args), + Some(arg) => { + // We have to distinguish the "runner" and "rustfmt" 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`). + 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.") + } else { + show_error(format!("`cargo-miri` called with unexpected first argument `{}`; please only invoke this binary through `cargo miri`", arg)); + } + } _ => show_error(format!("`cargo-miri` called without first argument; please only invoke this binary through `cargo miri`")), } } diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 68970d7d16..6bc11ef0cc 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -12,5 +12,4 @@ rand = { version = "0.7", features = ["small_rng"] } num_cpus = "1.10.1" [lib] -test = false -doctest = false # FIXME: doctests should be skipped automatically until we can run them... +test = false # test that this is respected (will show in the output) diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 877a2a5706..82b3b88a63 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -78,11 +78,11 @@ def test_cargo_miri_test(): ) test("cargo miri test (test target)", cargo_miri("test") + ["--test", "test", "--", "--format=pretty"], - "test.stdout.ref4", "test.stderr.ref" + "test.stdout.ref4", "test.stderr.ref2" ) test("cargo miri test (bin target)", cargo_miri("test") + ["--bin", "cargo-miri-test", "--", "--format=pretty"], - "test.stdout.ref5", "test.stderr.ref" + "test.stdout.ref5", "test.stderr.ref2" ) os.chdir(os.path.dirname(os.path.realpath(__file__))) diff --git a/test-cargo-miri/test.stderr.ref b/test-cargo-miri/test.stderr.ref index e69de29bb2..a310169e30 100644 --- a/test-cargo-miri/test.stderr.ref +++ b/test-cargo-miri/test.stderr.ref @@ -0,0 +1 @@ +Running doctests is not currently supported by Miri. diff --git a/test-cargo-miri/test.stderr.ref2 b/test-cargo-miri/test.stderr.ref2 new file mode 100644 index 0000000000..e69de29bb2 From 8fd16749c71214f18a664edd97913e5fe519d625 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 11 Sep 2020 18:20:41 +0200 Subject: [PATCH 18/29] fix cargo-miri-test for cross-runs --- test-cargo-miri/run-test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 82b3b88a63..c7694f3854 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -62,18 +62,22 @@ 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.ref2" if is_foreign else "test.stderr.ref" + test("cargo miri test", cargo_miri("test"), - "test.stdout.ref", "test.stderr.ref", + "test.stdout.ref",rustdoc_ref, env={'MIRIFLAGS': "-Zmiri-seed=feed"}, ) test("cargo miri test (with filter)", cargo_miri("test") + ["--", "--format=pretty", "le1"], - "test.stdout.ref2", "test.stderr.ref" + "test.stdout.ref2", rustdoc_ref ) test("cargo miri test (without isolation)", cargo_miri("test") + ["--", "--format=pretty", "num_cpus"], - "test.stdout.ref3", "test.stderr.ref", + "test.stdout.ref3", rustdoc_ref, env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) test("cargo miri test (test target)", From 68f1f5b1ee448c43766349fca618aa4fe5f56f21 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 12:41:23 +0200 Subject: [PATCH 19/29] detect when the user passes Miri's flags the old way, and support this for now --- cargo-miri/bin.rs | 61 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index f23933a695..a7f880c9e2 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -12,28 +12,21 @@ use rustc_version::VersionMeta; const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 22); -const CARGO_MIRI_HELP: &str = r#"Interprets bin crates and tests in Miri +const CARGO_MIRI_HELP: &str = r#"Runs binary crates and tests in Miri Usage: - cargo miri [subcommand] [...] [--] [...] [--] [...] + cargo miri [subcommand] [...] [--] [...] Subcommands: - run Run binaries (default) + run Run binaries test Run tests setup Only perform automatic setup, but without asking questions (for getting a proper libstd) -Common options: - -h, --help Print this message - --features Features to compile for the package - -V, --version Print version info and exit - -Other [options] are the same as `cargo check`. Everything after the first "--" is -passed verbatim to Miri, which will pass everything after the second "--" verbatim -to the interpreted program. +The cargo options are exactly the same as for `cargo run` and `cargo test`, respectively. Examples: - cargo miri run -- -Zmiri-disable-stacked-borrows - cargo miri test -- -- test-suite-filter + cargo miri run + cargo miri test -- test-suite-filter "#; #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -390,7 +383,7 @@ fn phase_cargo_miri(mut args: env::Args) { Some("run") => MiriCommand::Run, Some("setup") => MiriCommand::Setup, // Invalid command. - _ => show_error(format!("`cargo miri` must be immediately followed by `test`, `run`, or `setup`.")), + _ => show_error(format!("`cargo miri` supports the following subcommands: `run`, `test`, and `setup`.")), }; let verbose = has_arg_flag("-v"); @@ -421,8 +414,44 @@ fn phase_cargo_miri(mut args: env::Args) { host }; - // Forward all further arguments. - cmd.args(args); + // Forward all further arguments. We do some processing here because we want to + // detect people still using the old way of passing flags to Miri + // (`cargo miri -- -Zmiri-foo`). + while let Some(arg) = args.next() { + cmd.arg(&arg); + if arg == "--" { + // Check if the next argument starts with `-Zmiri`. If yes, we assume + // this is an old-style invocation. + if let Some(next_arg) = args.next() { + if next_arg.starts_with("-Zmiri") { + eprintln!( + "WARNING: it seems like you are setting Miri's flags in `cargo miri` the old way,\n\ + i.e., by passing them after the first `--`. This style is deprecated; please set\n\ + the MIRIFLAGS environment variable instead. `cargo miri run/test` now interprets\n\ + arguments the exact same way as `cargo run/test`." + ); + // Old-style invocation. Turn these into MIRIFLAGS. + let mut miriflags = env::var("MIRIFLAGS").unwrap_or_default(); + miriflags.push(' '); + miriflags.push_str(&next_arg); + while let Some(further_arg) = args.next() { + if further_arg == "--" { + // End of the Miri flags! + break; + } + miriflags.push(' '); + miriflags.push_str(&further_arg); + } + env::set_var("MIRIFLAGS", miriflags); + // Pass the remaining flags to cargo. + cmd.args(args); + break; + } + // Not a Miri argument after all, make sure we pass it to cargo. + cmd.arg(next_arg); + } + } + } // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish From 93f6ca7769e6d7a37d171483f2294c9ec71d6073 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 12:52:14 +0200 Subject: [PATCH 20/29] test 'harness=false' tests --- cargo-miri/bin.rs | 2 +- test-cargo-miri/Cargo.toml | 4 ++++ test-cargo-miri/run-test.py | 22 +++++++++---------- test-cargo-miri/src/main.rs | 1 - test-cargo-miri/{stderr.ref => stderr.ref1} | 0 test-cargo-miri/{stdout.ref => stdout.ref1} | 0 .../{test.stderr.ref => test.stderr.ref1} | 0 .../{test.stdout.ref => test.stdout.ref1} | 1 + test-cargo-miri/test.stdout.ref2 | 1 + test-cargo-miri/test.stdout.ref3 | 1 + test-cargo-miri/tests/no-harness.rs | 3 +++ 11 files changed, 22 insertions(+), 13 deletions(-) rename test-cargo-miri/{stderr.ref => stderr.ref1} (100%) rename test-cargo-miri/{stdout.ref => stdout.ref1} (100%) rename test-cargo-miri/{test.stderr.ref => test.stderr.ref1} (100%) rename test-cargo-miri/{test.stdout.ref => test.stdout.ref1} (92%) create mode 100644 test-cargo-miri/tests/no-harness.rs diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index a7f880c9e2..8f7d444a15 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -499,7 +499,7 @@ fn phase_cargo_rustc(args: env::Args) { /// Cargo does not give us this information directly, so we need to check /// various command-line flags. fn is_runnable_crate() -> bool { - let is_bin = get_arg_flag_value("--crate-type").as_deref() == Some("bin"); + let is_bin = get_arg_flag_value("--crate-type").as_deref().unwrap_or("bin") == "bin"; let is_test = has_arg_flag("--test"); let print = get_arg_flag_value("--print").is_some(); (is_bin || is_test) && !print diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 6bc11ef0cc..f4847270ba 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -13,3 +13,7 @@ num_cpus = "1.10.1" [lib] test = false # test that this is respected (will show in the output) + +[[test]] +name = "no-harness" +harness = false diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index c7694f3854..665e9834e8 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -22,7 +22,7 @@ def cargo_miri(cmd): return args def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): - print("==> Testing `{}` <==".format(name)) + print("==> Testing {} <==".format(name)) ## Call `cargo miri`, capture all output p_env = os.environ.copy() p_env.update(env) @@ -50,13 +50,13 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): fail("stderr does not match reference") def test_cargo_miri_run(): - test("cargo miri run", + test("`cargo miri run`", cargo_miri("run"), - "stdout.ref", "stderr.ref", + "stdout.ref1", "stderr.ref1", stdin=b'12\n21\n', env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) - test("cargo miri run (with arguments and target)", + test("`cargo miri run` (with arguments and target)", cargo_miri("run") + ["--bin", "cargo-miri-test", "--", "hello world", '"hello world"'], "stdout.ref2", "stderr.ref2" ) @@ -64,27 +64,27 @@ 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.ref2" if is_foreign else "test.stderr.ref" + rustdoc_ref = "test.stderr.ref2" if is_foreign else "test.stderr.ref1" - test("cargo miri test", + test("`cargo miri test`", cargo_miri("test"), - "test.stdout.ref",rustdoc_ref, + "test.stdout.ref1",rustdoc_ref, env={'MIRIFLAGS': "-Zmiri-seed=feed"}, ) - test("cargo miri test (with filter)", + test("`cargo miri test` (with filter)", cargo_miri("test") + ["--", "--format=pretty", "le1"], "test.stdout.ref2", rustdoc_ref ) - test("cargo miri test (without isolation)", + test("`cargo miri test` (without isolation)", cargo_miri("test") + ["--", "--format=pretty", "num_cpus"], "test.stdout.ref3", rustdoc_ref, env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) - test("cargo miri test (test target)", + test("`cargo miri test` (test target)", cargo_miri("test") + ["--test", "test", "--", "--format=pretty"], "test.stdout.ref4", "test.stderr.ref2" ) - test("cargo miri test (bin target)", + test("`cargo miri test` (bin target)", cargo_miri("test") + ["--bin", "cargo-miri-test", "--", "--format=pretty"], "test.stdout.ref5", "test.stderr.ref2" ) diff --git a/test-cargo-miri/src/main.rs b/test-cargo-miri/src/main.rs index 0079328ff6..17808f5706 100644 --- a/test-cargo-miri/src/main.rs +++ b/test-cargo-miri/src/main.rs @@ -28,7 +28,6 @@ fn main() { println!("42"); } } - } #[cfg(test)] diff --git a/test-cargo-miri/stderr.ref b/test-cargo-miri/stderr.ref1 similarity index 100% rename from test-cargo-miri/stderr.ref rename to test-cargo-miri/stderr.ref1 diff --git a/test-cargo-miri/stdout.ref b/test-cargo-miri/stdout.ref1 similarity index 100% rename from test-cargo-miri/stdout.ref rename to test-cargo-miri/stdout.ref1 diff --git a/test-cargo-miri/test.stderr.ref b/test-cargo-miri/test.stderr.ref1 similarity index 100% rename from test-cargo-miri/test.stderr.ref rename to test-cargo-miri/test.stderr.ref1 diff --git a/test-cargo-miri/test.stdout.ref b/test-cargo-miri/test.stdout.ref1 similarity index 92% rename from test-cargo-miri/test.stdout.ref rename to test-cargo-miri/test.stdout.ref1 index 1eb18fe887..76144513c5 100644 --- a/test-cargo-miri/test.stdout.ref +++ b/test-cargo-miri/test.stdout.ref1 @@ -3,6 +3,7 @@ running 1 test . test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out +no-harness test running 8 tests ..i..... diff --git a/test-cargo-miri/test.stdout.ref2 b/test-cargo-miri/test.stdout.ref2 index d426bdf6db..1264c6da7f 100644 --- a/test-cargo-miri/test.stdout.ref2 +++ b/test-cargo-miri/test.stdout.ref2 @@ -3,6 +3,7 @@ running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out +no-harness test running 1 test test simple1 ... ok diff --git a/test-cargo-miri/test.stdout.ref3 b/test-cargo-miri/test.stdout.ref3 index bc4a7c47e9..a5edee2c5f 100644 --- a/test-cargo-miri/test.stdout.ref3 +++ b/test-cargo-miri/test.stdout.ref3 @@ -3,6 +3,7 @@ running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out +no-harness test running 1 test test num_cpus ... ok diff --git a/test-cargo-miri/tests/no-harness.rs b/test-cargo-miri/tests/no-harness.rs new file mode 100644 index 0000000000..8d1c5c3462 --- /dev/null +++ b/test-cargo-miri/tests/no-harness.rs @@ -0,0 +1,3 @@ +fn main() { + println!("no-harness test"); +} From 4b2410eb81d5796710d2002f292445a7461433e2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 13:10:23 +0200 Subject: [PATCH 21/29] test propagating env vars from build.rs to binary --- cargo-miri/bin.rs | 4 +++- test-cargo-miri/build.rs | 2 ++ test-cargo-miri/run-test.py | 7 +++++-- test-cargo-miri/src/main.rs | 3 +++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 8f7d444a15..a9b0c04a95 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -609,7 +609,9 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { let info: CrateRunInfo = serde_json::from_reader(file) .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); - // Set missing env vars. + // Set missing env vars. Looks like `build.rs` vars are still set at run-time, but + // `CARGO_BIN_EXE_*` are not. This means we can give the run-time environment precedence, + // to rather do too little than too much. for (name, val) in info.env { if env::var_os(&name).is_none() { env::set_var(name, val); diff --git a/test-cargo-miri/build.rs b/test-cargo-miri/build.rs index b1f5fc1726..9851ccf39f 100644 --- a/test-cargo-miri/build.rs +++ b/test-cargo-miri/build.rs @@ -12,4 +12,6 @@ fn not_in_miri() -> i32 { fn main() { not_in_miri(); println!("cargo:rerun-if-changed=build.rs"); + println!("cargo:rerun-if-env-changed=MIRITESTVAR"); + println!("cargo:rustc-env=MIRITESTVAR=testval"); } diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 665e9834e8..2250835c99 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -50,11 +50,14 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): fail("stderr does not match reference") def test_cargo_miri_run(): - test("`cargo miri run`", + test("`cargo miri run` (without isolation)", cargo_miri("run"), "stdout.ref1", "stderr.ref1", stdin=b'12\n21\n', - env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, + env={ + 'MIRIFLAGS': "-Zmiri-disable-isolation", + 'MIRITESTVAR': "wrongval", # make sure the build.rs value takes precedence + }, ) test("`cargo miri run` (with arguments and target)", cargo_miri("run") + ["--bin", "cargo-miri-test", "--", "hello world", '"hello world"'], diff --git a/test-cargo-miri/src/main.rs b/test-cargo-miri/src/main.rs index 17808f5706..b36b0c725f 100644 --- a/test-cargo-miri/src/main.rs +++ b/test-cargo-miri/src/main.rs @@ -3,6 +3,9 @@ use byteorder::{BigEndian, ByteOrder}; use std::io::{self, BufRead}; fn main() { + // Check env var set by `build.rs`. + assert_eq!(env!("MIRITESTVAR"), "testval"); + // Exercise external crate, printing to stdout. let buf = &[1,2,3,4]; let n = ::read_u32(buf); From f6ae8868c9c10b4dd8d8382c3c548119245594d1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 13:33:30 +0200 Subject: [PATCH 22/29] test 'cargo miri run' CWD, also for subcrate in a workspace --- test-cargo-miri/Cargo.lock | 72 +++++++++++++++-------------- test-cargo-miri/Cargo.toml | 5 +- test-cargo-miri/run-test.py | 5 ++ test-cargo-miri/src/main.rs | 8 +++- test-cargo-miri/stderr.ref3 | 0 test-cargo-miri/stdout.ref3 | 1 + test-cargo-miri/subcrate/Cargo.toml | 9 ++++ test-cargo-miri/subcrate/main.rs | 11 +++++ 8 files changed, 74 insertions(+), 37 deletions(-) create mode 100644 test-cargo-miri/stderr.ref3 create mode 100644 test-cargo-miri/stdout.ref3 create mode 100644 test-cargo-miri/subcrate/Cargo.toml create mode 100644 test-cargo-miri/subcrate/main.rs diff --git a/test-cargo-miri/Cargo.lock b/test-cargo-miri/Cargo.lock index ecf71a5f43..6bc70135a8 100644 --- a/test-cargo-miri/Cargo.lock +++ b/test-cargo-miri/Cargo.lock @@ -4,120 +4,122 @@ name = "byteorder" version = "1.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08c48aae112d48ed9f069b33538ea9e3e90aa263cfa3d1c24309612b1f7472de" [[package]] name = "cargo-miri-test" version = "0.1.0" dependencies = [ - "byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", - "num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)", - "rand 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)", + "byteorder", + "num_cpus", + "rand", ] [[package]] name = "cfg-if" version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" [[package]] name = "getrandom" version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7abc8dd8451921606d809ba32e95b6111925cd2906060d2dcc29c070220503eb" dependencies = [ - "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "wasi 0.9.0+wasi-snapshot-preview1 (registry+https://github.com/rust-lang/crates.io-index)", + "cfg-if", + "libc", + "wasi", ] [[package]] name = "hermit-abi" version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "725cf19794cf90aa94e65050cb4191ff5d8fa87a498383774c47b332e3af952e" dependencies = [ - "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", + "libc", ] [[package]] name = "libc" version = "0.2.68" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dea0c0405123bba743ee3f91f49b1c7cfb684eef0da0a50110f758ccf24cdff0" [[package]] name = "num_cpus" version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46203554f085ff89c235cd12f7075f3233af9b11ed7c9e16dfe2560d03313ce6" dependencies = [ - "hermit-abi 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", + "hermit-abi", + "libc", ] [[package]] name = "ppv-lite86" version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74490b50b9fbe561ac330df47c08f3f33073d2d00c150f719147d7c54522fa1b" [[package]] name = "rand" version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03" dependencies = [ - "getrandom 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_chacha 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_hc 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_pcg 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", + "getrandom", + "libc", + "rand_chacha", + "rand_core", + "rand_hc", + "rand_pcg", ] [[package]] name = "rand_chacha" version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4c8ed856279c9737206bf725bf36935d8666ead7aa69b52be55af369d193402" dependencies = [ - "ppv-lite86 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "ppv-lite86", + "rand_core", ] [[package]] name = "rand_core" version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "90bde5296fc891b0cef12a6d03ddccc162ce7b2aff54160af9338f8d40df6d19" dependencies = [ - "getrandom 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)", + "getrandom", ] [[package]] name = "rand_hc" version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3129af7b92a17112d59ad498c6f81eaf463253766b90396d39ea7a39d6613c" dependencies = [ - "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core", ] [[package]] name = "rand_pcg" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16abd0c1b639e9eb4d7c50c0b8100b0d0f849be2349829c740fe8e6eb4816429" dependencies = [ - "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core", ] +[[package]] +name = "subcrate" +version = "0.1.0" + [[package]] name = "wasi" version = "0.9.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" - -[metadata] -"checksum byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "08c48aae112d48ed9f069b33538ea9e3e90aa263cfa3d1c24309612b1f7472de" -"checksum cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" -"checksum getrandom 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)" = "7abc8dd8451921606d809ba32e95b6111925cd2906060d2dcc29c070220503eb" -"checksum hermit-abi 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)" = "725cf19794cf90aa94e65050cb4191ff5d8fa87a498383774c47b332e3af952e" -"checksum libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)" = "dea0c0405123bba743ee3f91f49b1c7cfb684eef0da0a50110f758ccf24cdff0" -"checksum num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)" = "46203554f085ff89c235cd12f7075f3233af9b11ed7c9e16dfe2560d03313ce6" -"checksum ppv-lite86 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "74490b50b9fbe561ac330df47c08f3f33073d2d00c150f719147d7c54522fa1b" -"checksum rand 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03" -"checksum rand_chacha 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f4c8ed856279c9737206bf725bf36935d8666ead7aa69b52be55af369d193402" -"checksum rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "90bde5296fc891b0cef12a6d03ddccc162ce7b2aff54160af9338f8d40df6d19" -"checksum rand_hc 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ca3129af7b92a17112d59ad498c6f81eaf463253766b90396d39ea7a39d6613c" -"checksum rand_pcg 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "16abd0c1b639e9eb4d7c50c0b8100b0d0f849be2349829c740fe8e6eb4816429" -"checksum wasi 0.9.0+wasi-snapshot-preview1 (registry+https://github.com/rust-lang/crates.io-index)" = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" +checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index f4847270ba..131e184985 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -1,7 +1,10 @@ +[workspace] +members = ["subcrate"] + [package] name = "cargo-miri-test" version = "0.1.0" -authors = ["Oliver Schneider "] +authors = ["Miri Team"] edition = "2018" [dependencies] diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 2250835c99..430c8a7583 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -63,6 +63,11 @@ def test_cargo_miri_run(): cargo_miri("run") + ["--bin", "cargo-miri-test", "--", "hello world", '"hello world"'], "stdout.ref2", "stderr.ref2" ) + test("`cargo miri run` (subcrate)", + cargo_miri("run") + ["-p", "subcrate"], + "stdout.ref3", "stderr.ref3", + env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, + ) def test_cargo_miri_test(): # rustdoc is not run on foreign targets diff --git a/test-cargo-miri/src/main.rs b/test-cargo-miri/src/main.rs index b36b0c725f..5f311441bf 100644 --- a/test-cargo-miri/src/main.rs +++ b/test-cargo-miri/src/main.rs @@ -1,4 +1,6 @@ use byteorder::{BigEndian, ByteOrder}; +use std::env; +use std::path::PathBuf; #[cfg(unix)] use std::io::{self, BufRead}; @@ -17,8 +19,12 @@ fn main() { eprintln!("{}", arg); } - // If there were no arguments, access stdin. + // If there were no arguments, access stdin and test working dir. if std::env::args().len() <= 1 { + let env_dir = env::current_dir().unwrap(); + let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + assert_eq!(env_dir, crate_dir); + #[cfg(unix)] for line in io::stdin().lock().lines() { let num: i32 = line.unwrap().parse().unwrap(); diff --git a/test-cargo-miri/stderr.ref3 b/test-cargo-miri/stderr.ref3 new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test-cargo-miri/stdout.ref3 b/test-cargo-miri/stdout.ref3 new file mode 100644 index 0000000000..53340a5023 --- /dev/null +++ b/test-cargo-miri/stdout.ref3 @@ -0,0 +1 @@ +subcrate running diff --git a/test-cargo-miri/subcrate/Cargo.toml b/test-cargo-miri/subcrate/Cargo.toml new file mode 100644 index 0000000000..13e9aa4c1a --- /dev/null +++ b/test-cargo-miri/subcrate/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "subcrate" +version = "0.1.0" +authors = ["Miri Team"] +edition = "2018" + +[[bin]] +name = "subcrate" +path = "main.rs" diff --git a/test-cargo-miri/subcrate/main.rs b/test-cargo-miri/subcrate/main.rs new file mode 100644 index 0000000000..db8ea7eb18 --- /dev/null +++ b/test-cargo-miri/subcrate/main.rs @@ -0,0 +1,11 @@ +use std::env; +use std::path::PathBuf; + +fn main() { + println!("subcrate running"); + + let env_dir = env::current_dir().unwrap(); + let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + // CWD should be workspace root, i.e., one level up from crate root. + assert_eq!(env_dir, crate_dir.parent().unwrap()); +} From 1a78ab804a5daa385a4e3ea84fe5de2b1bca816a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 13:52:05 +0200 Subject: [PATCH 23/29] make sure subcrate tests have the right cwd --- README.md | 4 ++++ cargo-miri/bin.rs | 10 +++++++++- src/bin/miri.rs | 5 +++++ test-cargo-miri/Cargo.toml | 4 ---- test-cargo-miri/run-test.py | 29 +++++++++++++++++------------ test-cargo-miri/subcrate/Cargo.toml | 5 +++++ test-cargo-miri/subcrate/test.rs | 11 +++++++++++ test-cargo-miri/test.stdout.ref1 | 1 - test-cargo-miri/test.stdout.ref2 | 1 - test-cargo-miri/test.stdout.ref3 | 17 +++++++++-------- test-cargo-miri/test.stdout.ref4 | 13 +++---------- test-cargo-miri/test.stdout.ref5 | 6 +++--- test-cargo-miri/tests/no-harness.rs | 3 --- 13 files changed, 66 insertions(+), 43 deletions(-) create mode 100644 test-cargo-miri/subcrate/test.rs delete mode 100644 test-cargo-miri/tests/no-harness.rs diff --git a/README.md b/README.md index 67161974b1..6654de10ab 100644 --- a/README.md +++ b/README.md @@ -244,6 +244,10 @@ different Miri binaries, and as such worth documenting: * `MIRI_BE_RUSTC` when set to any value tells the Miri driver to actually not interpret the code but compile it like rustc would. This is useful to be sure that the compiled `rlib`s are compatible with Miri. +* `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 + working directory at run-time than at build-time. ## Miri `extern` functions diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index a9b0c04a95..f39949f2ea 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -43,6 +43,8 @@ struct CrateRunInfo { args: Vec, /// The environment. env: Vec<(OsString, OsString)>, + /// The current working directory. + current_dir: OsString, } impl CrateRunInfo { @@ -50,7 +52,8 @@ impl CrateRunInfo { fn collect(args: env::Args) -> Self { let args = args.collect(); let env = env::vars_os().collect(); - CrateRunInfo { args, env } + let current_dir = env::current_dir().unwrap().into_os_string(); + CrateRunInfo { args, env, current_dir } } fn store(&self, filename: &Path) { @@ -669,6 +672,11 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { cmd.arg("--"); cmd.args(binary_args); + // Make sure we use the build-time working directory for interpreting Miri/rustc arguments. + // But then we need to switch to the run-time one, which we instruct Miri do do by setting `MIRI_CWD`. + cmd.current_dir(info.current_dir); + cmd.env("MIRI_CWD", env::current_dir().unwrap()); + // Run it. if verbose { eprintln!("[cargo-miri runner] {:?}", cmd); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 1347020475..4363f9a150 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -45,6 +45,11 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { // Add filename to `miri` arguments. config.args.insert(0, compiler.input().filestem().to_string()); + // Adjust working directory for interpretation. + if let Some(cwd) = env::var_os("MIRI_CWD") { + env::set_current_dir(cwd).unwrap(); + } + if let Some(return_code) = miri::eval_main(tcx, entry_def_id.to_def_id(), config) { std::process::exit( i32::try_from(return_code).expect("Return value was too large!"), diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 131e184985..4900ce9675 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -16,7 +16,3 @@ num_cpus = "1.10.1" [lib] test = false # test that this is respected (will show in the output) - -[[test]] -name = "no-harness" -harness = false diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 430c8a7583..78b10d1f2b 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -50,7 +50,7 @@ def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): fail("stderr does not match reference") def test_cargo_miri_run(): - test("`cargo miri run` (without isolation)", + test("`cargo miri run` (no isolation)", cargo_miri("run"), "stdout.ref1", "stderr.ref1", stdin=b'12\n21\n', @@ -61,9 +61,9 @@ def test_cargo_miri_run(): ) test("`cargo miri run` (with arguments and target)", cargo_miri("run") + ["--bin", "cargo-miri-test", "--", "hello world", '"hello world"'], - "stdout.ref2", "stderr.ref2" + "stdout.ref2", "stderr.ref2", ) - test("`cargo miri run` (subcrate)", + test("`cargo miri run` (subcrate, no ioslation)", cargo_miri("run") + ["-p", "subcrate"], "stdout.ref3", "stderr.ref3", env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, @@ -76,25 +76,30 @@ def test_cargo_miri_test(): test("`cargo miri test`", cargo_miri("test"), - "test.stdout.ref1",rustdoc_ref, + "test.stdout.ref1", rustdoc_ref, env={'MIRIFLAGS': "-Zmiri-seed=feed"}, ) + test("`cargo miri test` (no isolation)", + cargo_miri("test"), + "test.stdout.ref1", rustdoc_ref, + env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, + ) test("`cargo miri test` (with filter)", cargo_miri("test") + ["--", "--format=pretty", "le1"], - "test.stdout.ref2", rustdoc_ref - ) - test("`cargo miri test` (without isolation)", - cargo_miri("test") + ["--", "--format=pretty", "num_cpus"], - "test.stdout.ref3", rustdoc_ref, - env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, + "test.stdout.ref2", rustdoc_ref, ) test("`cargo miri test` (test target)", cargo_miri("test") + ["--test", "test", "--", "--format=pretty"], - "test.stdout.ref4", "test.stderr.ref2" + "test.stdout.ref3", "test.stderr.ref2", ) test("`cargo miri test` (bin target)", cargo_miri("test") + ["--bin", "cargo-miri-test", "--", "--format=pretty"], - "test.stdout.ref5", "test.stderr.ref2" + "test.stdout.ref4", "test.stderr.ref2", + ) + test("`cargo miri test` (subcrate)", + cargo_miri("test") + ["-p", "subcrate"], + "test.stdout.ref5", "test.stderr.ref2", + env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, ) os.chdir(os.path.dirname(os.path.realpath(__file__))) diff --git a/test-cargo-miri/subcrate/Cargo.toml b/test-cargo-miri/subcrate/Cargo.toml index 13e9aa4c1a..78552e6aed 100644 --- a/test-cargo-miri/subcrate/Cargo.toml +++ b/test-cargo-miri/subcrate/Cargo.toml @@ -7,3 +7,8 @@ edition = "2018" [[bin]] name = "subcrate" path = "main.rs" + +[[test]] +name = "subtest" +path = "test.rs" +harness = false diff --git a/test-cargo-miri/subcrate/test.rs b/test-cargo-miri/subcrate/test.rs new file mode 100644 index 0000000000..16c63411ce --- /dev/null +++ b/test-cargo-miri/subcrate/test.rs @@ -0,0 +1,11 @@ +use std::env; +use std::path::PathBuf; + +fn main() { + println!("subcrate testing"); + + let env_dir = env::current_dir().unwrap(); + let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + // CWD should be crate root. + assert_eq!(env_dir, crate_dir); +} diff --git a/test-cargo-miri/test.stdout.ref1 b/test-cargo-miri/test.stdout.ref1 index 76144513c5..1eb18fe887 100644 --- a/test-cargo-miri/test.stdout.ref1 +++ b/test-cargo-miri/test.stdout.ref1 @@ -3,7 +3,6 @@ running 1 test . test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out -no-harness test running 8 tests ..i..... diff --git a/test-cargo-miri/test.stdout.ref2 b/test-cargo-miri/test.stdout.ref2 index 1264c6da7f..d426bdf6db 100644 --- a/test-cargo-miri/test.stdout.ref2 +++ b/test-cargo-miri/test.stdout.ref2 @@ -3,7 +3,6 @@ running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out -no-harness test running 1 test test simple1 ... ok diff --git a/test-cargo-miri/test.stdout.ref3 b/test-cargo-miri/test.stdout.ref3 index a5edee2c5f..32bbcf9bf2 100644 --- a/test-cargo-miri/test.stdout.ref3 +++ b/test-cargo-miri/test.stdout.ref3 @@ -1,12 +1,13 @@ -running 0 tests - -test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out - -no-harness test - -running 1 test +running 8 tests +test cargo_env ... ok +test do_panic ... ok +test does_not_work_on_miri ... ignored +test entropy_rng ... ok +test fail_index_check ... ok test num_cpus ... ok +test simple1 ... ok +test simple2 ... ok -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 7 filtered out +test result: ok. 7 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out diff --git a/test-cargo-miri/test.stdout.ref4 b/test-cargo-miri/test.stdout.ref4 index 32bbcf9bf2..4caa30a7f0 100644 --- a/test-cargo-miri/test.stdout.ref4 +++ b/test-cargo-miri/test.stdout.ref4 @@ -1,13 +1,6 @@ -running 8 tests -test cargo_env ... ok -test do_panic ... ok -test does_not_work_on_miri ... ignored -test entropy_rng ... ok -test fail_index_check ... ok -test num_cpus ... ok -test simple1 ... ok -test simple2 ... ok +running 1 test +test test::rng ... ok -test result: ok. 7 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out diff --git a/test-cargo-miri/test.stdout.ref5 b/test-cargo-miri/test.stdout.ref5 index 4caa30a7f0..67e5c7f8e9 100644 --- a/test-cargo-miri/test.stdout.ref5 +++ b/test-cargo-miri/test.stdout.ref5 @@ -1,6 +1,6 @@ -running 1 test -test test::rng ... ok +running 0 tests -test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out +subcrate testing diff --git a/test-cargo-miri/tests/no-harness.rs b/test-cargo-miri/tests/no-harness.rs deleted file mode 100644 index 8d1c5c3462..0000000000 --- a/test-cargo-miri/tests/no-harness.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - println!("no-harness test"); -} From f330035b926aaf2af0a0d11919cda0216aede8ae Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 13:57:49 +0200 Subject: [PATCH 24/29] cleaner output for cargo-miri-test harness --- test-cargo-miri/run-test.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 78b10d1f2b..bbb8df9059 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -22,7 +22,7 @@ def cargo_miri(cmd): return args def test(name, cmd, stdout_ref, stderr_ref, stdin=b'', env={}): - print("==> Testing {} <==".format(name)) + print("Testing {}...".format(name)) ## Call `cargo miri`, capture all output p_env = os.environ.copy() p_env.update(env) @@ -36,18 +36,17 @@ 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(): + # All good! + return # Show output - print("=> captured stdout <=") + print("--- BEGIN stdout ---") print(stdout, end="") - print("=> captured stderr <=") + print("--- END stdout ---") + print("--- BEGIN stderr ---") print(stderr, end="") - # Test for failures - if p.returncode != 0: - fail("Non-zero exit status") - if stdout != open(stdout_ref).read(): - fail("stdout does not match reference") - if stderr != open(stderr_ref).read(): - fail("stderr does not match reference") + print("--- END stderr ---") + fail("exit code was {}".format(p.returncode)) def test_cargo_miri_run(): test("`cargo miri run` (no isolation)", @@ -96,7 +95,7 @@ def test_cargo_miri_test(): cargo_miri("test") + ["--bin", "cargo-miri-test", "--", "--format=pretty"], "test.stdout.ref4", "test.stderr.ref2", ) - test("`cargo miri test` (subcrate)", + test("`cargo miri test` (subcrate, no isolation)", cargo_miri("test") + ["-p", "subcrate"], "test.stdout.ref5", "test.stderr.ref2", env={'MIRIFLAGS': "-Zmiri-disable-isolation"}, From 3962b563a63bd17d3ff2c3205a83440811918067 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 14:02:08 +0200 Subject: [PATCH 25/29] more consistent error capitalization --- cargo-miri/bin.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index f39949f2ea..2f06f91c6b 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -58,10 +58,10 @@ impl CrateRunInfo { fn store(&self, filename: &Path) { let file = File::create(filename) - .unwrap_or_else(|_| show_error(format!("Cannot create `{}`", filename.display()))); + .unwrap_or_else(|_| show_error(format!("cannot create `{}`", filename.display()))); let file = BufWriter::new(file); serde_json::ser::to_writer(file, self) - .unwrap_or_else(|_| show_error(format!("Cannot write to `{}`", filename.display()))); + .unwrap_or_else(|_| show_error(format!("cannot write to `{}`", filename.display()))); } } @@ -204,15 +204,15 @@ fn ask_to_run(mut cmd: Command, ask: bool, text: &str) { match buf.trim().to_lowercase().as_ref() { // Proceed. "" | "y" | "yes" => {} - "n" | "no" => show_error(format!("Aborting as per your request")), - a => show_error(format!("I do not understand `{}`", a)), + "n" | "no" => show_error(format!("aborting as per your request")), + a => show_error(format!("invalid answer `{}`", a)), }; } else { println!("Running `{:?}` to {}.", cmd, text); } if cmd.status().expect(&format!("failed to execute {:?}", cmd)).success().not() { - show_error(format!("Failed to {}", text)); + show_error(format!("failed to {}", text)); } } @@ -235,7 +235,7 @@ fn setup(subcommand: MiriCommand) { if xargo_version().map_or(true, |v| v < XARGO_MIN_VERSION) { if std::env::var_os("XARGO_CHECK").is_some() { // The user manually gave us a xargo binary; don't do anything automatically. - show_error(format!("Your xargo is too old; please upgrade to the latest version")) + show_error(format!("xargo is too old; please upgrade to the latest version")) } let mut cmd = cargo(); cmd.args(&["install", "xargo", "-f"]); @@ -275,7 +275,7 @@ fn setup(subcommand: MiriCommand) { } }; if !rust_src.exists() { - show_error(format!("Given Rust source directory `{}` does not exist.", rust_src.display())); + show_error(format!("given Rust source directory `{}` does not exist.", rust_src.display())); } // Next, we need our own libstd. Prepare a xargo project for that purpose. @@ -349,7 +349,7 @@ path = "lib.rs" command.env_remove("RUSTFLAGS"); // Finally run it! if command.status().expect("failed to run xargo").success().not() { - show_error(format!("Failed to run xargo")); + show_error(format!("failed to run xargo")); } // That should be it! But we need to figure out where xargo built stuff. @@ -575,7 +575,7 @@ fn phase_cargo_rustc(args: env::Args) { // Use our custom sysroot. let sysroot = - env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); + env::var_os("MIRI_SYSROOT").expect("the wrapper should have set MIRI_SYSROOT"); cmd.arg("--sysroot"); cmd.arg(sysroot); } else { @@ -597,9 +597,9 @@ fn phase_cargo_rustc(args: env::Args) { if emit_link_hack { // Some platforms prepend "lib", some do not... let's just create both files. let filename = out_filename("lib", ".rlib"); - File::create(filename).expect("Failed to create rlib file"); + File::create(filename).expect("failed to create rlib file"); let filename = out_filename("", ".rlib"); - File::create(filename).expect("Failed to create rlib file"); + File::create(filename).expect("failed to create rlib file"); } } @@ -607,10 +607,10 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let file = File::open(&binary) - .unwrap_or_else(|_| show_error(format!("File {:?} not found or `cargo-miri` invoked incorrectly; please only invoke this binary through `cargo miri`", binary))); + .unwrap_or_else(|_| show_error(format!("file {:?} not found or `cargo-miri` invoked incorrectly; please only invoke this binary through `cargo miri`", binary))); let file = BufReader::new(file); let info: CrateRunInfo = serde_json::from_reader(file) - .unwrap_or_else(|_| show_error(format!("File {:?} does not contain valid JSON", binary))); + .unwrap_or_else(|_| show_error(format!("file {:?} contains outdated or invalid JSON; try `cargo clean`", binary))); // Set missing env vars. Looks like `build.rs` vars are still set at run-time, but // `CARGO_BIN_EXE_*` are not. This means we can give the run-time environment precedence, @@ -654,7 +654,7 @@ fn phase_cargo_runner(binary: &Path, binary_args: env::Args) { } // Set sysroot. let sysroot = - env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); + env::var_os("MIRI_SYSROOT").expect("the wrapper should have set MIRI_SYSROOT"); cmd.arg("--sysroot"); cmd.arg(sysroot); // Respect `MIRIFLAGS`. From 8ca22e2dda9ed2a560fca2ebfc98b7f1ddf845d0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 14:09:43 +0200 Subject: [PATCH 26/29] make (not yet actually used) doctest actually use the crate, and fix a comment --- cargo-miri/bin.rs | 2 +- test-cargo-miri/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 2f06f91c6b..f50bb44755 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -140,7 +140,7 @@ fn xargo_check() -> Command { Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check"))) } -/// Execute the command if it fails, fail this process with the same exit code. +/// Execute the command. If it fails, fail this process with the same exit code. /// Otherwise, continue. fn exec(mut cmd: Command) { let exit_status = cmd.status().expect("failed to run command"); diff --git a/test-cargo-miri/src/lib.rs b/test-cargo-miri/src/lib.rs index 064954ba98..4e2c8b572c 100644 --- a/test-cargo-miri/src/lib.rs +++ b/test-cargo-miri/src/lib.rs @@ -1,6 +1,6 @@ /// Doc-test test /// ```rust -/// assert!(true); +/// assert!(cargo_miri_test::make_true()); /// ``` pub fn make_true() -> bool { true From 3025e2d4c4d1d66f423a9b38a2dcd66c05ea7a28 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 14:18:19 +0200 Subject: [PATCH 27/29] enable some forgotten panic tests on Windows --- test-cargo-miri/tests/test.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test-cargo-miri/tests/test.rs b/test-cargo-miri/tests/test.rs index 915312dfff..eb97713b4b 100644 --- a/test-cargo-miri/tests/test.rs +++ b/test-cargo-miri/tests/test.rs @@ -48,21 +48,15 @@ fn cargo_env() { env!("CARGO_BIN_EXE_cargo-miri-test"); // Asserts that this exists. } -// FIXME: Remove this `cfg` once we fix https://github.com/rust-lang/miri/issues/1059. -// We cfg-gate the `should_panic` attribute and the `panic!` itself, so that the test -// stdout does not depend on the target. #[test] -#[cfg_attr(not(windows), should_panic(expected="Explicit panic"))] +#[should_panic(expected="Explicit panic")] fn do_panic() { // In large, friendly letters :) - #[cfg(not(windows))] panic!("Explicit panic from test!"); } -// FIXME: see above #[test] +#[should_panic(expected="the len is 0 but the index is 42")] #[allow(unconditional_panic)] -#[cfg_attr(not(windows), should_panic(expected="the len is 0 but the index is 42"))] fn fail_index_check() { - #[cfg(not(windows))] [][42] } From decb78405504fd53081b0553599dafe96a424226 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 14:24:56 +0200 Subject: [PATCH 28/29] make sure tests pass even with RUST_TEST_NOCAPTURE set --- test-cargo-miri/run-test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index bbb8df9059..e7c341a1f0 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -102,6 +102,7 @@ def test_cargo_miri_test(): ) os.chdir(os.path.dirname(os.path.realpath(__file__))) +os.environ["RUST_TEST_NOCAPTURE"] = "0" # this affects test output, so make sure it is not set target_str = " for target {}".format(os.environ['MIRI_TEST_TARGET']) if 'MIRI_TEST_TARGET' in os.environ else "" print(CGREEN + CBOLD + "## Running `cargo miri` tests{}".format(target_str) + CEND) From 5a946de5ae06fb84b8c1a80ffb3dae74d7c0f3cd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Sep 2020 16:01:33 +0200 Subject: [PATCH 29/29] test-cargo-miri: normalize slashes before comparing paths --- test-cargo-miri/src/main.rs | 4 ++++ test-cargo-miri/subcrate/main.rs | 8 ++++++-- test-cargo-miri/subcrate/test.rs | 5 ++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test-cargo-miri/src/main.rs b/test-cargo-miri/src/main.rs index 5f311441bf..3a2b910fb7 100644 --- a/test-cargo-miri/src/main.rs +++ b/test-cargo-miri/src/main.rs @@ -22,7 +22,11 @@ fn main() { // If there were no arguments, access stdin and test working dir. if std::env::args().len() <= 1 { let env_dir = env::current_dir().unwrap(); + // CWD should be crate root. let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + // We have to normalize slashes, as the env var might be set for a different target's conventions. + let env_dir = env_dir.to_string_lossy().replace("\\", "/"); + let crate_dir = crate_dir.to_string_lossy().replace("\\", "/"); assert_eq!(env_dir, crate_dir); #[cfg(unix)] diff --git a/test-cargo-miri/subcrate/main.rs b/test-cargo-miri/subcrate/main.rs index db8ea7eb18..5ccdc5a1c3 100644 --- a/test-cargo-miri/subcrate/main.rs +++ b/test-cargo-miri/subcrate/main.rs @@ -5,7 +5,11 @@ fn main() { println!("subcrate running"); let env_dir = env::current_dir().unwrap(); - let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); // CWD should be workspace root, i.e., one level up from crate root. - assert_eq!(env_dir, crate_dir.parent().unwrap()); + let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + let crate_dir = crate_dir.parent().unwrap(); + // We have to normalize slashes, as the env var might be set for a different target's conventions. + let env_dir = env_dir.to_string_lossy().replace("\\", "/"); + let crate_dir = crate_dir.to_string_lossy().replace("\\", "/"); + assert_eq!(env_dir, crate_dir); } diff --git a/test-cargo-miri/subcrate/test.rs b/test-cargo-miri/subcrate/test.rs index 16c63411ce..f21d6e13ff 100644 --- a/test-cargo-miri/subcrate/test.rs +++ b/test-cargo-miri/subcrate/test.rs @@ -5,7 +5,10 @@ fn main() { println!("subcrate testing"); let env_dir = env::current_dir().unwrap(); - let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); // CWD should be crate root. + let crate_dir = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + // We have to normalize slashes, as the env var might be set for a different target's conventions. + let env_dir = env_dir.to_string_lossy().replace("\\", "/"); + let crate_dir = crate_dir.to_string_lossy().replace("\\", "/"); assert_eq!(env_dir, crate_dir); }