Auto merge of #2436 - RalfJung:lib-crates, r=oli-obk

fix build.rs invoking RUSTC to do check builds

This makes the Miri driver, when invokved via the RUSTC env var from inside a build script, behave almost entirely like rustc. I had to redo how we propagate sysroot information for this (which is actually back to how we used to do sysroot propagation many years ago).

Fixes #2431

Author: bors

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.

cargo-miri/bin.rs

@@ -205,12 +205,6 @@ fn forward_patched_extern_arg(args: &mut impl Iterator<Item = String>, 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<Item = String>) {
     };
     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<Item = String>) {
     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<Item = String>) {
             "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<Item = String>, 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<Item = String>, 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<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 !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,20 +964,16 @@ fn phase_rustc(mut args: impl Iterator<Item = String>, 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")
         {
             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<Item = String>, 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<Item = String>, 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<Item = String>, 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<Item = String>) {
     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");
 

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' \