From 602efddb0a40ef224f8bab3eecbe6500c9be6478 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 8 Oct 2024 08:46:14 +0200 Subject: [PATCH 1/2] remove handle_unsupported_foreign_item helper --- src/helpers.rs | 16 ---------------- src/shims/foreign_items.rs | 12 +++++++++--- src/shims/unix/linux/foreign_items.rs | 5 +---- tests/panic/unsupported_syscall.rs | 9 --------- tests/panic/unsupported_syscall.stderr | 4 ---- 5 files changed, 10 insertions(+), 36 deletions(-) delete mode 100644 tests/panic/unsupported_syscall.rs delete mode 100644 tests/panic/unsupported_syscall.stderr diff --git a/src/helpers.rs b/src/helpers.rs index 013bfe03aa..8bd429deef 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -14,7 +14,6 @@ use rustc_index::IndexVec; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::dependency_format::Linkage; use rustc_middle::middle::exported_symbols::ExportedSymbol; -use rustc_middle::mir; use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, MaybeResult, TyAndLayout}; use rustc_middle::ty::{self, FloatTy, IntTy, Ty, TyCtxt, UintTy}; use rustc_session::config::CrateType; @@ -949,21 +948,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { crate_name == "std" || crate_name == "std_miri_test" } - /// Handler that should be called when an unsupported foreign item is encountered. - /// This function will either panic within the context of the emulated application - /// or return an error in the Miri process context - fn handle_unsupported_foreign_item(&mut self, error_msg: String) -> InterpResult<'tcx, ()> { - let this = self.eval_context_mut(); - if this.machine.panic_on_unsupported { - // message is slightly different here to make automated analysis easier - let error_msg = format!("unsupported Miri functionality: {error_msg}"); - this.start_panic(error_msg.as_ref(), mir::UnwindAction::Continue)?; - interp_ok(()) - } else { - throw_machine_stop!(TerminationInfo::UnsupportedForeignItem(error_msg)); - } - } - fn check_abi_and_shim_symbol_clash( &mut self, abi: Abi, diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 78b07f68b4..1a6c5e9112 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -83,11 +83,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return interp_ok(Some(body)); } - this.handle_unsupported_foreign_item(format!( + let error_msg = format!( "can't call foreign function `{link_name}` on OS `{os}`", os = this.tcx.sess.target.os, - ))?; - return interp_ok(None); + ); + if this.machine.panic_on_unsupported { + // message is slightly different here to make automated analysis easier + let error_msg = format!("unsupported Miri functionality: {error_msg}"); + this.start_panic(error_msg.as_ref(), mir::UnwindAction::Continue)?; + } else { + throw_machine_stop!(TerminationInfo::UnsupportedForeignItem(error_msg)); + } } } diff --git a/src/shims/unix/linux/foreign_items.rs b/src/shims/unix/linux/foreign_items.rs index 2a72004378..2f3e1f29dd 100644 --- a/src/shims/unix/linux/foreign_items.rs +++ b/src/shims/unix/linux/foreign_items.rs @@ -168,10 +168,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_int(result.to_i32()?, dest)?; } id => { - this.handle_unsupported_foreign_item(format!( - "can't execute syscall with ID {id}" - ))?; - return interp_ok(EmulateItemResult::AlreadyJumped); + throw_unsup_format!("can't execute syscall with ID {id}"); } } } diff --git a/tests/panic/unsupported_syscall.rs b/tests/panic/unsupported_syscall.rs deleted file mode 100644 index bbb076b169..0000000000 --- a/tests/panic/unsupported_syscall.rs +++ /dev/null @@ -1,9 +0,0 @@ -//@ignore-target: windows # no `syscall` on Windows -//@ignore-target: apple # `syscall` is not supported on macOS -//@compile-flags: -Zmiri-panic-on-unsupported - -fn main() { - unsafe { - libc::syscall(0); - } -} diff --git a/tests/panic/unsupported_syscall.stderr b/tests/panic/unsupported_syscall.stderr deleted file mode 100644 index e9b2b5b665..0000000000 --- a/tests/panic/unsupported_syscall.stderr +++ /dev/null @@ -1,4 +0,0 @@ -thread 'main' panicked at tests/panic/unsupported_syscall.rs:LL:CC: -unsupported Miri functionality: can't execute syscall with ID 0 -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect From 2b3deec2e606f080f1ecff90c6bf049b29716408 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 10 Oct 2024 08:32:17 +0200 Subject: [PATCH 2/2] remove -Zmiri-panic-on-unsupported flag --- README.md | 5 ----- src/bin/miri.rs | 2 -- src/eval.rs | 3 --- src/machine.rs | 7 ------- src/shims/foreign_items.rs | 11 ++--------- tests/panic/unsupported_foreign_function.rs | 12 ------------ tests/panic/unsupported_foreign_function.stderr | 4 ---- 7 files changed, 2 insertions(+), 42 deletions(-) delete mode 100644 tests/panic/unsupported_foreign_function.rs delete mode 100644 tests/panic/unsupported_foreign_function.stderr diff --git a/README.md b/README.md index 9a50079bc9..a73fefaaf3 100644 --- a/README.md +++ b/README.md @@ -392,11 +392,6 @@ to Miri failing to detect cases of undefined behavior in a program. but reports to the program that it did actually write. This is useful when you are not interested in the actual program's output, but only want to see Miri's errors and warnings. -* `-Zmiri-panic-on-unsupported` will make some forms of unsupported functionality, - such as FFI and unsupported syscalls, panic within the context of the emulated - application instead of raising an error within the context of Miri (and halting - execution). Note that code might not expect these operations to ever panic, so - this flag can lead to strange (mis)behavior. * `-Zmiri-recursive-validation` is a *highly experimental* flag that makes validity checking recurse below references. * `-Zmiri-retag-fields[=]` controls when Stacked Borrows retagging recurses into diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 8d3ae97e0e..717229ba8b 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -530,8 +530,6 @@ fn main() { } else if arg == "-Zmiri-ignore-leaks" { miri_config.ignore_leaks = true; miri_config.collect_leak_backtraces = false; - } else if arg == "-Zmiri-panic-on-unsupported" { - miri_config.panic_on_unsupported = true; } else if arg == "-Zmiri-strict-provenance" { miri_config.provenance_mode = ProvenanceMode::Strict; } else if arg == "-Zmiri-permissive-provenance" { diff --git a/src/eval.rs b/src/eval.rs index ece76e581f..75dce211dd 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -129,8 +129,6 @@ pub struct MiriConfig { /// If `Some`, enable the `measureme` profiler, writing results to a file /// with the specified prefix. pub measureme_out: Option, - /// Panic when unsupported functionality is encountered. - pub panic_on_unsupported: bool, /// Which style to use for printing backtraces. pub backtrace_style: BacktraceStyle, /// Which provenance to use for int2ptr casts @@ -183,7 +181,6 @@ impl Default for MiriConfig { track_outdated_loads: false, cmpxchg_weak_failure_rate: 0.8, // 80% measureme_out: None, - panic_on_unsupported: false, backtrace_style: BacktraceStyle::Short, provenance_mode: ProvenanceMode::Default, mute_stdout_stderr: false, diff --git a/src/machine.rs b/src/machine.rs index b9cebcfe9c..0d28a4ed3e 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -496,11 +496,6 @@ pub struct MiriMachine<'tcx> { /// `None` means no `Instance` exported under the given name is found. pub(crate) exported_symbols_cache: FxHashMap>>, - /// Whether to raise a panic in the context of the evaluated process when unsupported - /// functionality is encountered. If `false`, an error is propagated in the Miri application context - /// instead (default behavior) - pub(crate) panic_on_unsupported: bool, - /// Equivalent setting as RUST_BACKTRACE on encountering an error. pub(crate) backtrace_style: BacktraceStyle, @@ -667,7 +662,6 @@ impl<'tcx> MiriMachine<'tcx> { profiler, string_cache: Default::default(), exported_symbols_cache: FxHashMap::default(), - panic_on_unsupported: config.panic_on_unsupported, backtrace_style: config.backtrace_style, local_crates, extern_statics: FxHashMap::default(), @@ -807,7 +801,6 @@ impl VisitProvenance for MiriMachine<'_> { profiler: _, string_cache: _, exported_symbols_cache: _, - panic_on_unsupported: _, backtrace_style: _, local_crates: _, rng: _, diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 1a6c5e9112..7fce5b6330 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -83,17 +83,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return interp_ok(Some(body)); } - let error_msg = format!( + throw_machine_stop!(TerminationInfo::UnsupportedForeignItem(format!( "can't call foreign function `{link_name}` on OS `{os}`", os = this.tcx.sess.target.os, - ); - if this.machine.panic_on_unsupported { - // message is slightly different here to make automated analysis easier - let error_msg = format!("unsupported Miri functionality: {error_msg}"); - this.start_panic(error_msg.as_ref(), mir::UnwindAction::Continue)?; - } else { - throw_machine_stop!(TerminationInfo::UnsupportedForeignItem(error_msg)); - } + ))); } } diff --git a/tests/panic/unsupported_foreign_function.rs b/tests/panic/unsupported_foreign_function.rs deleted file mode 100644 index b8301c5077..0000000000 --- a/tests/panic/unsupported_foreign_function.rs +++ /dev/null @@ -1,12 +0,0 @@ -//@compile-flags: -Zmiri-panic-on-unsupported -//@normalize-stderr-test: "OS `.*`" -> "$$OS" - -fn main() { - extern "Rust" { - fn foo(); - } - - unsafe { - foo(); - } -} diff --git a/tests/panic/unsupported_foreign_function.stderr b/tests/panic/unsupported_foreign_function.stderr deleted file mode 100644 index 278af9612d..0000000000 --- a/tests/panic/unsupported_foreign_function.stderr +++ /dev/null @@ -1,4 +0,0 @@ -thread 'main' panicked at tests/panic/unsupported_foreign_function.rs:LL:CC: -unsupported Miri functionality: can't call foreign function `foo` on $OS -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect