Skip to content

Commit

Permalink
miri-script: use --remap-path-prefix to print errors relative to the …
Browse files Browse the repository at this point in the history 
…right root
  • Loading branch information
@RalfJung
RalfJung committed Aug 9, 2024
1 parent 83248ce commit 9c0b5c1
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 61 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ anyone but Miri itself. They are used to communicate between different Miri
binaries, and as such worth documenting:

* `CARGO_EXTRA_FLAGS` is understood by `./miri` and passed to all host cargo invocations.
It is reserved for CI usage; setting the wrong flags this way can easily confuse the script.
* `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.
Expand Down
4 changes: 0 additions & 4 deletions cargo-miri/miri
View file

This file was deleted.

14 changes: 11 additions & 3 deletions miri
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
#!/usr/bin/env bash
set -e
# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through
# rustup (that sets it's own environmental variables), which is undesirable.
# We want to call the binary directly, so we need to know where it ends up.
MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target
cargo +stable build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml || \
# If stdout is not a terminal and we are not on CI, assume that we are being invoked by RA, and use JSON output.
if ! [ -t 1 ] && [ -z "$CI" ]; then
MESSAGE_FORMAT="--message-format=json"
fi
# Set --remap-path-prefix so miri-script build failures are printed relative to the Miri root.
RUSTFLAGS="$RUSTFLAGS --remap-path-prefix =miri-script" \
cargo +stable build $CARGO_EXTRA_FLAGS --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml \
-q --target-dir "$MIRI_SCRIPT_TARGET_DIR" $MESSAGE_FORMAT || \
( echo "Failed to build miri-script. Is the 'stable' toolchain installed?"; exit 1 )
# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through
# rustup (that sets it's own environmental variables), which is undesirable.
"$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@"
4 changes: 0 additions & 4 deletions miri-script/miri
View file

This file was deleted.

42 changes: 21 additions & 21 deletions miri-script/src/commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ impl MiriEnv {
return Ok(miri_sysroot.into());
}
let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml");
let Self { toolchain, cargo_extra_flags, .. } = &self;

// Make sure everything is built. Also Miri itself.
// Make sure everything is built. Also Miri itself; we need the Miri binary.
self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?;
let miri_bin =
path!(self.miri_dir / "target" / "debug" / format!("miri{}", env::consts::EXE_SUFFIX));
self.build(&manifest_path, &[], quiet)?;

let target_flag = if let Some(target) = &target {
Expand All @@ -56,10 +57,13 @@ impl MiriEnv {
eprintln!();
}

let mut cmd = cmd!(self.sh,
"cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} --
miri setup --print-sysroot {target_flag...}"
);
let mut cmd = self
.cargo_cmd(&manifest_path, "run")
.arg("--quiet")
.arg("--")
.args(&["miri", "setup", "--print-sysroot"])
.args(target_flag)
.env("MIRI", miri_bin);
cmd.set_quiet(quiet);
let output = cmd.read()?;
self.sh.set_var("MIRI_SYSROOT", &output);
Expand Down Expand Up @@ -513,23 +517,19 @@ impl Command {
let miri_manifest = path!(e.miri_dir / "Cargo.toml");
let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default();
let miri_flags = flagsplit(&miri_flags);
let toolchain = &e.toolchain;
let extra_flags = &e.cargo_extra_flags;
let quiet_flag = if verbose { None } else { Some("--quiet") };
// This closure runs the command with the given `seed_flag` added between the MIRIFLAGS and
// the `flags` given on the command-line.
let run_miri = |sh: &Shell, seed_flag: Option<String>| -> Result<()> {
let run_miri = |e: &MiriEnv, seed_flag: Option<String>| -> Result<()> {
// The basic command that executes the Miri driver.
let mut cmd = if dep {
cmd!(
sh,
"cargo +{toolchain} {quiet_flag...} test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode"
)
e.cargo_cmd(&miri_manifest, "test")
.args(&["--test", "ui"])
.args(quiet_flag)
.arg("--")
.args(&["--miri-run-dep-mode"])
} else {
cmd!(
sh,
"cargo +{toolchain} {quiet_flag...} run {extra_flags...} --manifest-path {miri_manifest} --"
)
e.cargo_cmd(&miri_manifest, "run").args(quiet_flag).arg("--")
};
cmd.set_quiet(!verbose);
// Add Miri flags
Expand All @@ -545,14 +545,14 @@ impl Command {
};
// Run the closure once or many times.
if let Some(seed_range) = many_seeds {
e.run_many_times(seed_range, |sh, seed| {
e.run_many_times(seed_range, |e, seed| {
eprintln!("Trying seed: {seed}");
run_miri(sh, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| {
run_miri(e, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| {
eprintln!("FAILING SEED: {seed}");
})
})?;
} else {
run_miri(&e.sh, None)?;
run_miri(&e, None)?;
}
Ok(())
}
Expand All @@ -579,6 +579,6 @@ impl Command {
.filter_ok(|item| item.file_type().is_file())
.map_ok(|item| item.into_path());

e.format_files(files, &e.toolchain[..], &config_path, &flags)
e.format_files(files, &config_path, &flags)
}
}
71 changes: 42 additions & 29 deletions miri-script/src/util.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::ffi::{OsStr, OsString};
use std::fmt::Write;
use std::ops::Range;
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
Expand All @@ -7,7 +8,7 @@ use std::thread;
use anyhow::{anyhow, Context, Result};
use dunce::canonicalize;
use path_macro::path;
use xshell::{cmd, Shell};
use xshell::{cmd, Cmd, Shell};

pub fn miri_dir() -> std::io::Result<PathBuf> {
const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR");
Expand All @@ -28,13 +29,14 @@ pub fn flagsplit(flags: &str) -> Vec<String> {
}

/// Some extra state we track for building Miri, such as the right RUSTFLAGS.
#[derive(Clone)]
pub struct MiriEnv {
/// miri_dir is the root of the miri repository checkout we are working in.
pub miri_dir: PathBuf,
/// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations.
pub toolchain: String,
/// Extra flags to pass to cargo.
pub cargo_extra_flags: Vec<String>,
cargo_extra_flags: Vec<String>,
/// The rustc sysroot
pub sysroot: PathBuf,
/// The shell we use.
Expand All @@ -59,10 +61,6 @@ impl MiriEnv {
println!("Please report a bug at https://github.com/rust-lang/miri/issues.");
std::process::exit(2);
}
// Share target dir between `miri` and `cargo-miri`.
let target_dir = std::env::var_os("CARGO_TARGET_DIR")
.unwrap_or_else(|| path!(miri_dir / "target").into());
sh.set_var("CARGO_TARGET_DIR", target_dir);

// We configure dev builds to not be unusably slow.
let devel_opt_level =
Expand Down Expand Up @@ -95,13 +93,37 @@ impl MiriEnv {
Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags })
}

pub fn cargo_cmd(&self, manifest_path: impl AsRef<OsStr>, cmd: &str) -> Cmd<'_> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
let manifest_path = Path::new(manifest_path.as_ref());
let cmd = cmd!(
self.sh,
"cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path}"
);
// Hard-code the target dir, since we rely on knowing where the binaries end up.
let manifest_dir = manifest_path.parent().unwrap();
let cmd = cmd.env("CARGO_TARGET_DIR", path!(manifest_dir / "target"));
// Apply path remapping to have errors printed relative to `miri_dir`.
let cmd = if let Ok(relative_to_miri) = manifest_dir.strip_prefix(&self.miri_dir) {
// Add `--remap-path-prefix` to RUSTFLAGS.
let mut rustflags = self.sh.var("RUSTFLAGS").unwrap();
write!(rustflags, " --remap-path-prefix ={}", relative_to_miri.display()).unwrap();
cmd.env("RUSTFLAGS", rustflags)
} else {
cmd
};
// All set up!
cmd
}

pub fn install_to_sysroot(
&self,
path: impl AsRef<OsStr>,
args: impl IntoIterator<Item = impl AsRef<OsStr>>,
) -> Result<()> {
let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self;
// Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains.
// (Not using `cargo_cmd` as `install` is special and doesn't use `--manifest-path`.)
cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?;
Ok(())
}
Expand All @@ -112,40 +134,31 @@ impl MiriEnv {
args: &[String],
quiet: bool,
) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
let quiet_flag = if quiet { Some("--quiet") } else { None };
// We build the tests as well, (a) to avoid having rebuilds when building the tests later
// and (b) to have more parallelism during the build of Miri and its tests.
let mut cmd = cmd!(
self.sh,
"cargo +{toolchain} build --bins --tests {cargo_extra_flags...} --manifest-path {manifest_path} {quiet_flag...} {args...}"
);
let mut cmd = self
.cargo_cmd(manifest_path, "build")
.args(&["--bins", "--tests"])
.args(quiet_flag)
.args(args);
cmd.set_quiet(quiet);
cmd.run()?;
Ok(())
}

pub fn check(&self, manifest_path: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
cmd!(self.sh, "cargo +{toolchain} check {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}")
.run()?;
self.cargo_cmd(manifest_path, "check").arg("--all-targets").args(args).run()?;
Ok(())
}

pub fn clippy(&self, manifest_path: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
cmd!(self.sh, "cargo +{toolchain} clippy {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}")
.run()?;
self.cargo_cmd(manifest_path, "clippy").arg("--all-targets").args(args).run()?;
Ok(())
}

pub fn test(&self, manifest_path: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
cmd!(
self.sh,
"cargo +{toolchain} test {cargo_extra_flags...} --manifest-path {manifest_path} {args...}"
)
.run()?;
self.cargo_cmd(manifest_path, "test").args(args).run()?;
Ok(())
}

Expand All @@ -155,7 +168,6 @@ impl MiriEnv {
pub fn format_files(
&self,
files: impl Iterator<Item = Result<PathBuf, walkdir::Error>>,
toolchain: &str,
config_path: &Path,
flags: &[String],
) -> anyhow::Result<()> {
Expand All @@ -166,6 +178,7 @@ impl MiriEnv {
// Format in batches as not all our files fit into Windows' command argument limit.
for batch in &files.chunks(256) {
// Build base command.
let toolchain = &self.toolchain;
let mut cmd = cmd!(
self.sh,
"rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}"
Expand Down Expand Up @@ -197,7 +210,7 @@ impl MiriEnv {
pub fn run_many_times(
&self,
range: Range<u32>,
run: impl Fn(&Shell, u32) -> Result<()> + Sync,
run: impl Fn(&Self, u32) -> Result<()> + Sync,
) -> Result<()> {
// `next` is atomic so threads can concurrently fetch their next value to run.
let next = AtomicU32::new(range.start);
Expand All @@ -207,10 +220,10 @@ impl MiriEnv {
let mut handles = Vec::new();
// Spawn one worker per core.
for _ in 0..thread::available_parallelism()?.get() {
// Create a copy of the shell for this thread.
let local_shell = self.sh.clone();
// Create a copy of the environment for this thread.
let local_miri = self.clone();
let handle = s.spawn(|| -> Result<()> {
let local_shell = local_shell; // move the copy into this thread.
let local_miri = local_miri; // move the copy into this thread.
// Each worker thread keeps asking for numbers until we're all done.
loop {
let cur = next.fetch_add(1, Ordering::Relaxed);
Expand All @@ -219,7 +232,7 @@ impl MiriEnv {
break;
}
// Run the command with this seed.
run(&local_shell, cur).inspect_err(|_| {
run(&local_miri, cur).inspect_err(|_| {
// If we failed, tell everyone about this.
failed.store(true, Ordering::Relaxed);
})?;
Expand Down

0 comments on commit 9c0b5c1

Please sign in to comment.