Skip to content
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.

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

Don't use MIRI_DEFAULT_ARGS to compile host crates #1761

Merged
merged 1 commit into from Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 8 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions cargo-miri/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
This conversation was marked as resolved.
Show resolved Hide resolved
// 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`
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down
33 changes: 26 additions & 7 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
//
Expand All @@ -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 || {
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
5 changes: 5 additions & 0 deletions test-cargo-miri/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test-cargo-miri/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
8 changes: 8 additions & 0 deletions test-cargo-miri/build.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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");
Expand Down
8 changes: 8 additions & 0 deletions test-cargo-miri/issue-1760/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "issue_1760"
version = "0.1.0"
authors = ["Miri Team"]
edition = "2018"

[lib]
proc-macro = true
10 changes: 10 additions & 0 deletions test-cargo-miri/issue-1760/build.rs
Original file line number Diff line number Diff line change
@@ -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());
}
9 changes: 9 additions & 0 deletions test-cargo-miri/issue-1760/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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()
}
1 change: 1 addition & 0 deletions test-cargo-miri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
3 changes: 3 additions & 0 deletions tests/run-pass/cfg_miri.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
assert!(cfg!(miri));
}