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.

Sign up for GitHub

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

Jump to bottom

fix build.rs invoking RUSTC to do check builds #2436

Merged
merged 3 commits into from
Jul 25, 2022
Merged

fix build.rs invoking RUSTC to do check builds #2436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d905901
make miri a better RUSTC by default inside cargo-miri
RalfJung Jul 24, 2022
0b1d5a4
test all of the autocfg queries
RalfJung Jul 24, 2022
9406b6d
fmt
RalfJung Jul 24, 2022
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
7 changes: 3 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
113 changes: 65 additions & 48 deletions cargo-miri/bin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{}",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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:
Expand All @@ -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!(
Expand Down Expand Up @@ -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() {
Expand All @@ -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") {
Expand All @@ -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);
Expand Down Expand Up @@ -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(|_| {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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");

Expand Down
8 changes: 6 additions & 2 deletions miri
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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' \
Expand Down
Loading