Skip to content

cargo-miri, miri-script, tests/ui: misc simplifications and comments #3418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 26, 2024
Merged

cargo-miri, miri-script, tests/ui: misc simplifications and comments #3418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6737f1
run_dep_mode: treat program.args and program.env consistently
RalfJung Mar 25, 2024
2e3eb05
run command: simplify flag computation
RalfJung Mar 26, 2024
4890fdb
cargo-miri: clean up info_query treatment a bit, and update comment a…
RalfJung Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 31 additions & 30 deletions cargo-miri/src/phases.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,14 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
"WARNING: Ignoring `RUSTC` environment variable; set `MIRI` if you want to control the binary used as the driver."
);
}
// Build scripts (and also cargo: https://github.com/rust-lang/cargo/issues/10885) will invoke
// `rustc` even when `RUSTC_WRAPPER` is set. To make sure everything is coherent, we want that
// to be the Miri driver, but acting as rustc, on the target level. (Target, rather than host,
// is needed for cross-interpretation situations.) This is not a perfect emulation of real rustc
// (it might be unable to produce binaries since the sysroot is check-only), but it's as close
// as we can get, and it's good enough for autocfg.
// Ideally we would set RUSTC to some non-existent path, so we can be sure our wrapping is
// always applied. However, buggy build scripts (https://github.com/eyre-rs/eyre/issues/84) and
// also cargo (https://github.com/rust-lang/cargo/issues/10885) will invoke `rustc` even when
// `RUSTC_WRAPPER` is set, bypassing the wrapper. To make sure everything is coherent, we want
// that to be the Miri driver, but acting as rustc, on the target level. (Target, rather than
// host, is needed for cross-interpretation situations.) This is not a perfect emulation of real
// rustc (it might be unable to produce binaries since the sysroot is check-only), but it's as
// close as we can get, and it's good enough for autocfg.
//
// In `main`, we need the value of `RUSTC` to distinguish RUSTC_WRAPPER invocations from rustdoc
// or TARGET_RUNNER invocations, so we canonicalize it here to make it exceedingly unlikely that
Expand Down Expand Up @@ -251,6 +253,16 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
/// Cargo does not give us this information directly, so we need to check
/// various command-line flags.
fn is_runnable_crate() -> bool {
// Determine whether this is cargo invoking rustc to get some infos. Ideally we'd check "is
// there a filename passed to rustc", but that's very hard as we would have to know whether
// e.g. `--print foo` is a booolean flag `--print` followed by filename `foo` or equivalent
// to `--print=foo`. So instead we use this more fragile approach of detecting the presence
// of a "query" flag rather than the absence of a filename.
let info_query = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV");
if info_query {
// Nothing to run.
return false;
}
let is_bin = get_arg_flag_value("--crate-type").as_deref().unwrap_or("bin") == "bin";
let is_test = has_arg_flag("--test");
is_bin || is_test
Expand Down Expand Up @@ -297,8 +309,6 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
let verbose = std::env::var("MIRI_VERBOSE")
.map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer"));
let target_crate = is_target_crate();
// Determine whether this is cargo invoking rustc to get some infos.
let info_query = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV");

let store_json = |info: CrateRunInfo| {
if get_arg_flag_value("--emit").unwrap_or_default().split(',').any(|e| e == "dep-info") {
Expand All @@ -325,7 +335,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
}
};

let runnable_crate = !info_query && is_runnable_crate();
let runnable_crate = is_runnable_crate();

if runnable_crate && target_crate {
assert!(
Expand Down Expand Up @@ -399,7 +409,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
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 !info_query && target_crate {
if target_crate {
// Forward arguments, but remove "link" from "--emit" to make this a check-only build.
let emit_flag = "--emit";
while let Some(arg) = args.next() {
Expand Down Expand Up @@ -433,17 +443,14 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
cmd.arg("-C").arg("panic=abort");
}
} else {
// For host crates (but not when we are just printing some info),
// we might still have to set the sysroot.
if !info_query {
// When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly
// due to bootstrap complications.
if let Some(sysroot) = std::env::var_os("MIRI_HOST_SYSROOT") {
cmd.arg("--sysroot").arg(sysroot);
}
// This is a host crate.
// When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly
// due to bootstrap complications.
if let Some(sysroot) = std::env::var_os("MIRI_HOST_SYSROOT") {
cmd.arg("--sysroot").arg(sysroot);
}

// For host crates or when we are printing, just forward everything.
// Forward everything.
cmd.args(args);
}

Expand All @@ -455,9 +462,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {

// Run it.
if verbose > 0 {
eprintln!(
"[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate} info_query={info_query}"
);
eprintln!("[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate}");
}

// Create a stub .rlib file if "link" was requested by cargo.
Expand Down Expand Up @@ -546,15 +551,13 @@ pub fn phase_runner(mut binary_args: impl Iterator<Item = String>, phase: Runner
// but when we run here, cargo does not interpret the JSON any more. `--json`
// then also needs to be dropped.
let mut args = info.args.into_iter();
let error_format_flag = "--error-format";
let json_flag = "--json";
while let Some(arg) = args.next() {
if arg == "--extern" {
forward_patched_extern_arg(&mut args, &mut cmd);
} else if let Some(suffix) = arg.strip_prefix(error_format_flag) {
} else if let Some(suffix) = arg.strip_prefix("--error-format") {
assert!(suffix.starts_with('='));
// Drop this argument.
} else if let Some(suffix) = arg.strip_prefix(json_flag) {
} else if let Some(suffix) = arg.strip_prefix("--json") {
assert!(suffix.starts_with('='));
// Drop this argument.
} else {
Expand Down Expand Up @@ -592,13 +595,11 @@ pub fn phase_rustdoc(mut args: impl Iterator<Item = String>) {
// just default to a straight-forward invocation for now:
let mut cmd = Command::new("rustdoc");

let extern_flag = "--extern";
let runtool_flag = "--runtool";
while let Some(arg) = args.next() {
if arg == extern_flag {
if arg == "--extern" {
// Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files.
forward_patched_extern_arg(&mut args, &mut cmd);
} else if arg == runtool_flag {
} else if arg == "--runtool" {
// An existing --runtool flag indicates cargo is running in cross-target mode, which we don't support.
// Note that this is only passed when cargo is run with the unstable -Zdoctest-xcompile flag;
// otherwise, we won't be called as rustdoc at all.
Expand Down
23 changes: 13 additions & 10 deletions miri-script/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,11 @@ impl Command {
Ok(())
}

fn run(dep: bool, flags: Vec<OsString>) -> Result<()> {
fn run(dep: bool, mut flags: Vec<OsString>) -> Result<()> {
let mut e = MiriEnv::new()?;
// Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so
// that we set the MIRI_SYSROOT up the right way.
// that we set the MIRI_SYSROOT up the right way. We must make sure that
// MIRI_TEST_TARGET and `--target` are in sync.
use itertools::Itertools;
let target = flags
.iter()
Expand All @@ -493,33 +494,35 @@ impl Command {
// Found it!
e.sh.set_var("MIRI_TEST_TARGET", target);
} else if let Ok(target) = std::env::var("MIRI_TEST_TARGET") {
// Make sure miri actually uses this target.
let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default();
e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}"));
// Convert `MIRI_TEST_TARGET` into `--target`.
flags.push("--target".into());
flags.push(target.into());
}
// Scan for "--edition" (we'll set one ourselves if that flag is not present).
// Scan for "--edition", set one ourselves if that flag is not present.
let have_edition =
flags.iter().take_while(|arg| *arg != "--").any(|arg| *arg == "--edition");
if !have_edition {
flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.`
}

// Prepare a sysroot.
e.build_miri_sysroot(/* quiet */ true)?;

// Then run the actual command.
// Then run the actual command. Also add MIRIFLAGS.
let miri_manifest = path!(e.miri_dir / "Cargo.toml");
let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default();
let miri_flags = flagsplit(&miri_flags);
let toolchain = &e.toolchain;
let extra_flags = &e.cargo_extra_flags;
let edition_flags = (!have_edition).then_some("--edition=2021"); // keep in sync with `tests/ui.rs`.`
if dep {
cmd!(
e.sh,
"cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode {miri_flags...} {edition_flags...} {flags...}"
"cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode {miri_flags...} {flags...}"
).quiet().run()?;
} else {
cmd!(
e.sh,
"cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {edition_flags...} {flags...}"
"cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {flags...}"
).quiet().run()?;
}
Ok(())
Expand Down
57 changes: 28 additions & 29 deletions tests/ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,13 @@ fn build_so_for_c_ffi_tests() -> PathBuf {
so_file_path
}

fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> Config {
/// Does *not* set any args or env vars, since it is shared between the test runner and
/// run_dep_mode.
fn miri_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> Config {
// Miri is rustc-like, so we create a default builder for rustc and modify it
let mut program = CommandBuilder::rustc();
program.program = miri_path();

// Add some flags we always want.
program.args.push("-Dwarnings".into());
program.args.push("-Dunused".into());
program.args.push("-Ainternal_features".into());
if let Ok(extra_flags) = env::var("MIRIFLAGS") {
for flag in extra_flags.split_whitespace() {
program.args.push(flag.into());
}
}
program.args.push("-Zui-testing".into());
program.args.push("--target".into());
program.args.push(target.into());

// If we're on linux, and we're testing the extern-so functionality,
// then build the shared object file for testing external C function calls
// and push the relevant compiler flag.
if cfg!(target_os = "linux") && path.starts_with("tests/extern-so/") {
let so_file_path = build_so_for_c_ffi_tests();
let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file=");
flag.push(so_file_path.into_os_string());
program.args.push(flag);
}

let mut config = Config {
target: Some(target.to_owned()),
stderr_filters: STDERR.clone(),
Expand Down Expand Up @@ -119,17 +98,38 @@ fn run_tests(
with_dependencies: bool,
tmpdir: &Path,
) -> Result<()> {
let mut config = test_config(target, path, mode, with_dependencies);
let mut config = miri_config(target, path, mode, with_dependencies);

// Add a test env var to do environment communication tests.
config.program.envs.push(("MIRI_ENV_VAR_TEST".into(), Some("0".into())));

// Let the tests know where to store temp files (they might run for a different target, which can make this hard to find).
config.program.envs.push(("MIRI_TEMP".into(), Some(tmpdir.to_owned().into())));

// If a test ICEs, we want to see a backtrace.
config.program.envs.push(("RUST_BACKTRACE".into(), Some("1".into())));

// Add some flags we always want.
config.program.args.push("-Dwarnings".into());
config.program.args.push("-Dunused".into());
config.program.args.push("-Ainternal_features".into());
if let Ok(extra_flags) = env::var("MIRIFLAGS") {
for flag in extra_flags.split_whitespace() {
config.program.args.push(flag.into());
}
}
config.program.args.push("-Zui-testing".into());
config.program.args.push("--target".into());
config.program.args.push(target.into());

// If we're on linux, and we're testing the extern-so functionality,
// then build the shared object file for testing external C function calls
// and push the relevant compiler flag.
if cfg!(target_os = "linux") && path.starts_with("tests/extern-so/") {
let so_file_path = build_so_for_c_ffi_tests();
let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file=");
flag.push(so_file_path.into_os_string());
config.program.args.push(flag);
}

// Handle command-line arguments.
let args = ui_test::Args::test()?;
let default_bless = env::var_os("RUSTC_BLESS").is_some_and(|v| v != "0");
Expand Down Expand Up @@ -292,13 +292,12 @@ fn main() -> Result<()> {

fn run_dep_mode(target: String, mut args: impl Iterator<Item = OsString>) -> Result<()> {
let path = args.next().expect("./miri run-dep must be followed by a file name");
let mut config = test_config(
let config = miri_config(
&target,
"",
Mode::Yolo { rustfix: RustfixMode::Disabled },
/* with dependencies */ true,
);
config.program.args.clear(); // We want to give the user full control over flags
let dep_args = config.build_dependencies()?;

let mut cmd = config.program.build(&config.out_dir);
Expand Down