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

support panic=abort #1549

Merged
merged 2 commits into from
Sep 18, 2020
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ 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.
When set while running `cargo-miri`, it indicates that we are part of a sysroot
build (for which some crates need special treatment).
* `MIRI_CWD` when set to any value tells the Miri driver to change to the given
directory after loading all the source files, but before commencing
interpretation. This is useful if the interpreted program wants a different
Expand Down
48 changes: 36 additions & 12 deletions cargo-miri/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,7 @@ fn setup(subcommand: MiriCommand) {
br#"
[dependencies.std]
default_features = false
# We need the `panic_unwind` feature because we use the `unwind` panic strategy.
# Using `abort` works for libstd, but then libtest will not compile.
# We support unwinding, so enable that panic runtime.
features = ["panic_unwind"]

[dependencies.test]
Expand Down Expand Up @@ -338,10 +337,14 @@ path = "lib.rs"
// because we still need bootstrap to distinguish between host and target crates.
// In that case we overwrite `RUSTC_REAL` instead which determines the rustc used
// for target crates.
// We set ourselves (`cargo-miri`) instead of Miri directly to be able to patch the flags
// for `libpanic_abort` (usually this is done by bootstrap but we have to do it ourselves).
// The `MIRI_BE_RUSTC` will mean we dispatch to `phase_setup_rustc`.
let cargo_miri_path = std::env::current_exe().expect("current executable path invalid");
if env::var_os("RUSTC_STAGE").is_some() {
command.env("RUSTC_REAL", find_miri());
command.env("RUSTC_REAL", &cargo_miri_path);
} else {
command.env("RUSTC", find_miri());
command.env("RUSTC", &cargo_miri_path);
}
command.env("MIRI_BE_RUSTC", "1");
// Make sure there are no other wrappers or flags getting in our way
Expand Down Expand Up @@ -370,6 +373,21 @@ path = "lib.rs"
}
}

fn phase_setup_rustc(args: env::Args) {
// Mostly we just forward everything.
// `MIRI_BE_RUST` is already set.
let mut cmd = miri();
cmd.args(args);

// Patch the panic runtime for `libpanic_abort` (mirroring what bootstrap usually does).
if get_arg_flag_value("--crate-name").as_deref() == Some("panic_abort") {
cmd.arg("-C").arg("panic=abort");
}

// Run it!
exec(cmd);
}

fn phase_cargo_miri(mut args: env::Args) {
// Check for version and help flags even when invoked as `cargo-miri`.
if has_arg_flag("--help") || has_arg_flag("-h") {
Expand Down Expand Up @@ -402,7 +420,7 @@ fn phase_cargo_miri(mut args: env::Args) {
// <https://github.com/rust-lang/miri/pull/1540#issuecomment-693553191> describes an alternative
// approach that uses `cargo check`, making that part easier but target and binary handling
// harder.
let miri_path = std::env::current_exe().expect("current executable path invalid");
let cargo_miri_path = std::env::current_exe().expect("current executable path invalid");
let cargo_cmd = match subcommand {
MiriCommand::Test => "test",
MiriCommand::Run => "run",
Expand Down Expand Up @@ -470,22 +488,22 @@ fn phase_cargo_miri(mut args: env::Args) {
if env::var_os("RUSTC_WRAPPER").is_some() {
println!("WARNING: Ignoring `RUSTC_WRAPPER` environment variable, Miri does not support wrapping.");
}
cmd.env("RUSTC_WRAPPER", &miri_path);
if verbose {
eprintln!("+ RUSTC_WRAPPER={:?}", miri_path);
}
cmd.env("RUSTC_WRAPPER", &cargo_miri_path);

// Set the runner for the current target to us as well, so we can interpret the binaries.
let runner_env_name = format!("CARGO_TARGET_{}_RUNNER", target.to_uppercase().replace('-', "_"));
cmd.env(runner_env_name, &miri_path);
cmd.env(&runner_env_name, &cargo_miri_path);

// Set rustdoc to us as well, so we can make it do nothing (see issue #584).
cmd.env("RUSTDOC", &miri_path);
cmd.env("RUSTDOC", &cargo_miri_path);

// Run cargo.
if verbose {
cmd.env("MIRI_VERBOSE", ""); // This makes the other phases verbose.
eprintln!("[cargo-miri miri] RUSTC_WRAPPER={:?}", cargo_miri_path);
eprintln!("[cargo-miri miri] {}={:?}", runner_env_name, cargo_miri_path);
eprintln!("[cargo-miri miri] RUSTDOC={:?}", cargo_miri_path);
eprintln!("[cargo-miri miri] {:?}", cmd);
cmd.env("MIRI_VERBOSE", ""); // This makes the other phases verbose.
}
exec(cmd)
}
Expand Down Expand Up @@ -699,6 +717,12 @@ fn main() {
// Skip binary name.
args.next().unwrap();

// Dispatch running as part of sysroot compilation.
if env::var_os("MIRI_BE_RUSTC").is_some() {
phase_setup_rustc(args);
return;
}

// Dispatch to `cargo-miri` phase. There are three phases:
// - When we are called via `cargo miri`, we run as the frontend and invoke the underlying
// cargo. We set RUSTC_WRAPPER and CARGO_TARGET_RUNNER to ourselves.
Expand Down
20 changes: 11 additions & 9 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use log::trace;

use rustc_hir::def_id::DefId;
use rustc_middle::{mir, ty};
use rustc_target::abi::{Align, Size};
use rustc_target::{abi::{Align, Size}, spec::PanicStrategy};
use rustc_apfloat::Float;
use rustc_span::symbol::sym;

Expand Down Expand Up @@ -146,6 +146,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let code = this.read_scalar(code)?.to_i32()?;
throw_machine_stop!(TerminationInfo::Exit(code.into()));
}
"abort" => {
throw_machine_stop!(TerminationInfo::Abort(None))
}
_ => throw_unsup_format!("can't call (diverging) foreign function: {}", link_name),
},
Some(p) => p,
Expand All @@ -159,14 +162,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// We forward this to the underlying *implementation* in the panic runtime crate.
// Normally, this will be either `libpanic_unwind` or `libpanic_abort`, but it could
// also be a custom user-provided implementation via `#![feature(panic_runtime)]`
"__rust_start_panic" | "__rust_panic_cleanup"=> {
// FIXME we might want to cache this... but it's not really performance-critical.
let panic_runtime = tcx
.crates()
.iter()
.find(|cnum| tcx.is_panic_runtime(**cnum))
.expect("No panic runtime found!");
let panic_runtime = tcx.crate_name(*panic_runtime);
"__rust_start_panic" | "__rust_panic_cleanup" => {
// This replicates some of the logic in `inject_panic_runtime`.
// FIXME: is there a way to reuse that logic?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could have a start_panic_instance function on Session in rustc

Copy link
Member Author

@RalfJung RalfJung Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe we could somehow figure out "which crate that was added as a dependency is the panic runtime". I thought that's what the previous code was doing, but it still saw the panic_unwind crate even with -C panic=abort so clearly that was not the right approach.

let panic_runtime = match this.tcx.sess.panic_strategy() {
PanicStrategy::Unwind => sym::panic_unwind,
PanicStrategy::Abort => sym::panic_abort,
};
let start_panic_instance =
this.resolve_path(&[&*panic_runtime.as_str(), link_name]);
return Ok(Some(&*this.load_mir(start_panic_instance.def, None)?));
Expand Down
2 changes: 2 additions & 0 deletions src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let this = self.eval_context_mut();

trace!("miri_start_panic: {:?}", this.frame().instance);
// Make sure we only start unwinding when this matches our panic strategy.
assert_eq!(this.tcx.sess.panic_strategy(), PanicStrategy::Unwind);

// Get the raw pointer stored in arg[0] (the panic payload).
let &[payload] = check_arg_count(args)?;
Expand Down
5 changes: 3 additions & 2 deletions tests/compile-fail/panic/panic_abort1.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// ignore-test: Abort panics are not yet supported
// error-pattern: the evaluated program panicked
// error-pattern: the evaluated program aborted execution
// compile-flags: -C panic=abort
// ignore-windows: windows panics via inline assembly (FIXME)

fn main() {
std::panic!("panicking from libstd");
}
4 changes: 2 additions & 2 deletions tests/compile-fail/panic/panic_abort2.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// ignore-test: Abort panics are not yet supported
// error-pattern: the evaluated program panicked
// error-pattern: the evaluated program aborted execution
// compile-flags: -C panic=abort
// ignore-windows: windows panics via inline assembly (FIXME)

fn main() {
std::panic!("{}-panicking from libstd", 42);
Expand Down
4 changes: 2 additions & 2 deletions tests/compile-fail/panic/panic_abort3.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// ignore-test: Abort panics are not yet supported
//error-pattern: the evaluated program panicked
// error-pattern: the evaluated program aborted execution
// compile-flags: -C panic=abort
// ignore-windows: windows panics via inline assembly (FIXME)

fn main() {
core::panic!("panicking from libcore");
Expand Down
4 changes: 2 additions & 2 deletions tests/compile-fail/panic/panic_abort4.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// ignore-test: Abort panics are not yet supported
//error-pattern: the evaluated program panicked
// error-pattern: the evaluated program aborted execution
// compile-flags: -C panic=abort
// ignore-windows: windows panics via inline assembly (FIXME)

fn main() {
core::panic!("{}-panicking from libcore", 42);
Expand Down