Auto merge of #1549 - RalfJung:panic-abort, r=oli-obk

support panic=abort

This adds support for abort-on-panic (#1058). To achieve this, we insert `cargo-miri` as `RUSTC` when building the standard library, and patch the rustc flags in a way similar to what bootstrap does.

However, this is currently not supported on Windows as the Windows code uses inline assembly to cause an abort (?!?). I'll submit a rustc PR with some `cffg(miri)` to make that work. (EDIT: that would be rust-lang/rust#76871)

Cc `@Aaron1011` r? `@oli-obk`

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

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) {
     // <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",
@@ -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.

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,
@@ -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?
+                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)?));

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)?;

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");
 }

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);

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");

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);