From 6e4530c0beae98683d900961ee92d0ce58fd6537 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 17:41:34 +0100 Subject: [PATCH] remove the ability to disable ABI checking --- README.md | 2 -- src/bin/miri.rs | 5 ++-- src/eval.rs | 3 -- src/helpers.rs | 4 +-- src/machine.rs | 9 ++---- .../concurrency/unwind_top_of_stack.rs | 29 ------------------- .../concurrency/unwind_top_of_stack.stderr | 21 -------------- .../fail/function_calls/arg_inplace_mutate.rs | 1 - .../arg_inplace_mutate.stack.stderr | 2 +- .../arg_inplace_mutate.tree.stderr | 2 +- .../exported_symbol_bad_unwind1.rs | 1 - .../exported_symbol_bad_unwind1.stderr | 2 -- tests/fail/panic/bad_miri_start_unwind.rs | 12 -------- tests/fail/panic/bad_miri_start_unwind.stderr | 17 ----------- tests/fail/panic/bad_unwind.rs | 3 ++ .../pass/function_calls/disable_abi_check.rs | 24 --------------- .../function_calls/disable_abi_check.stderr | 2 -- 17 files changed, 11 insertions(+), 128 deletions(-) delete mode 100644 tests/fail-dep/concurrency/unwind_top_of_stack.rs delete mode 100644 tests/fail-dep/concurrency/unwind_top_of_stack.stderr delete mode 100644 tests/fail/panic/bad_miri_start_unwind.rs delete mode 100644 tests/fail/panic/bad_miri_start_unwind.stderr delete mode 100644 tests/pass/function_calls/disable_abi_check.rs delete mode 100644 tests/pass/function_calls/disable_abi_check.stderr diff --git a/README.md b/README.md index 2b5ffb1a33..dd9c6097eb 100644 --- a/README.md +++ b/README.md @@ -359,8 +359,6 @@ The remaining flags are for advanced use only, and more likely to change or be r Some of these are **unsound**, which means they can lead to Miri failing to detect cases of undefined behavior in a program. -* `-Zmiri-disable-abi-check` disables checking [function ABI]. Using this flag - is **unsound**. This flag is **deprecated**. * `-Zmiri-disable-alignment-check` disables checking pointer alignment, so you can focus on other failures, but it means Miri can miss bugs in your program. Using this flag is **unsound**. diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 2f37a64576..ae9a5f7ff6 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -413,10 +413,9 @@ fn main() { ); } else if arg == "-Zmiri-disable-abi-check" { eprintln!( - "WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed.\n\ - If you have a use-case for it, please file an issue." + "WARNING: the flag `-Zmiri-disable-abi-check` no longer has any effect; \ + ABI checks cannot be disabled any more" ); - miri_config.check_abi = false; } else if arg == "-Zmiri-disable-isolation" { if matches!(isolation_enabled, Some(true)) { show_error!( diff --git a/src/eval.rs b/src/eval.rs index ef50bd43de..4e080f0cc3 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -94,8 +94,6 @@ pub struct MiriConfig { pub unique_is_unique: bool, /// Controls alignment checking. pub check_alignment: AlignmentCheck, - /// Controls function [ABI](Abi) checking. - pub check_abi: bool, /// Action for an op requiring communication with the host. pub isolated_op: IsolatedOp, /// Determines if memory leaks should be ignored. @@ -162,7 +160,6 @@ impl Default for MiriConfig { borrow_tracker: Some(BorrowTrackerMethod::StackedBorrows), unique_is_unique: false, check_alignment: AlignmentCheck::Int, - check_abi: true, isolated_op: IsolatedOp::Reject(RejectOpWith::Abort), ignore_leaks: false, forwarded_env_vars: vec![], diff --git a/src/helpers.rs b/src/helpers.rs index 65260254ae..9806884290 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -387,7 +387,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let param_env = ty::ParamEnv::reveal_all(); // in Miri this is always the param_env we use... and this.param_env is private. let callee_abi = f.ty(*this.tcx, param_env).fn_sig(*this.tcx).abi(); - if this.machine.enforce_abi && callee_abi != caller_abi { + if callee_abi != caller_abi { throw_ub_format!( "calling a function with ABI {} using caller ABI {}", callee_abi.name(), @@ -945,7 +945,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Check that the ABI is what we expect. fn check_abi<'a>(&self, abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> { - if self.eval_context_ref().machine.enforce_abi && abi != exp_abi { + if abi != exp_abi { throw_ub_format!( "calling a function with ABI {} using caller ABI {}", exp_abi.name(), diff --git a/src/machine.rs b/src/machine.rs index c411962fcf..f6f57dcd59 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -460,9 +460,6 @@ pub struct MiriMachine<'mir, 'tcx> { /// Whether to enforce the validity invariant. pub(crate) validate: bool, - /// Whether to enforce [ABI](Abi) of function calls. - pub(crate) enforce_abi: bool, - /// The table of file descriptors. pub(crate) file_handler: shims::unix::FileHandler, /// The table of directory descriptors. @@ -641,7 +638,6 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { tls: TlsData::default(), isolated_op: config.isolated_op, validate: config.validate, - enforce_abi: config.check_abi, file_handler: FileHandler::new(config.mute_stdout_stderr), dir_handler: Default::default(), layouts, @@ -784,7 +780,6 @@ impl VisitProvenance for MiriMachine<'_, '_> { tcx: _, isolated_op: _, validate: _, - enforce_abi: _, clock: _, layouts: _, static_roots: _, @@ -932,8 +927,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } #[inline(always)] - fn enforce_abi(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { - ecx.machine.enforce_abi + fn enforce_abi(_ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { + true } #[inline(always)] diff --git a/tests/fail-dep/concurrency/unwind_top_of_stack.rs b/tests/fail-dep/concurrency/unwind_top_of_stack.rs deleted file mode 100644 index 4704cfed03..0000000000 --- a/tests/fail-dep/concurrency/unwind_top_of_stack.rs +++ /dev/null @@ -1,29 +0,0 @@ -//@ignore-target-windows: No libc on Windows - -//@compile-flags: -Zmiri-disable-abi-check - -//! Unwinding past the top frame of a stack is Undefined Behavior. - -#![feature(c_unwind)] - -use std::{mem, ptr}; - -extern "C-unwind" fn thread_start(_null: *mut libc::c_void) -> *mut libc::c_void { - //~^ ERROR: unwinding past the topmost frame of the stack - panic!() -} - -fn main() { - unsafe { - let mut native: libc::pthread_t = mem::zeroed(); - let attr: libc::pthread_attr_t = mem::zeroed(); - // assert_eq!(libc::pthread_attr_init(&mut attr), 0); FIXME: this function is not yet implemented. - // Cast to avoid inserting abort-on-unwind. - let thread_start: extern "C-unwind" fn(*mut libc::c_void) -> *mut libc::c_void = - thread_start; - let thread_start: extern "C" fn(*mut libc::c_void) -> *mut libc::c_void = - mem::transmute(thread_start); - assert_eq!(libc::pthread_create(&mut native, &attr, thread_start, ptr::null_mut()), 0); - assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0); - } -} diff --git a/tests/fail-dep/concurrency/unwind_top_of_stack.stderr b/tests/fail-dep/concurrency/unwind_top_of_stack.stderr deleted file mode 100644 index 0c755215c0..0000000000 --- a/tests/fail-dep/concurrency/unwind_top_of_stack.stderr +++ /dev/null @@ -1,21 +0,0 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. -thread '' panicked at $DIR/unwind_top_of_stack.rs:LL:CC: -explicit panic -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -error: Undefined Behavior: unwinding past the topmost frame of the stack - --> $DIR/unwind_top_of_stack.rs:LL:CC - | -LL | / extern "C-unwind" fn thread_start(_null: *mut libc::c_void) -> *mut libc::c_void { -LL | | -LL | | panic!() -LL | | } - | |_^ unwinding past the topmost frame of the stack - | - = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior - = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - = note: BACKTRACE on thread `unnamed-ID`: - = note: inside `thread_start` at $DIR/unwind_top_of_stack.rs:LL:CC - -error: aborting due to 1 previous error - diff --git a/tests/fail/function_calls/arg_inplace_mutate.rs b/tests/fail/function_calls/arg_inplace_mutate.rs index 8a5c10913b..4f7a12ebd2 100644 --- a/tests/fail/function_calls/arg_inplace_mutate.rs +++ b/tests/fail/function_calls/arg_inplace_mutate.rs @@ -19,7 +19,6 @@ fn main() { after_call = { Return() } - } } diff --git a/tests/fail/function_calls/arg_inplace_mutate.stack.stderr b/tests/fail/function_calls/arg_inplace_mutate.stack.stderr index 422dc24343..e0d1bed633 100644 --- a/tests/fail/function_calls/arg_inplace_mutate.stack.stderr +++ b/tests/fail/function_calls/arg_inplace_mutate.stack.stderr @@ -14,7 +14,7 @@ LL | | let _unit: (); LL | | { LL | | let non_copy = S(42); ... | -LL | | +LL | | } LL | | } | |_____^ help: is this argument diff --git a/tests/fail/function_calls/arg_inplace_mutate.tree.stderr b/tests/fail/function_calls/arg_inplace_mutate.tree.stderr index 4fe9b7b472..21c814e664 100644 --- a/tests/fail/function_calls/arg_inplace_mutate.tree.stderr +++ b/tests/fail/function_calls/arg_inplace_mutate.tree.stderr @@ -16,7 +16,7 @@ LL | | let _unit: (); LL | | { LL | | let non_copy = S(42); ... | -LL | | +LL | | } LL | | } | |_____^ help: the protected tag was created here, in the initial state Reserved diff --git a/tests/fail/function_calls/exported_symbol_bad_unwind1.rs b/tests/fail/function_calls/exported_symbol_bad_unwind1.rs index 5f4df7c6a1..6d68b9a46d 100644 --- a/tests/fail/function_calls/exported_symbol_bad_unwind1.rs +++ b/tests/fail/function_calls/exported_symbol_bad_unwind1.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-disable-abi-check #![feature(c_unwind)] #[no_mangle] diff --git a/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr b/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr index c5ca127457..ab32883a03 100644 --- a/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr +++ b/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr @@ -1,5 +1,3 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. thread 'main' panicked at $DIR/exported_symbol_bad_unwind1.rs:LL:CC: explicit panic note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/tests/fail/panic/bad_miri_start_unwind.rs b/tests/fail/panic/bad_miri_start_unwind.rs deleted file mode 100644 index deca836a36..0000000000 --- a/tests/fail/panic/bad_miri_start_unwind.rs +++ /dev/null @@ -1,12 +0,0 @@ -//@compile-flags: -Zmiri-disable-abi-check -// This feature is required to trigger the error using the "C" ABI. -#![feature(c_unwind)] - -extern "C" { - fn miri_start_unwind(payload: *mut u8) -> !; -} - -fn main() { - unsafe { miri_start_unwind(&mut 0) } - //~^ ERROR: unwinding past a stack frame that does not allow unwinding -} diff --git a/tests/fail/panic/bad_miri_start_unwind.stderr b/tests/fail/panic/bad_miri_start_unwind.stderr deleted file mode 100644 index 6c85aac050..0000000000 --- a/tests/fail/panic/bad_miri_start_unwind.stderr +++ /dev/null @@ -1,17 +0,0 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. -error: Undefined Behavior: unwinding past a stack frame that does not allow unwinding - --> $DIR/bad_miri_start_unwind.rs:LL:CC - | -LL | unsafe { miri_start_unwind(&mut 0) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding - | - = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior - = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - = note: BACKTRACE: - = note: inside `main` at $DIR/bad_miri_start_unwind.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to 1 previous error - diff --git a/tests/fail/panic/bad_unwind.rs b/tests/fail/panic/bad_unwind.rs index 370b372a7d..8c8a9f18cd 100644 --- a/tests/fail/panic/bad_unwind.rs +++ b/tests/fail/panic/bad_unwind.rs @@ -1,6 +1,9 @@ #![feature(c_unwind)] //! Unwinding when the caller ABI is "C" (without "-unwind") is UB. +// The opposite version (callee does not allow unwinding) is impossible to +// even write: MIR validation catches functions that have `UnwindContinue` but +// are not allowed to unwind. extern "C-unwind" fn unwind() { panic!(); diff --git a/tests/pass/function_calls/disable_abi_check.rs b/tests/pass/function_calls/disable_abi_check.rs deleted file mode 100644 index 0f41047c60..0000000000 --- a/tests/pass/function_calls/disable_abi_check.rs +++ /dev/null @@ -1,24 +0,0 @@ -//@compile-flags: -Zmiri-disable-abi-check -#![feature(core_intrinsics)] - -fn main() { - fn foo() {} - - extern "C" fn try_fn(ptr: *mut u8) { - assert!(ptr.is_null()); - } - - extern "Rust" { - fn malloc(size: usize) -> *mut std::ffi::c_void; - } - - unsafe { - let _ = malloc(0); - std::mem::transmute::(foo)(); - std::intrinsics::catch_unwind( - std::mem::transmute::(try_fn), - std::ptr::null_mut(), - |_, _| unreachable!(), - ); - } -} diff --git a/tests/pass/function_calls/disable_abi_check.stderr b/tests/pass/function_calls/disable_abi_check.stderr deleted file mode 100644 index e5b84f6d70..0000000000 --- a/tests/pass/function_calls/disable_abi_check.stderr +++ /dev/null @@ -1,2 +0,0 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue.