Skip to content

Commit

Permalink
Auto merge of #1241 - RalfJung:dont-panic, r=RalfJung
Browse files Browse the repository at this point in the history
whitelist platforms where panicking should work

@CAD97 [proposed](#1059 (comment)) trying to get a better error for failed panics on Windows.

Could you test if this works for you?
  • Loading branch information
bors committed Mar 21, 2020
2 parents ee71b2e + bde3111 commit ff9f866
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// This matches calls to the foreign item `panic_impl`.
// The implementation is provided by the function with the `#[panic_handler]` attribute.
"panic_impl" => {
this.check_panic_supported()?;
let panic_impl_id = this.tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(*this.tcx, panic_impl_id);
return Ok(Some(&*this.load_mir(panic_impl_instance.def, None)?));
Expand Down
8 changes: 8 additions & 0 deletions src/shims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
return this.emulate_foreign_item(instance.def_id(), args, ret, unwind);
}

// Better error message for panics on Windows.
let def_id = instance.def_id();
if Some(def_id) == this.tcx.lang_items().begin_panic_fn() ||
Some(def_id) == this.tcx.lang_items().panic_impl()
{
this.check_panic_supported()?;
}

// Otherwise, load the MIR.
Ok(Some(&*this.load_mir(instance.def, None)?))
}
Expand Down
8 changes: 8 additions & 0 deletions src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ pub struct CatchUnwindData<'tcx> {

impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
/// Check if panicking is supported on this platform, and give a good error otherwise.
fn check_panic_supported(&self) -> InterpResult<'tcx> {
match self.eval_context_ref().tcx.sess.target.target.target_os.as_str() {
"linux" | "macos" => Ok(()),
_ => throw_unsup_format!("panicking is not supported on this platform"),
}
}

/// Handles the special `miri_start_panic` intrinsic, which is called
/// by libpanic_unwind to delegate the actual unwinding process to Miri.
fn handle_miri_start_panic(
Expand Down
9 changes: 9 additions & 0 deletions tests/compile-fail/panic/windows1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// ignore-linux
// ignore-macos

// Test that panics on Windows give a reasonable error message.

// error-pattern: panicking is not supported on this platform
fn main() {
core::panic!("this is {}", "Windows");
}
9 changes: 9 additions & 0 deletions tests/compile-fail/panic/windows2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// ignore-linux
// ignore-macos

// Test that panics on Windows give a reasonable error message.

// error-pattern: panicking is not supported on this platform
fn main() {
std::panic!("this is Windows");
}
10 changes: 10 additions & 0 deletions tests/compile-fail/panic/windows3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// ignore-linux
// ignore-macos

// Test that panics on Windows give a reasonable error message.

// error-pattern: panicking is not supported on this platform
#[allow(unconditional_panic)]
fn main() {
let _val = 1/0;
}

0 comments on commit ff9f866

Please sign in to comment.