From 32cdb7131b275c00f6a711e2c0f71d5ae76b67d5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 18 Sep 2020 13:10:18 +0200 Subject: [PATCH 1/2] support panic=abort --- README.md | 2 + cargo-miri/bin.rs | 48 ++++++++++++++++++------ src/shims/foreign_items.rs | 18 +++++---- src/shims/panic.rs | 2 + tests/compile-fail/panic/panic_abort1.rs | 5 ++- tests/compile-fail/panic/panic_abort2.rs | 4 +- tests/compile-fail/panic/panic_abort3.rs | 4 +- tests/compile-fail/panic/panic_abort4.rs | 4 +- 8 files changed, 59 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 6654de10ab..22823f5bf8 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/cargo-miri/bin.rs b/cargo-miri/bin.rs index 522570626d..2eefc105ab 100644 --- a/cargo-miri/bin.rs +++ b/cargo-miri/bin.rs @@ -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] @@ -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 @@ -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") { @@ -402,7 +420,7 @@ fn phase_cargo_miri(mut args: env::Args) { // 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", @@ -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) } @@ -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. diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 355801eb8f..a2ff39d0b4 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -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; @@ -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, @@ -160,13 +163,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // 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); + // This replicates some of the logic in `inject_panic_runtime`. + // FIXME: is there a way to reuse that logic? + 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)?)); diff --git a/src/shims/panic.rs b/src/shims/panic.rs index f907b76b67..52d27a1bb5 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -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)?; diff --git a/tests/compile-fail/panic/panic_abort1.rs b/tests/compile-fail/panic/panic_abort1.rs index 4a1bb11483..ee1d5b312d 100644 --- a/tests/compile-fail/panic/panic_abort1.rs +++ b/tests/compile-fail/panic/panic_abort1.rs @@ -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"); } diff --git a/tests/compile-fail/panic/panic_abort2.rs b/tests/compile-fail/panic/panic_abort2.rs index ce4471c0ef..4c08ab4ddc 100644 --- a/tests/compile-fail/panic/panic_abort2.rs +++ b/tests/compile-fail/panic/panic_abort2.rs @@ -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); diff --git a/tests/compile-fail/panic/panic_abort3.rs b/tests/compile-fail/panic/panic_abort3.rs index 842a0f5435..81a603d5e3 100644 --- a/tests/compile-fail/panic/panic_abort3.rs +++ b/tests/compile-fail/panic/panic_abort3.rs @@ -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"); diff --git a/tests/compile-fail/panic/panic_abort4.rs b/tests/compile-fail/panic/panic_abort4.rs index 816cc90cfa..d015316ef2 100644 --- a/tests/compile-fail/panic/panic_abort4.rs +++ b/tests/compile-fail/panic/panic_abort4.rs @@ -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); From 97a71c0c777fa5873aca866a9a8f4adfab48feef Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 18 Sep 2020 13:34:25 +0200 Subject: [PATCH 2/2] fmt Co-authored-by: Oli Scherer --- src/shims/foreign_items.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index a2ff39d0b4..5bcbd797ca 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -162,7 +162,7 @@ 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"=> { + "__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? let panic_runtime = match this.tcx.sess.panic_strategy() {