Auto merge of #1761 - hyd-dev:build-rs-proc-macro-no-default-args, r=RalfJung

Don't use `MIRI_DEFAULT_ARGS` to compile host crates

They (specifically, `--cfg=miri`) may cause procedural macros (and probably build scripts) to depend on Miri-only symbols, such as `miri_resolve_frame`.

This PR makes `miri` detect host crates inspecting the value of the `MIRI_BE_RUSTC` environment variable (`target` -> target crate, `host` -> host crate, other -> panic) and skip the insertion of `MIRI_DEFAULT_ARGS` if it's a host crate.

Fixes #1760

Author: bors

README.md

@@ -100,9 +100,9 @@ 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.
 
-When compiling code via `cargo miri`, the `cfg(miri)` config flag is set. You
-can use this to ignore test cases that fail under Miri because they do things
-Miri does not support:
+When compiling code via `cargo miri`, the `cfg(miri)` config flag is set for code
+that will be interpret under Miri. You can use this to ignore test cases that fail
+under Miri because they do things Miri does not support:
 
 ```rust
 #[test]
@@ -286,9 +286,11 @@ Moreover, Miri recognizes some environment variables:
 The following environment variables are internal, but used to communicate between
 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_BE_RUSTC` can be set to `host` or `target`. It tells the Miri driver to
+  actually not interpret the code but compile it like rustc would. With `target`, Miri sets
+  some compiler flags to prepare the code for interpretation; with `host`, this is not done.
+  This environment variable is useful to be sure that the compiled `rlib`s are compatible
+  with Miri.
   When set while running `cargo-miri`, it indicates that we are part of a sysroot
   build (for which some crates need special treatment).
 * `MIRI_CALLED_FROM_RUSTDOC` when set to any value tells `cargo-miri` that it is

cargo-miri/bin.rs

@@ -420,7 +420,7 @@ path = "lib.rs"
     } else {
         command.env("RUSTC", &cargo_miri_path);
     }
-    command.env("MIRI_BE_RUSTC", "1");
+    command.env("MIRI_BE_RUSTC", "target");
     // Make sure there are no other wrappers or flags getting in our way
     // (Cc https://github.com/rust-lang/miri/issues/1421).
     // This is consistent with normal `cargo build` that does not apply `RUSTFLAGS`
@@ -694,7 +694,7 @@ fn phase_cargo_rustc(mut args: env::Args) {
             }
 
             cmd.args(&env.args);
-            cmd.env("MIRI_BE_RUSTC", "1");
+            cmd.env("MIRI_BE_RUSTC", "target");
 
             if verbose {
                 eprintln!("[cargo-miri rustc] captured input:\n{}", std::str::from_utf8(&env.stdin).unwrap());
@@ -758,7 +758,9 @@ fn phase_cargo_rustc(mut args: env::Args) {
 
     // 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");
+    // MIRI_DEFAULT_ARGS should not be used to build host crates, hence setting "target" or "host"
+    // as the value here to help Miri differentiate them.
+    cmd.env("MIRI_BE_RUSTC", if target_crate { "target" } else { "host" });
 
     // Run it.
     if verbose {

src/bin/miri.rs

@@ -135,7 +135,11 @@ fn compile_time_sysroot() -> Option<String> {
 }
 
 /// Execute a compiler with the given CLI arguments and callbacks.
-fn run_compiler(mut args: Vec<String>, callbacks: &mut (dyn rustc_driver::Callbacks + Send)) -> ! {
+fn run_compiler(
+    mut args: Vec<String>,
+    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`.
     //
@@ -151,9 +155,11 @@ fn run_compiler(mut args: Vec<String>, callbacks: &mut (dyn rustc_driver::Callba
         }
     }
 
-    // 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));
+    if insert_default_args {
+        // 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));
+    }
 
     // Invoke compiler, and handle return code.
     let exit_code = rustc_driver::catch_with_exit_code(move || {
@@ -166,11 +172,24 @@ fn main() {
     rustc_driver::install_ice_hook();
 
     // If the environment asks us to actually be rustc, then do that.
-    if env::var_os("MIRI_BE_RUSTC").is_some() {
+    if let Some(crate_kind) = env::var_os("MIRI_BE_RUSTC") {
         rustc_driver::init_rustc_env_logger();
+
+        // 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
+        let insert_default_args = if crate_kind == "target" {
+            true
+        } else if crate_kind == "host" {
+            false
+        } else {
+            panic!("invalid `MIRI_BE_RUSTC` value: {:?}", crate_kind)
+        };
+
         // We cannot use `rustc_driver::main` as we need to adjust the CLI arguments.
         let mut callbacks = rustc_driver::TimePassesCallbacks::default();
-        run_compiler(env::args().collect(), &mut callbacks)
+        run_compiler(env::args().collect(), &mut callbacks, insert_default_args)
     }
 
     // Init loggers the Miri way.
@@ -300,5 +319,5 @@ fn main() {
 
     debug!("rustc arguments: {:?}", rustc_args);
     debug!("crate arguments: {:?}", miri_config.args);
-    run_compiler(rustc_args, &mut MiriCompilerCalls { miri_config })
+    run_compiler(rustc_args, &mut MiriCompilerCalls { miri_config }, /* insert_default_args: */ true)
 }

test-cargo-miri/Cargo.lock

@@ -13,6 +13,7 @@ cdylib = { path = "cdylib" }
 issue_1567 = { path = "issue-1567" }
 issue_1691 = { path = "issue-1691" }
 issue_1705 = { path = "issue-1705" }
+issue_1760 = { path = "issue-1760" }
 
 [dev-dependencies]
 rand = { version = "0.8", features = ["small_rng"] }

test-cargo-miri/Cargo.toml

@@ -1,5 +1,10 @@
 #![feature(llvm_asm)]
 
+use std::env;
+
+#[cfg(miri)]
+compile_error!("`miri` cfg should not be set in build script");
+
 fn not_in_miri() -> i32 {
     // Inline assembly definitely does not work in Miri.
     let dummy = 42;
@@ -11,6 +16,9 @@ fn not_in_miri() -> i32 {
 
 fn main() {
     not_in_miri();
+    // Cargo calls `miri --print=cfg` to populate the `CARGO_CFG_*` env vars.
+    // Make sure that the "miri" flag is set.
+    assert!(env::var_os("CARGO_CFG_MIRI").is_some());
     println!("cargo:rerun-if-changed=build.rs");
     println!("cargo:rerun-if-env-changed=MIRITESTVAR");
     println!("cargo:rustc-env=MIRITESTVAR=testval");

test-cargo-miri/build.rs

@@ -0,0 +1,8 @@
+[package]
+name = "issue_1760"
+version = "0.1.0"
+authors = ["Miri Team"]
+edition = "2018"
+
+[lib]
+proc-macro = true

test-cargo-miri/issue-1760/Cargo.toml

@@ -0,0 +1,10 @@
+use std::env;
+
+#[cfg(miri)]
+compile_error!("`miri` cfg should not be set in build script");
+
+fn main() {
+    // Cargo calls `miri --print=cfg` to populate the `CARGO_CFG_*` env vars.
+    // Make sure that the "miri" flag is not set since we are building a procedural macro crate.
+    assert!(env::var_os("CARGO_CFG_MIRI").is_none());
+}

test-cargo-miri/issue-1760/build.rs

@@ -0,0 +1,9 @@
+use proc_macro::TokenStream;
+
+#[cfg(miri)]
+compile_error!("`miri` cfg should not be set in proc-macro");
+
+#[proc_macro]
+pub fn use_the_dependency(_: TokenStream) -> TokenStream {
+    TokenStream::new()
+}

test-cargo-miri/issue-1760/src/lib.rs

@@ -11,5 +11,6 @@
 pub fn make_true() -> bool {
     issue_1567::use_the_dependency();
     issue_1705::use_the_dependency();
+    issue_1760::use_the_dependency!();
     issue_1691::use_me()
 }