mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #2477 - RalfJung:show-error, r=RalfJung

Browse Source 
don't make it quite so easy to get Miri to panic

Panicking on incorrect `-Zmiri` flags is a bit embarrassing, so let's finally fix that.
This commit is contained in:
bors 2022-08-08 15:37:01 +00:00
commit df3c141762
5 changed files with 69 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
cargo-miri/src/main.rs
View File

@ -1,15 +1,17 @@
#![feature(let_else)]
#![allow(clippy::useless_format, clippy::derive_partial_eq_without_eq, rustc::internal)]
#[macro_use]
mod util;
mod arg;
mod phases;
mod setup;
mod util;
mod version;
use std::{env, iter};
use crate::{phases::*, util::*};
use crate::phases::*;
fn main() {
// Rustc does not support non-UTF-8 arguments so we make no attempt either.
@ -73,9 +75,9 @@ fn main() {
}
let Some(first) = args.next() else {
show_error(format!(
show_error!(
"`cargo-miri` called without first argument; please only invoke this binary through `cargo miri`"
))
)
};
match first.as_str() {
"miri" => phase_cargo_miri(args),

19
cargo-miri/src/phases.rs
View File

@ -77,15 +77,15 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
// We cannot know which of those flags take arguments and which do not,
// so we cannot detect subcommands later.
let Some(subcommand) = args.next() else {
show_error(format!("`cargo miri` needs to be called with a subcommand (`run`, `test`)"));
show_error!("`cargo miri` needs to be called with a subcommand (`run`, `test`)");
};
let subcommand = match &*subcommand {
"setup" => MiriCommand::Setup,
"test" | "t" | "run" | "r" | "nextest" => MiriCommand::Forward(subcommand),
_ =>
show_error(format!(
show_error!(
"`cargo miri` supports the following subcommands: `run`, `test`, `nextest`, and `setup`."
)),
),
};
let verbose = num_arg_flag("-v");
@ -123,7 +123,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
match arg {
Ok(value) => {
if target_dir.is_some() {
show_error(format!("`--target-dir` is provided more than once"));
show_error!("`--target-dir` is provided more than once");
}
target_dir = Some(value.into());
}
@ -456,16 +456,13 @@ pub fn phase_runner(mut binary_args: impl Iterator<Item = String>, phase: Runner
let binary = binary_args.next().unwrap();
let file = File::open(&binary)
.unwrap_or_else(|_| show_error(format!(
.unwrap_or_else(|_| show_error!(
"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(|_| {
show_error(format!(
"file {:?} contains outdated or invalid JSON; try `cargo clean`",
binary
))
show_error!("file {:?} contains outdated or invalid JSON; try `cargo clean`", binary)
});
let info = match info {
CrateRunInfo::RunWith(info) => info,
@ -562,7 +559,7 @@ pub fn phase_rustdoc(mut args: impl Iterator<Item = String>) {
// An existing --runtool flag indicates cargo is running in cross-target mode, which we don't support.
// Note that this is only passed when cargo is run with the unstable -Zdoctest-xcompile flag;
// otherwise, we won't be called as rustdoc at all.
show_error(format!("cross-interpreting doctests is not currently supported by Miri."));
show_error!("cross-interpreting doctests is not currently supported by Miri.");
} else {
cmd.arg(arg);
}

18
cargo-miri/src/setup.rs
View File

@ -73,7 +73,7 @@ pub fn setup(subcommand: &MiriCommand, host: &str, target: &str) {
if xargo_version().map_or(true, |v| v < XARGO_MIN_VERSION) {
if std::env::var_os("XARGO_CHECK").is_some() {
// The user manually gave us a xargo binary; don't do anything automatically.
show_error(format!("xargo is too old; please upgrade to the latest version"))
show_error!("xargo is too old; please upgrade to the latest version")
}
let mut cmd = cargo();
cmd.args(&["install", "xargo"]);
@ -97,10 +97,10 @@ pub fn setup(subcommand: &MiriCommand, host: &str, target: &str) {
.output()
.expect("failed to determine sysroot");
if !output.status.success() {
show_error(format!(
show_error!(
"Failed to determine sysroot; Miri said:\n{}",
String::from_utf8_lossy(&output.stderr).trim_end()
));
);
}
let sysroot = std::str::from_utf8(&output.stdout).unwrap();
let sysroot = Path::new(sysroot.trim_end_matches('\n'));
@ -121,14 +121,14 @@ pub fn setup(subcommand: &MiriCommand, host: &str, target: &str) {
}
};
if !rust_src.exists() {
show_error(format!("given Rust source directory `{}` does not exist.", rust_src.display()));
show_error!("given Rust source directory `{}` does not exist.", rust_src.display());
}
if rust_src.file_name().and_then(OsStr::to_str) != Some("library") {
show_error(format!(
show_error!(
"given Rust source directory `{}` does not seem to be the `library` subdirectory of \
a Rust source checkout.",
rust_src.display()
));
);
}
// Next, we need our own libstd. Prepare a xargo project for that purpose.
@ -226,11 +226,9 @@ pub fn setup(subcommand: &MiriCommand, host: &str, target: &str) {
// Finally run it!
if command.status().expect("failed to run xargo").success().not() {
if only_setup {
show_error(format!("failed to run xargo, see error details above"))
show_error!("failed to run xargo, see error details above")
} else {
show_error(format!(
"failed to run xargo; run `cargo miri setup` to see the error details"
))
show_error!("failed to run xargo; run `cargo miri setup` to see the error details")
}
}

24
cargo-miri/src/util.rs
View File

@ -14,6 +14,15 @@
pub use crate::arg::*;
pub fn show_error(msg: &impl std::fmt::Display) -> ! {
eprintln!("fatal error: {msg}");
std::process::exit(1)
}
macro_rules! show_error {
($($tt:tt)*) => { crate::util::show_error(&format_args!($($tt)*)) };
}
/// The information to run a crate with the given environment.
#[derive(Clone, Serialize, Deserialize)]
pub struct CrateRunEnv {
@ -55,10 +64,10 @@ pub enum CrateRunInfo {
impl CrateRunInfo {
pub fn store(&self, filename: &Path) {
let file = File::create(filename)
.unwrap_or_else(|_| show_error(format!("cannot create `{}`", filename.display())));
.unwrap_or_else(|_| show_error!("cannot create `{}`", filename.display()));
let file = BufWriter::new(file);
serde_json::ser::to_writer(file, self)
.unwrap_or_else(|_| show_error(format!("cannot write to `{}`", filename.display())));
.unwrap_or_else(|_| show_error!("cannot write to `{}`", filename.display()));
}
}
@ -70,11 +79,6 @@ pub enum MiriCommand {
Forward(String),
}
pub fn show_error(msg: String) -> ! {
eprintln!("fatal error: {}", msg);
std::process::exit(1)
}
/// Escapes `s` in a way that is suitable for using it as a string literal in TOML syntax.
pub fn escape_for_toml(s: &str) -> String {
// We want to surround this string in quotes `"`. So we first escape all quotes,
@ -187,15 +191,15 @@ pub fn ask_to_run(mut cmd: Command, ask: bool, text: &str) {
match buf.trim().to_lowercase().as_ref() {
// Proceed.
"" | "y" | "yes" => {}
"n" | "no" => show_error(format!("aborting as per your request")),
a => show_error(format!("invalid answer `{}`", a)),
"n" | "no" => show_error!("aborting as per your request"),
a => show_error!("invalid answer `{}`", a),
};
} else {
eprintln!("Running `{:?}` to {}.", cmd, text);
}
if cmd.status().unwrap_or_else(|_| panic!("failed to execute {:?}", cmd)).success().not() {
show_error(format!("failed to {}", text));
show_error!("failed to {}", text);
}
}

58
src/bin/miri.rs
View File

@ -152,11 +152,15 @@ fn config(&mut self, config: &mut Config) {
}
}
fn show_error(msg: String) -> ! {
eprintln!("fatal error: {}", msg);
fn show_error(msg: &impl std::fmt::Display) -> ! {
eprintln!("fatal error: {msg}");
std::process::exit(1)
}
macro_rules! show_error {
($($tt:tt)*) => { show_error(&format_args!($($tt)*)) };
}
fn init_early_loggers() {
// Note that our `extern crate log` is *not* the same as rustc's; as a result, we have to
// initialize them both, and we always initialize `miri`'s first.
@ -234,19 +238,19 @@ fn host_sysroot() -> Option<String> {
env::var_os("RUSTUP_TOOLCHAIN").or_else(|| env::var_os("MULTIRUST_TOOLCHAIN"))
{
if toolchain_runtime != toolchain {
show_error(format!(
show_error!(
"This Miri got built with local toolchain `{toolchain}`, but now is being run under a different toolchain. \n\
Make sure to run Miri in the toolchain it got built with, e.g. via `cargo +{toolchain} miri`."
));
)
}
}
format!("{}/toolchains/{}", home, toolchain)
}
_ => option_env!("RUST_SYSROOT")
.unwrap_or_else(|| {
show_error(format!(
show_error!(
"To build Miri without rustup, set the `RUST_SYSROOT` env var at build time",
))
)
})
.to_owned(),
})
@ -272,9 +276,9 @@ fn run_compiler(
// Using the built-in default here would be plain wrong, so we *require*
// the env var to make sure things make sense.
Some(env::var("MIRI_SYSROOT").unwrap_or_else(|_| {
show_error(format!(
show_error!(
"Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set"
))
)
}))
} else {
host_default_sysroot
@ -379,7 +383,9 @@ fn main() {
miri_config.check_abi = false;
} else if arg == "-Zmiri-disable-isolation" {
if matches!(isolation_enabled, Some(true)) {
panic!("-Zmiri-disable-isolation cannot be used along with -Zmiri-isolation-error");
show_error!(
"-Zmiri-disable-isolation cannot be used along with -Zmiri-isolation-error"
);
} else {
isolation_enabled = Some(false);
}
@ -390,7 +396,9 @@ fn main() {
miri_config.track_outdated_loads = true;
} else if let Some(param) = arg.strip_prefix("-Zmiri-isolation-error=") {
if matches!(isolation_enabled, Some(false)) {
panic!("-Zmiri-isolation-error cannot be used along with -Zmiri-disable-isolation");
show_error!(
"-Zmiri-isolation-error cannot be used along with -Zmiri-disable-isolation"
);
} else {
isolation_enabled = Some(true);
}
@ -402,7 +410,7 @@ fn main() {
"warn-nobacktrace" =>
miri::IsolatedOp::Reject(miri::RejectOpWith::WarningWithoutBacktrace),
_ =>
panic!(
show_error!(
"-Zmiri-isolation-error must be `abort`, `hide`, `warn`, or `warn-nobacktrace`"
),
};
@ -426,11 +434,11 @@ fn main() {
);
} else if let Some(param) = arg.strip_prefix("-Zmiri-seed=") {
if miri_config.seed.is_some() {
panic!("Cannot specify -Zmiri-seed multiple times!");
show_error!("Cannot specify -Zmiri-seed multiple times!");
}
let seed = u64::from_str_radix(param, 16)
.unwrap_or_else(|_| panic!(
"-Zmiri-seed should only contain valid hex digits [0-9a-fA-F] and fit into a u64 (max 16 characters)"
.unwrap_or_else(|_| show_error!(
"-Zmiri-seed should only contain valid hex digits [0-9a-fA-F] and must fit into a u64 (max 16 characters)"
));
miri_config.seed = Some(seed);
} else if let Some(param) = arg.strip_prefix("-Zmiri-env-exclude=") {
@ -441,7 +449,7 @@ fn main() {
let ids: Vec<u64> = match parse_comma_list(param) {
Ok(ids) => ids,
Err(err) =>
panic!(
show_error!(
"-Zmiri-track-pointer-tag requires a comma separated list of valid `u64` arguments: {}",
err
),
@ -450,14 +458,14 @@ fn main() {
if let Some(id) = id {
miri_config.tracked_pointer_tags.insert(id);
} else {
panic!("-Zmiri-track-pointer-tag requires nonzero arguments");
show_error!("-Zmiri-track-pointer-tag requires nonzero arguments");
}
}
} else if let Some(param) = arg.strip_prefix("-Zmiri-track-call-id=") {
let ids: Vec<u64> = match parse_comma_list(param) {
Ok(ids) => ids,
Err(err) =>
panic!(
show_error!(
"-Zmiri-track-call-id requires a comma separated list of valid `u64` arguments: {}",
err
),
@ -466,14 +474,14 @@ fn main() {
if let Some(id) = id {
miri_config.tracked_call_ids.insert(id);
} else {
panic!("-Zmiri-track-call-id requires a nonzero argument");
show_error!("-Zmiri-track-call-id requires a nonzero argument");
}
}
} else if let Some(param) = arg.strip_prefix("-Zmiri-track-alloc-id=") {
let ids: Vec<miri::AllocId> = match parse_comma_list::<NonZeroU64>(param) {
Ok(ids) => ids.into_iter().map(miri::AllocId).collect(),
Err(err) =>
panic!(
show_error!(
"-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {}",
err
),
@ -483,11 +491,11 @@ fn main() {
let rate = match param.parse::<f64>() {
Ok(rate) if rate >= 0.0 && rate <= 1.0 => rate,
Ok(_) =>
panic!(
show_error!(
"-Zmiri-compare-exchange-weak-failure-rate must be between `0.0` and `1.0`"
),
Err(err) =>
panic!(
show_error!(
"-Zmiri-compare-exchange-weak-failure-rate requires a `f64` between `0.0` and `1.0`: {}",
err
),
@ -496,9 +504,9 @@ fn main() {
} else if let Some(param) = arg.strip_prefix("-Zmiri-preemption-rate=") {
let rate = match param.parse::<f64>() {
Ok(rate) if rate >= 0.0 && rate <= 1.0 => rate,
Ok(_) => panic!("-Zmiri-preemption-rate must be between `0.0` and `1.0`"),
Ok(_) => show_error!("-Zmiri-preemption-rate must be between `0.0` and `1.0`"),
Err(err) =>
panic!(
show_error!(
"-Zmiri-preemption-rate requires a `f64` between `0.0` and `1.0`: {}",
err
),
@ -510,7 +518,7 @@ fn main() {
} else if let Some(param) = arg.strip_prefix("-Zmiri-report-progress=") {
let interval = match param.parse::<u32>() {
Ok(i) => i,
Err(err) => panic!("-Zmiri-report-progress requires a `u32`: {}", err),
Err(err) => show_error!("-Zmiri-report-progress requires a `u32`: {}", err),
};
miri_config.report_progress = Some(interval);
} else if let Some(param) = arg.strip_prefix("-Zmiri-measureme=") {
@ -520,7 +528,7 @@ fn main() {
"0" => BacktraceStyle::Off,
"1" => BacktraceStyle::Short,
"full" => BacktraceStyle::Full,
_ => panic!("-Zmiri-backtrace may only be 0, 1, or full"),
_ => show_error!("-Zmiri-backtrace may only be 0, 1, or full"),
};
} else {
// Forward to rustc.