From a486efadb5d054ed1586032b7cc7c5e3c0d7602d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 Mar 2024 11:01:28 +0100 Subject: [PATCH] set --sysroot outside the driver rather than messing with the arguments passed to the driver --- README.md | 5 ++--- cargo-miri/src/phases.rs | 22 +++++++++++++++------- miri-script/src/commands.rs | 18 +++++++++++------- src/bin/miri.rs | 19 ------------------- tests/ui.rs | 7 +++++++ 5 files changed, 35 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index b043805291..4efaee40d4 100644 --- a/README.md +++ b/README.md @@ -464,10 +464,9 @@ Moreover, Miri recognizes some environment variables: standard library that it will build and use for interpretation. This directory must point to the `library` subdirectory of a `rust-lang/rust` repository checkout. -* `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When +* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the sysroot to use. When using `cargo miri`, this skips the automatic setup -- 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. When invoking `cargo miri setup`, this indicates where the sysroot + automatically created sysroot. When invoking `cargo miri setup`, this indicates where the sysroot will be put. * `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 diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index 9428f76deb..facd58d092 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -168,8 +168,6 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo. cmd.args(args); - // Let it know where the Miri sysroot lives. - cmd.env("MIRI_SYSROOT", miri_sysroot); // 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`.) @@ -196,10 +194,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // 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. + // that to be the Miri driver, but acting as rustc, on the target level. // // 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 @@ -208,11 +203,16 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // 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! + // In case we get invoked as RUSTC without the wrapper, let's be a host rustc. This makes no + // sense for cross-interpretation situations, but without the wrapper, this will use the host + // sysroot, so asking it to behave like a target build makes even less sense. + cmd.env("MIRI_BE_RUSTC", "host"); // 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); + // Forward some crucial information to our own re-invocations. + cmd.env("MIRI_SYSROOT", miri_sysroot); cmd.env("MIRI_LOCAL_CRATES", local_crates(&metadata)); if verbose > 0 { cmd.env("MIRI_VERBOSE", verbose.to_string()); // This makes the other phases verbose. @@ -405,6 +405,8 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { // 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 { + // Set the sysroot. + cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); // Forward arguments, but remove "link" from "--emit" to make this a check-only build. let emit_flag = "--emit"; while let Some(arg) = args.next() { @@ -533,6 +535,12 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner cmd.env(name, val); } + if phase != RunnerPhase::Rustdoc { + // Set the sysroot. Not necessary in rustdoc, where we already set the sysroot when invoking + // rustdoc itself, which will forward that flag when invoking rustc (i.e., us), so the flag + // is present in `info.args`. + cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); + } // 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 diff --git a/miri-script/src/commands.rs b/miri-script/src/commands.rs index 58deac6656..19095fe89c 100644 --- a/miri-script/src/commands.rs +++ b/miri-script/src/commands.rs @@ -2,6 +2,7 @@ use std::env; use std::ffi::OsString; use std::io::Write; use std::ops::Not; +use std::path::PathBuf; use std::process; use std::thread; use std::time; @@ -20,10 +21,11 @@ const JOSH_FILTER: &str = const JOSH_PORT: &str = "42042"; impl MiriEnv { - fn build_miri_sysroot(&mut self, quiet: bool) -> Result<()> { - if self.sh.var("MIRI_SYSROOT").is_ok() { + /// Returns the location of the sysroot. + fn build_miri_sysroot(&mut self, quiet: bool) -> Result { + if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") { // Sysroot already set, use that. - return Ok(()); + return Ok(miri_sysroot.into()); } let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); let Self { toolchain, cargo_extra_flags, .. } = &self; @@ -57,8 +59,8 @@ impl MiriEnv { .with_context(|| "`cargo miri setup` failed")?; panic!("`cargo miri setup` didn't fail again the 2nd time?"); }; - self.sh.set_var("MIRI_SYSROOT", output); - Ok(()) + self.sh.set_var("MIRI_SYSROOT", &output); + Ok(output.into()) } } @@ -505,8 +507,10 @@ impl Command { flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.` } - // Prepare a sysroot. - e.build_miri_sysroot(/* quiet */ true)?; + // Prepare a sysroot, and add it to the flags. + let miri_sysroot = e.build_miri_sysroot(/* quiet */ true)?; + flags.push("--sysroot".into()); + flags.push(miri_sysroot.into()); // Then run the actual command. Also add MIRIFLAGS. let miri_manifest = path!(e.miri_dir / "Cargo.toml"); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 8fffb91542..67a5bf3d14 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -271,25 +271,6 @@ fn run_compiler( callbacks: &mut (dyn rustc_driver::Callbacks + Send), using_internal_features: std::sync::Arc, ) -> ! { - if target_crate { - // Miri needs a custom sysroot for target crates. - // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where - // that sysroot lives, and that is passed to rustc. - let sysroot_flag = "--sysroot"; - if !args.iter().any(|e| e.starts_with(sysroot_flag)) { - // Using the built-in default here would be plain wrong, so we *require* - // the env var to make sure things make sense. - let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| { - show_error!( - "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set" - ) - }); - - args.push(sysroot_flag.to_owned()); - args.push(miri_sysroot); - } - } - // 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`: diff --git a/tests/ui.rs b/tests/ui.rs index a75fa4cf98..55a9ac6fb6 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -108,6 +108,13 @@ fn run_tests( config.program.envs.push(("RUST_BACKTRACE".into(), Some("1".into()))); // Add some flags we always want. + config.program.args.push( + format!( + "--sysroot={}", + env::var("MIRI_SYSROOT").expect("MIRI_SYSROOT must be set to run the ui test suite") + ) + .into(), + ); config.program.args.push("-Dwarnings".into()); config.program.args.push("-Dunused".into()); config.program.args.push("-Ainternal_features".into());