diff --git a/README.md b/README.md index fa235a45f5..5fbf89c86b 100644 --- a/README.md +++ b/README.md @@ -409,10 +409,9 @@ Moreover, Miri recognizes some environment variables: checkout. Note that changing files in that directory does not automatically trigger a re-build of the standard library; you have to clear the Miri build cache manually (on Linux, `rm -rf ~/.cache/miri`). -* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the - sysroot to use. Only set this if you do not want to use the automatically - created sysroot. (The `miri` driver sysroot is controlled via the `--sysroot` - flag instead.) +* `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When + using `cargo miri`, only set this if you do not want to use the automatically created sysroot. For + directly invoking the Miri driver, this variable (or a `--sysroot` flag) is mandatory. * `MIRI_TEST_TARGET` (recognized by the test suite and the `./miri` script) indicates which target architecture to test against. `miri` and `cargo miri` accept the `--target` flag for the same purpose. diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 216584ac9b..8aeeb044d3 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -205,12 +205,6 @@ fn forward_patched_extern_arg(args: &mut impl Iterator, cmd: &mut } } -fn forward_miri_sysroot(cmd: &mut Command) { - let sysroot = env::var_os("MIRI_SYSROOT").expect("the wrapper should have set MIRI_SYSROOT"); - cmd.arg("--sysroot"); - cmd.arg(sysroot); -} - /// Escapes `s` in a way that is suitable for using it as a string literal in TOML syntax. fn escape_for_toml(s: &str) -> String { // We want to surround this string in quotes `"`. So we first escape all quotes, @@ -237,8 +231,15 @@ fn miri() -> Command { Command::new(find_miri()) } +fn miri_for_host() -> Command { + let mut cmd = miri(); + cmd.env("MIRI_BE_RUSTC", "host"); + cmd +} + fn version_info() -> VersionMeta { - VersionMeta::for_command(miri()).expect("failed to determine underlying rustc version of Miri") + VersionMeta::for_command(miri_for_host()) + .expect("failed to determine underlying rustc version of Miri") } fn cargo() -> Command { @@ -336,7 +337,7 @@ fn ask_to_run(mut cmd: Command, ask: bool, text: &str) { a => show_error(format!("invalid answer `{}`", a)), }; } else { - println!("Running `{:?}` to {}.", cmd, text); + eprintln!("Running `{:?}` to {}.", cmd, text); } if cmd.status().unwrap_or_else(|_| panic!("failed to execute {:?}", cmd)).success().not() { @@ -364,7 +365,7 @@ fn write_to_file(filename: &Path, content: &str) { /// Performs the setup required to make `cargo miri` work: Getting a custom-built libstd. Then sets /// `MIRI_SYSROOT`. Skipped if `MIRI_SYSROOT` is already set, in which case we expect the user has /// done all this already. -fn setup(subcommand: &MiriCommand) { +fn setup(subcommand: &MiriCommand, host: &str, target: &str) { let only_setup = matches!(subcommand, MiriCommand::Setup); let ask_user = !only_setup; let print_sysroot = only_setup && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path @@ -398,8 +399,10 @@ fn setup(subcommand: &MiriCommand) { } None => { // Check for `rust-src` rustup component. - let output = - miri().args(&["--print", "sysroot"]).output().expect("failed to determine sysroot"); + let output = miri_for_host() + .args(&["--print", "sysroot"]) + .output() + .expect("failed to determine sysroot"); if !output.status.success() { show_error(format!( "Failed to determine sysroot; Miri said:\n{}", @@ -472,18 +475,21 @@ path = "lib.rs" ); write_to_file(&dir.join("lib.rs"), "#![no_std]"); - // Determine architectures. - // We always need to set a target so rustc bootstrap can tell apart host from target crates. - let host = version_info().host; - let target = get_arg_flag_value("--target"); - let target = target.as_ref().unwrap_or(&host); + // Figure out where xargo will build its stuff. + // Unfortunately, it puts things into a different directory when the + // architecture matches the host. + let sysroot = if target == host { dir.join("HOST") } else { PathBuf::from(dir) }; + // Make sure all target-level Miri invocations know their sysroot. + std::env::set_var("MIRI_SYSROOT", &sysroot); + // Now invoke xargo. let mut command = xargo_check(); command.arg("check").arg("-q"); - command.arg("--target").arg(target); command.current_dir(&dir); command.env("XARGO_HOME", &dir); command.env("XARGO_RUST_SRC", &rust_src); + // We always need to set a target so rustc bootstrap can tell apart host from target crates. + command.arg("--target").arg(target); // Use Miri as rustc to build a libstd compatible with us (and use the right flags). // However, when we are running in bootstrap, we cannot just overwrite `RUSTC`, // because we still need bootstrap to distinguish between host and target crates. @@ -523,6 +529,7 @@ path = "lib.rs" command.stdout(process::Stdio::null()); command.stderr(process::Stdio::null()); } + // Finally run it! if command.status().expect("failed to run xargo").success().not() { if only_setup { @@ -534,11 +541,6 @@ path = "lib.rs" } } - // That should be it! But we need to figure out where xargo built stuff. - // Unfortunately, it puts things into a different directory when the - // architecture matches the host. - let sysroot = if target == &host { dir.join("HOST") } else { PathBuf::from(dir) }; - std::env::set_var("MIRI_SYSROOT", &sysroot); // pass the env var to the processes we spawn, which will turn it into "--sysroot" flags // Figure out what to print. if only_setup { eprintln!("A sysroot for Miri is now available in `{}`.", sysroot.display()); @@ -677,8 +679,13 @@ fn phase_cargo_miri(mut args: impl Iterator) { }; let verbose = num_arg_flag("-v"); + // Determine the involved architectures. + let host = version_info().host; + let target = get_arg_flag_value("--target"); + let target = target.as_ref().unwrap_or(&host); + // We always setup. - setup(&subcommand); + setup(&subcommand, &host, target); // Invoke actual cargo for the job, but with different flags. // We re-use `cargo test` and `cargo run`, which makes target and binary handling very easy but @@ -727,7 +734,7 @@ fn phase_cargo_miri(mut args: impl Iterator) { if get_arg_flag_value("--target").is_none() { // No target given. Explicitly pick the host. cmd.arg("--target"); - cmd.arg(version_info().host); + cmd.arg(&host); } // Set ourselves as runner for al binaries invoked by cargo. @@ -754,16 +761,21 @@ fn phase_cargo_miri(mut args: impl Iterator) { "WARNING: Ignoring `RUSTC` environment variable; set `MIRI` if you want to control the binary used as the driver." ); } - // We'd prefer to just clear this env var, but cargo does not always honor `RUSTC_WRAPPER` - // (https://github.com/rust-lang/cargo/issues/10885). There is no good way to single out these invocations; - // some build scripts use the RUSTC env var as well. So we set it directly to the `miri` driver and - // hope that all they do is ask for the version number -- things could quickly go downhill from here. + // 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. + // // 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 - // there would be a collision with other invocations of cargo-miri (as rustdoc or as runner). - // We explicitly do this even if RUSTC_STAGE is set, since for these builds we do *not* want the - // bootstrap `rustc` thing in our way! Instead, we have MIRI_HOST_SYSROOT to use for host builds. + // there would be a collision with other invocations of cargo-miri (as rustdoc or as runner). We + // explicitly do this even if RUSTC_STAGE is set, since for these builds we do *not* want the + // bootstrap `rustc` thing in our way! Instead, we have MIRI_HOST_SYSROOT to use for host + // builds. cmd.env("RUSTC", &fs::canonicalize(find_miri()).unwrap()); + cmd.env("MIRI_BE_RUSTC", "target"); // we better remember to *unset* this in the other phases! // Set rustdoc to us as well, so we can run doctests. cmd.env("RUSTDOC", &cargo_miri_path); @@ -832,10 +844,16 @@ fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } } + // phase_cargo_miri set `MIRI_BE_RUSTC` for when build scripts directly invoke the driver; + // however, if we get called back by cargo here, we'll carefully compute the right flags + // ourselves, so we first un-do what the earlier phase did. + env::remove_var("MIRI_BE_RUSTC"); + 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(); - let print = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV"); // whether this is cargo/xargo invoking rustc to get some infos + // Determine whether this is cargo/xargo 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| { // Create a stub .d file to stop Cargo from "rebuilding" the crate: @@ -857,7 +875,7 @@ fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { info.store(&out_filename("", ".exe")); }; - let runnable_crate = !print && is_runnable_crate(); + let runnable_crate = !info_query && is_runnable_crate(); if runnable_crate && target_crate { assert!( @@ -919,7 +937,7 @@ fn phase_rustc(mut args: impl Iterator, 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 !print && target_crate { + if !info_query && 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() { @@ -946,11 +964,6 @@ fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } } - // Use our custom sysroot (but not if that is what we are currently building). - if phase != RustcPhase::Setup { - forward_miri_sysroot(&mut cmd); - } - // During setup, patch the panic runtime for `libpanic_abort` (mirroring what bootstrap usually does). if phase == RustcPhase::Setup && get_arg_flag_value("--crate-name").as_deref() == Some("panic_abort") @@ -958,8 +971,9 @@ fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { cmd.arg("-C").arg("panic=abort"); } } else { - // For host crates (but not when we are printing), we might still have to set the sysroot. - if !print { + // 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") { @@ -980,7 +994,7 @@ fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { // Run it. if verbose > 0 { eprintln!( - "[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate} print={print}" + "[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate} info_query={info_query}" ); } debug_cmd("[cargo-miri rustc]", verbose, &cmd); @@ -1010,12 +1024,19 @@ enum RunnerPhase { } fn phase_runner(mut binary_args: impl Iterator, phase: RunnerPhase) { + // phase_cargo_miri set `MIRI_BE_RUSTC` for when build scripts directly invoke the driver; + // however, if we get called back by cargo here, we'll carefully compute the right flags + // ourselves, so we first un-do what the earlier phase did. + env::remove_var("MIRI_BE_RUSTC"); + let verbose = std::env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); let binary = binary_args.next().unwrap(); 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 = serde_json::from_reader(file).unwrap_or_else(|_| { @@ -1077,10 +1098,6 @@ fn phase_runner(mut binary_args: impl Iterator, phase: RunnerPhas cmd.arg(arg); } } - // Set sysroot (if we are inside rustdoc, we already did that in `phase_cargo_rustdoc`). - if phase != RunnerPhase::Rustdoc { - forward_miri_sysroot(&mut cmd); - } // Respect `MIRIFLAGS`. if let Ok(a) = env::var("MIRIFLAGS") { // This code is taken from `RUSTFLAGS` handling in cargo. @@ -1151,7 +1168,7 @@ fn phase_rustdoc(mut args: impl Iterator) { cmd.arg("-Z").arg("unstable-options"); // rustdoc needs to know the right sysroot. - forward_miri_sysroot(&mut cmd); + cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); // make sure the 'miri' flag is set for rustdoc cmd.arg("--cfg").arg("miri"); diff --git a/miri b/miri index 8bef9d1291..956b8cca75 100755 --- a/miri +++ b/miri @@ -131,7 +131,11 @@ export RUSTFLAGS="-C link-args=-Wl,-rpath,$LIBDIR $RUSTFLAGS" # Build a sysroot and set MIRI_SYSROOT to use it. Arguments are passed to `cargo miri setup`. build_sysroot() { - export MIRI_SYSROOT="$($CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")" + if ! MIRI_SYSROOT="$($CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")"; then + echo "'cargo miri setup' failed" + exit 1 + fi + export MIRI_SYSROOT } # Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account @@ -201,7 +205,7 @@ run) $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml find_sysroot # Then run the actual command. - exec $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- --sysroot "$MIRI_SYSROOT" $MIRIFLAGS "$@" + exec $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- $MIRIFLAGS "$@" ;; fmt) find "$MIRIDIR" -not \( -name target -prune \) -name '*.rs' \ diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 14694ac7b0..489eb95990 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -27,7 +27,7 @@ use rustc_middle::{ }, ty::{query::ExternProviders, TyCtxt}, }; -use rustc_session::{search_paths::PathKind, CtfeBacktrace}; +use rustc_session::{config::CrateType, search_paths::PathKind, CtfeBacktrace}; use miri::{BacktraceStyle, ProvenanceMode}; @@ -60,6 +60,10 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { queries.global_ctxt().unwrap().peek_mut().enter(|tcx| { init_late_loggers(tcx); + if !tcx.sess.crate_types().contains(&CrateType::Executable) { + tcx.sess.fatal("miri only makes sense on bin crates"); + } + let (entry_def_id, entry_type) = if let Some(entry_def) = tcx.entry_fn(()) { entry_def } else { @@ -204,9 +208,9 @@ fn init_late_loggers(tcx: TyCtxt<'_>) { } } -/// Returns the "default sysroot" that Miri will use if no `--sysroot` flag is set. +/// Returns the "default sysroot" that Miri will use for host things if no `--sysroot` flag is set. /// Should be a compile-time constant. -fn compile_time_sysroot() -> Option { +fn host_sysroot() -> Option { if option_env!("RUSTC_STAGE").is_some() { // This is being built as part of rustc, and gets shipped with rustup. // We can rely on the sysroot computation in librustc_session. @@ -227,7 +231,7 @@ fn compile_time_sysroot() -> Option { if toolchain_runtime != toolchain { show_error(format!( "This Miri got built with local toolchain `{toolchain}`, but now is being run under a different toolchain. \n\ - Make sure to run Miri in the toolchain it got built with, e.g. via `cargo +{toolchain} miri`." + Make sure to run Miri in the toolchain it got built with, e.g. via `cargo +{toolchain} miri`." )); } } @@ -246,25 +250,42 @@ fn compile_time_sysroot() -> Option { /// Execute a compiler with the given CLI arguments and callbacks. fn run_compiler( mut args: Vec, + target_crate: bool, callbacks: &mut (dyn rustc_driver::Callbacks + Send), - insert_default_args: bool, ) -> ! { // Make sure we use the right default sysroot. The default sysroot is wrong, // because `get_or_default_sysroot` in `librustc_session` bases that on `current_exe`. // - // Make sure we always call `compile_time_sysroot` as that also does some sanity-checks - // of the environment we were built in. - // FIXME: Ideally we'd turn a bad build env into a compile-time error via CTFE or so. - if let Some(sysroot) = compile_time_sysroot() { - let sysroot_flag = "--sysroot"; - if !args.iter().any(|e| e == sysroot_flag) { + // Make sure we always call `host_sysroot` as that also does some sanity-checks + // of the environment we were built in and whether it matches what we are running in. + let host_default_sysroot = host_sysroot(); + // Now see if we even need to set something. + let sysroot_flag = "--sysroot"; + if !args.iter().any(|e| e == sysroot_flag) { + // No sysroot was set, let's see if we have a custom default we want to configure. + let default_sysroot = if target_crate { + // Using the built-in default here would be plain wrong, so we *require* + // the env var to make sure things make sense. + Some(env::var("MIRI_SYSROOT").unwrap_or_else(|_| { + show_error(format!( + "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set" + )) + })) + } else { + host_default_sysroot + }; + if let Some(sysroot) = default_sysroot { // We need to overwrite the default that librustc_session would compute. args.push(sysroot_flag.to_owned()); args.push(sysroot); } } - if insert_default_args { + // Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building + // a "host" crate. That may cause procedural macros (and probably build scripts) to + // depend on Miri-only symbols, such as `miri_resolve_frame`: + // https://github.com/rust-lang/miri/issues/1760 + if target_crate { // Some options have different defaults in Miri than in plain rustc; apply those by making // them the first arguments after the binary name (but later arguments can overwrite them). args.splice(1..1, miri::MIRI_DEFAULT_ARGS.iter().map(ToString::to_string)); @@ -302,13 +323,8 @@ fn main() { // We cannot use `rustc_driver::main` as we need to adjust the CLI arguments. run_compiler( env::args().collect(), + target_crate, &mut MiriBeRustCompilerCalls { target_crate }, - // Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building - // a "host" crate. That may cause procedural macros (and probably build scripts) to - // depend on Miri-only symbols, such as `miri_resolve_frame`: - // https://github.com/rust-lang/miri/issues/1760 - #[rustfmt::skip] - /* insert_default_args: */ target_crate, ) } @@ -502,9 +518,5 @@ fn main() { debug!("rustc arguments: {:?}", rustc_args); debug!("crate arguments: {:?}", miri_config.args); - run_compiler( - rustc_args, - &mut MiriCompilerCalls { miri_config }, - /* insert_default_args: */ true, - ) + run_compiler(rustc_args, /* target_crate: */ true, &mut MiriCompilerCalls { miri_config }) } diff --git a/test-cargo-miri/Cargo.lock b/test-cargo-miri/Cargo.lock index 38bfb2ac62..a297dd27db 100644 --- a/test-cargo-miri/Cargo.lock +++ b/test-cargo-miri/Cargo.lock @@ -2,6 +2,12 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "autocfg" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" + [[package]] name = "byteorder" version = "0.5.3" @@ -18,6 +24,7 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" name = "cargo-miri-test" version = "0.1.0" dependencies = [ + "autocfg", "byteorder 0.5.3", "byteorder 1.4.3", "cdylib", diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 51967c54e1..5d9e5d143b 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -21,6 +21,9 @@ issue_rust_86261 = { path = "issue-rust-86261" } byteorder_2 = { package = "byteorder", version = "0.5" } # to test dev-dependencies behave as expected, with renaming serde_derive = "1.0" # not actually used, but exercises some unique code path (`--extern` .so file) +[build-dependencies] +autocfg = "1" + [lib] test = false # test that this is respected (will show in the output) diff --git a/test-cargo-miri/build.rs b/test-cargo-miri/build.rs index 8b1e3af623..6c1f4d80d3 100644 --- a/test-cargo-miri/build.rs +++ b/test-cargo-miri/build.rs @@ -20,4 +20,21 @@ fn main() { println!("cargo:rerun-if-changed=build.rs"); println!("cargo:rerun-if-env-changed=MIRITESTVAR"); println!("cargo:rustc-env=MIRITESTVAR=testval"); + + // Test that autocfg works. This invokes RUSTC. + let a = autocfg::new(); + assert!(a.probe_sysroot_crate("std")); + assert!(!a.probe_sysroot_crate("doesnotexist")); + assert!(a.probe_rustc_version(1, 0)); + assert!(!a.probe_rustc_version(2, 0)); + assert!(a.probe_type("i128")); + assert!(!a.probe_type("doesnotexist")); + assert!(a.probe_trait("Send")); + assert!(!a.probe_trait("doesnotexist")); + assert!(a.probe_path("std::num")); + assert!(!a.probe_path("doesnotexist")); + assert!(a.probe_constant("i32::MAX")); + assert!(!a.probe_constant("doesnotexist")); + assert!(a.probe_expression("Box::new(0)")); + assert!(!a.probe_expression("doesnotexist")); } diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 0f0179de5d..48e0ae855b 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -24,10 +24,6 @@ fn run_tests(mode: Mode, path: &str, target: Option) -> Result<()> { flags.push("-Dwarnings".into()); flags.push("-Dunused".into()); } - if let Some(sysroot) = env::var_os("MIRI_SYSROOT") { - flags.push("--sysroot".into()); - flags.push(sysroot); - } if let Ok(extra_flags) = env::var("MIRIFLAGS") { for flag in extra_flags.split_whitespace() { flags.push(flag.into());