diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 27d2f4a6df304..a61a467ebce1c 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -73,7 +73,10 @@ jobs: sudo bash -c "echo 'https://ports.ubuntu.com/ priority:4' >> /etc/apt/apt-mirrors.txt" # Add architecture sudo dpkg --add-architecture ${{ matrix.multiarch }} - sudo apt update + # Ubuntu Ports often has outdated mirrors so try a few times to get the apt repo + for TRY in $(seq 3); do + { sudo apt update && break; } || sleep 30 + done # Install needed packages sudo apt install $(echo "libatomic1: zlib1g-dev:" | sed 's/:/:${{ matrix.multiarch }}/g') - uses: ./.github/workflows/setup diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index d66529a464b69..a2b02f286d659 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -1519,9 +1519,9 @@ dependencies = [ [[package]] name = "tikv-jemalloc-sys" -version = "0.6.0+5.3.0-1-ge13ca993e8ccb9ba9847cc330696e02839f328f7" +version = "0.6.1+5.3.0-1-ge13ca993e8ccb9ba9847cc330696e02839f328f7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd3c60906412afa9c2b5b5a48ca6a5abe5736aec9eb48ad05037a677e52e4e2d" +checksum = "cd8aa5b2ab86a2cefa406d889139c162cbb230092f7d1d7cbc1716405d852a3b" dependencies = [ "cc", "libc", diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index f6c675839e01f..b89d664963c9b 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -464,11 +464,6 @@ to Miri failing to detect cases of undefined behavior in a program. errors and warnings. * `-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 - fields. `all` means it always recurses (the default, and equivalent to `-Zmiri-retag-fields` - without an explicit value), `none` means it never recurses, `scalar` means it only recurses for - types where we would also emit `noalias` annotations in the generated LLVM IR (types passed as - individual scalars or pairs of scalars). Setting this to `none` is **unsound**. * `-Zmiri-preemption-rate` configures the probability that at the end of a basic block, the active thread will be preempted. The default is `0.01` (i.e., 1%). Setting this to `0` disables preemption. Note that even without preemption, the schedule is still non-deterministic: diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 04d41c96f5c08..06a5970646243 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -8401398e1f14a24670ee1a3203713dc2f0f8b3a8 +7a72c5459dd58f81b0e1a0e5436d145485889375 diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 920fc29481916..9ce2aab8658f2 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -23,18 +23,18 @@ extern crate rustc_span; mod log; use std::env; -use std::num::NonZero; +use std::num::{NonZero, NonZeroI32}; use std::ops::Range; use std::rc::Rc; use std::str::FromStr; -use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; +use std::sync::atomic::{AtomicU32, Ordering}; use miri::{ BacktraceStyle, BorrowTrackerMethod, GenmcConfig, GenmcCtx, MiriConfig, MiriEntryFnType, - ProvenanceMode, RetagFields, TreeBorrowsParams, ValidationMode, run_genmc_mode, + ProvenanceMode, TreeBorrowsParams, ValidationMode, run_genmc_mode, }; use rustc_abi::ExternAbi; -use rustc_data_structures::sync; +use rustc_data_structures::sync::{self, DynSync}; use rustc_driver::Compilation; use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::{self as hir, Node}; @@ -120,15 +120,47 @@ fn entry_fn(tcx: TyCtxt<'_>) -> (DefId, MiriEntryFnType) { } } +fn run_many_seeds( + many_seeds: ManySeedsConfig, + eval_entry_once: impl Fn(u64) -> Result<(), NonZeroI32> + DynSync, +) -> Result<(), NonZeroI32> { + let exit_code = + sync::IntoDynSyncSend(AtomicU32::new(rustc_driver::EXIT_SUCCESS.cast_unsigned())); + let num_failed = sync::IntoDynSyncSend(AtomicU32::new(0)); + sync::par_for_each_in(many_seeds.seeds.clone(), |&seed| { + if let Err(return_code) = eval_entry_once(seed.into()) { + eprintln!("FAILING SEED: {seed}"); + if !many_seeds.keep_going { + // `abort_if_errors` would unwind but would not actually stop miri, since + // `par_for_each` waits for the rest of the threads to finish. + exit(return_code.get()); + } + // Preserve the "maximum" return code (when interpreted as `u32`), to make + // the result order-independent and to make it 0 only if all executions were 0. + exit_code.fetch_max(return_code.get().cast_unsigned(), Ordering::Relaxed); + num_failed.fetch_add(1, Ordering::Relaxed); + } + }); + let num_failed = num_failed.0.into_inner(); + let exit_code = exit_code.0.into_inner().cast_signed(); + if num_failed > 0 { + eprintln!("{num_failed}/{total} SEEDS FAILED", total = many_seeds.seeds.count()); + Err(NonZeroI32::new(exit_code).unwrap()) + } else { + assert!(exit_code == 0); + Ok(()) + } +} + impl rustc_driver::Callbacks for MiriCompilerCalls { fn after_analysis<'tcx>( &mut self, _: &rustc_interface::interface::Compiler, tcx: TyCtxt<'tcx>, ) -> Compilation { - if tcx.sess.dcx().has_errors_or_delayed_bugs().is_some() { - tcx.dcx().fatal("miri cannot be run on programs that fail compilation"); - } + tcx.dcx().abort_if_errors(); + tcx.dcx().flush_delayed(); + if !tcx.crate_types().contains(&CrateType::Executable) { tcx.dcx().fatal("miri only makes sense on bin crates"); } @@ -161,64 +193,28 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { optimizations is usually marginal at best."); } - // Run in GenMC mode if enabled. - if config.genmc_config.is_some() { - // Validate GenMC settings. - if let Err(err) = GenmcConfig::validate(&mut config, tcx) { - fatal_error!("Invalid settings: {err}"); - } - - // This is the entry point used in GenMC mode. - // This closure will be called multiple times to explore the concurrent execution space of the program. - let eval_entry_once = |genmc_ctx: Rc| { + let res = if config.genmc_config.is_some() { + assert!(self.many_seeds.is_none()); + run_genmc_mode(tcx, &config, |genmc_ctx: Rc| { miri::eval_entry(tcx, entry_def_id, entry_type, &config, Some(genmc_ctx)) - }; - let return_code = run_genmc_mode(&config, eval_entry_once, tcx).unwrap_or_else(|| { - tcx.dcx().abort_if_errors(); - rustc_driver::EXIT_FAILURE - }); - exit(return_code); - }; - - if let Some(many_seeds) = self.many_seeds.take() { + }) + } else if let Some(many_seeds) = self.many_seeds.take() { assert!(config.seed.is_none()); - let exit_code = sync::IntoDynSyncSend(AtomicI32::new(rustc_driver::EXIT_SUCCESS)); - let num_failed = sync::IntoDynSyncSend(AtomicU32::new(0)); - sync::par_for_each_in(many_seeds.seeds.clone(), |seed| { + run_many_seeds(many_seeds, |seed| { let mut config = config.clone(); - config.seed = Some((*seed).into()); + config.seed = Some(seed); eprintln!("Trying seed: {seed}"); - let return_code = miri::eval_entry( - tcx, - entry_def_id, - entry_type, - &config, - /* genmc_ctx */ None, - ) - .unwrap_or(rustc_driver::EXIT_FAILURE); - if return_code != rustc_driver::EXIT_SUCCESS { - eprintln!("FAILING SEED: {seed}"); - if !many_seeds.keep_going { - // `abort_if_errors` would actually not stop, since `par_for_each` waits for the - // rest of the to finish, so we just exit immediately. - exit(return_code); - } - exit_code.store(return_code, Ordering::Relaxed); - num_failed.fetch_add(1, Ordering::Relaxed); - } - }); - let num_failed = num_failed.0.into_inner(); - if num_failed > 0 { - eprintln!("{num_failed}/{total} SEEDS FAILED", total = many_seeds.seeds.count()); - } - exit(exit_code.0.into_inner()); + miri::eval_entry(tcx, entry_def_id, entry_type, &config, /* genmc_ctx */ None) + }) } else { - let return_code = miri::eval_entry(tcx, entry_def_id, entry_type, &config, None) - .unwrap_or_else(|| { - tcx.dcx().abort_if_errors(); - rustc_driver::EXIT_FAILURE - }); - exit(return_code); + miri::eval_entry(tcx, entry_def_id, entry_type, &config, None) + }; + + if let Err(return_code) = res { + tcx.dcx().abort_if_errors(); + exit(return_code.get()); + } else { + exit(rustc_driver::EXIT_SUCCESS); } // Unreachable. @@ -571,7 +567,10 @@ fn main() { } else if arg == "-Zmiri-mute-stdout-stderr" { miri_config.mute_stdout_stderr = true; } else if arg == "-Zmiri-retag-fields" { - miri_config.retag_fields = RetagFields::Yes; + eprintln!( + "warning: `-Zmiri-retag-fields` is a NOP and will be removed in a future version of Miri.\n\ + Field retagging has been on-by-default for a long time." + ); } else if arg == "-Zmiri-fixed-schedule" { miri_config.fixed_scheduling = true; } else if arg == "-Zmiri-deterministic-concurrency" { @@ -579,13 +578,6 @@ fn main() { miri_config.address_reuse_cross_thread_rate = 0.0; miri_config.cmpxchg_weak_failure_rate = 0.0; miri_config.weak_memory_emulation = false; - } else if let Some(retag_fields) = arg.strip_prefix("-Zmiri-retag-fields=") { - miri_config.retag_fields = match retag_fields { - "all" => RetagFields::Yes, - "none" => RetagFields::No, - "scalar" => RetagFields::OnlyScalar, - _ => fatal_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), - }; } else if let Some(param) = arg.strip_prefix("-Zmiri-seed=") { let seed = param.parse::().unwrap_or_else(|_| { fatal_error!("-Zmiri-seed must be an integer that fits into u64") @@ -747,6 +739,13 @@ fn main() { ); }; + // Validate GenMC settings. + if miri_config.genmc_config.is_some() + && let Err(err) = GenmcConfig::validate(&mut miri_config) + { + fatal_error!("Invalid settings: {err}"); + } + debug!("rustc arguments: {:?}", rustc_args); debug!("crate arguments: {:?}", miri_config.args); if !miri_config.native_lib.is_empty() && miri_config.native_lib_enable_tracing { diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index ebca7377fdbcc..e83028f0248f6 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -116,8 +116,6 @@ pub struct GlobalStateInner { protected_tags: FxHashMap, /// The pointer ids to trace tracked_pointer_tags: FxHashSet, - /// Whether to recurse into datatypes when searching for pointers to retag. - retag_fields: RetagFields, } impl VisitProvenance for GlobalStateInner { @@ -131,18 +129,6 @@ impl VisitProvenance for GlobalStateInner { /// We need interior mutable access to the global state. pub type GlobalState = RefCell; -/// Policy on whether to recurse into fields to retag -#[derive(Copy, Clone, Debug)] -pub enum RetagFields { - /// Don't retag any fields. - No, - /// Retag all fields. - Yes, - /// Only retag fields of types with Scalar and ScalarPair layout, - /// to match the LLVM `noalias` we generate. - OnlyScalar, -} - /// The flavor of the protector. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum ProtectorKind { @@ -168,7 +154,6 @@ impl GlobalStateInner { pub fn new( borrow_tracker_method: BorrowTrackerMethod, tracked_pointer_tags: FxHashSet, - retag_fields: RetagFields, ) -> Self { GlobalStateInner { borrow_tracker_method, @@ -176,7 +161,6 @@ impl GlobalStateInner { root_ptr_tags: FxHashMap::default(), protected_tags: FxHashMap::default(), tracked_pointer_tags, - retag_fields, } } @@ -244,11 +228,7 @@ pub struct TreeBorrowsParams { impl BorrowTrackerMethod { pub fn instantiate_global_state(self, config: &MiriConfig) -> GlobalState { - RefCell::new(GlobalStateInner::new( - self, - config.tracked_pointer_tags.clone(), - config.retag_fields, - )) + RefCell::new(GlobalStateInner::new(self, config.tracked_pointer_tags.clone())) } pub fn get_tree_borrows_params(self) -> TreeBorrowsParams { diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index fa60f27185f83..e8d97491acaf3 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -9,7 +9,7 @@ use std::fmt::Write; use std::sync::atomic::AtomicBool; use std::{cmp, mem}; -use rustc_abi::{BackendRepr, Size}; +use rustc_abi::Size; use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{Mutability, RetagKind}; use rustc_middle::ty::layout::HasTypingEnv; @@ -887,14 +887,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { place: &PlaceTy<'tcx>, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields; let retag_cause = match kind { RetagKind::TwoPhase => unreachable!(), // can only happen in `retag_ptr_value` RetagKind::FnEntry => RetagCause::FnEntry, RetagKind::Default | RetagKind::Raw => RetagCause::Normal, }; - let mut visitor = - RetagVisitor { ecx: this, kind, retag_cause, retag_fields, in_field: false }; + let mut visitor = RetagVisitor { ecx: this, kind, retag_cause, in_field: false }; return visitor.visit_value(place); // The actual visitor. @@ -902,7 +900,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ecx: &'ecx mut MiriInterpCx<'tcx>, kind: RetagKind, retag_cause: RetagCause, - retag_fields: RetagFields, in_field: bool, } impl<'ecx, 'tcx> RetagVisitor<'ecx, 'tcx> { @@ -967,24 +964,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { self.walk_value(place)?; } _ => { - // Not a reference/pointer/box. Only recurse if configured appropriately. - let recurse = match self.retag_fields { - RetagFields::No => false, - RetagFields::Yes => true, - RetagFields::OnlyScalar => { - // Matching `ArgAbi::new` at the time of writing, only fields of - // `Scalar` and `ScalarPair` ABI are considered. - matches!( - place.layout.backend_repr, - BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..) - ) - } - }; - if recurse { - let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value - self.walk_value(place)?; - self.in_field = in_field; - } + // Not a reference/pointer/box. Recurse. + let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value + self.walk_value(place)?; + self.in_field = in_field; } } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 720c5b239495e..756ccc53222e7 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -1,4 +1,4 @@ -use rustc_abi::{BackendRepr, Size}; +use rustc_abi::Size; use rustc_middle::mir::{Mutability, RetagKind}; use rustc_middle::ty::layout::HasTypingEnv; use rustc_middle::ty::{self, Ty}; @@ -468,16 +468,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { place: &PlaceTy<'tcx>, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let options = this.machine.borrow_tracker.as_mut().unwrap().get_mut(); - let retag_fields = options.retag_fields; - let mut visitor = RetagVisitor { ecx: this, kind, retag_fields }; + let mut visitor = RetagVisitor { ecx: this, kind }; return visitor.visit_value(place); // The actual visitor. struct RetagVisitor<'ecx, 'tcx> { ecx: &'ecx mut MiriInterpCx<'tcx>, kind: RetagKind, - retag_fields: RetagFields, } impl<'ecx, 'tcx> RetagVisitor<'ecx, 'tcx> { #[inline(always)] // yes this helps in our benchmarks @@ -545,22 +542,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { self.walk_value(place)?; } _ => { - // Not a reference/pointer/box. Only recurse if configured appropriately. - let recurse = match self.retag_fields { - RetagFields::No => false, - RetagFields::Yes => true, - RetagFields::OnlyScalar => { - // Matching `ArgAbi::new` at the time of writing, only fields of - // `Scalar` and `ScalarPair` ABI are considered. - matches!( - place.layout.backend_repr, - BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..) - ) - } - }; - if recurse { - self.walk_value(place)?; - } + // Not a reference/pointer/box. Recurse. + self.walk_value(place)?; } } interp_ok(()) diff --git a/src/tools/miri/src/concurrency/genmc/config.rs b/src/tools/miri/src/concurrency/genmc/config.rs index a05cda46f3e74..ea5760f7acde8 100644 --- a/src/tools/miri/src/concurrency/genmc/config.rs +++ b/src/tools/miri/src/concurrency/genmc/config.rs @@ -1,6 +1,4 @@ use genmc_sys::LogLevel; -use rustc_abi::Endian; -use rustc_middle::ty::TyCtxt; use super::GenmcParams; use crate::{IsolatedOp, MiriConfig, RejectOpWith}; @@ -86,16 +84,11 @@ impl GenmcConfig { /// /// Unsupported configurations return an error. /// Adjusts Miri settings where required, printing a warnings if the change might be unexpected for the user. - pub fn validate(miri_config: &mut MiriConfig, tcx: TyCtxt<'_>) -> Result<(), &'static str> { + pub fn validate(miri_config: &mut MiriConfig) -> Result<(), &'static str> { let Some(genmc_config) = miri_config.genmc_config.as_mut() else { return Ok(()); }; - // Check for supported target. - if tcx.data_layout.endian != Endian::Little || tcx.data_layout.pointer_size().bits() != 64 { - return Err("GenMC only supports 64bit little-endian targets"); - } - // Check for disallowed configurations. if !miri_config.data_race_detector { return Err("Cannot disable data race detection in GenMC mode"); diff --git a/src/tools/miri/src/concurrency/genmc/dummy.rs b/src/tools/miri/src/concurrency/genmc/dummy.rs index b9e09e34dc360..d643bda28c34e 100644 --- a/src/tools/miri/src/concurrency/genmc/dummy.rs +++ b/src/tools/miri/src/concurrency/genmc/dummy.rs @@ -1,6 +1,5 @@ use rustc_abi::{Align, Size}; use rustc_const_eval::interpret::{AllocId, InterpCx, InterpResult}; -use rustc_middle::ty::TyCtxt; pub use self::intercept::EvalContextExt as GenmcEvalContextExt; pub use self::run::run_genmc_mode; @@ -23,6 +22,7 @@ pub struct GenmcCtx {} pub struct GenmcConfig {} mod run { + use std::num::NonZeroI32; use std::rc::Rc; use rustc_middle::ty::TyCtxt; @@ -30,10 +30,10 @@ mod run { use crate::{GenmcCtx, MiriConfig}; pub fn run_genmc_mode<'tcx>( - _config: &MiriConfig, - _eval_entry: impl Fn(Rc) -> Option, _tcx: TyCtxt<'tcx>, - ) -> Option { + _config: &MiriConfig, + _eval_entry: impl Fn(Rc) -> Result<(), NonZeroI32>, + ) -> Result<(), NonZeroI32> { unreachable!(); } } @@ -240,10 +240,7 @@ impl GenmcConfig { } } - pub fn validate( - _miri_config: &mut crate::MiriConfig, - _tcx: TyCtxt<'_>, - ) -> Result<(), &'static str> { + pub fn validate(_miri_config: &mut crate::MiriConfig) -> Result<(), &'static str> { Ok(()) } } diff --git a/src/tools/miri/src/concurrency/genmc/mod.rs b/src/tools/miri/src/concurrency/genmc/mod.rs index bc475c680b5b1..6628e096a25d0 100644 --- a/src/tools/miri/src/concurrency/genmc/mod.rs +++ b/src/tools/miri/src/concurrency/genmc/mod.rs @@ -8,9 +8,9 @@ use genmc_sys::{ use rustc_abi::{Align, Size}; use rustc_const_eval::interpret::{AllocId, InterpCx, InterpResult, interp_ok}; use rustc_data_structures::fx::FxHashMap; -use rustc_middle::{throw_machine_stop, throw_ub_format, throw_unsup_format}; +use rustc_middle::{throw_ub_format, throw_unsup_format}; // FIXME(genmc,tracing): Implement some work-around for enabling debug/trace level logging (currently disabled statically in rustc). -use tracing::{debug, info}; +use tracing::debug; use self::global_allocations::{EvalContextExt as _, GlobalAllocationHandler}; use self::helper::{ @@ -63,12 +63,6 @@ struct ExitStatus { exit_type: ExitType, } -impl ExitStatus { - fn do_leak_check(self) -> bool { - matches!(self.exit_type, ExitType::MainThreadFinish) - } -} - /// State that is reset at the start of every execution. #[derive(Debug, Default)] struct PerExecutionState { @@ -223,8 +217,6 @@ impl GenmcCtx { /// Inform GenMC that the program's execution has ended. /// - /// This function must be called even when the execution is blocked - /// (i.e., it returned a `InterpErrorKind::MachineStop` with error kind `TerminationInfo::GenmcBlockedExecution`). /// Don't call this function if an error was found. /// /// GenMC detects certain errors only when the execution ends. @@ -694,39 +686,37 @@ impl GenmcCtx { } /// Handle a call to `libc::exit` or the exit of the main thread. - /// Unless an error is returned, the program should continue executing (in a different thread, chosen by the next scheduling call). + /// Unless an error is returned, the program should continue executing (in a different thread, + /// chosen by the next scheduling call). pub(crate) fn handle_exit<'tcx>( &self, thread: ThreadId, exit_code: i32, exit_type: ExitType, ) -> InterpResult<'tcx> { - // Calling `libc::exit` doesn't do cleanup, so we skip the leak check in that case. - let exit_status = ExitStatus { exit_code, exit_type }; - if let Some(old_exit_status) = self.exec_state.exit_status.get() { throw_ub_format!( - "`exit` called twice, first with status {old_exit_status:?}, now with status {exit_status:?}", + "`exit` called twice, first with exit code {old_exit_code}, now with status {exit_code}", + old_exit_code = old_exit_status.exit_code, ); } - // FIXME(genmc): Add a flag to continue exploration even when the program exits with a non-zero exit code. - if exit_code != 0 { - info!("GenMC: 'exit' called with non-zero argument, aborting execution."); - let leak_check = exit_status.do_leak_check(); - throw_machine_stop!(TerminationInfo::Exit { code: exit_code, leak_check }); - } - - if matches!(exit_type, ExitType::ExitCalled) { - let thread_infos = self.exec_state.thread_id_manager.borrow(); - let genmc_tid = thread_infos.get_genmc_tid(thread); - - self.handle.borrow_mut().pin_mut().handle_thread_kill(genmc_tid); - } else { - assert_eq!(thread, ThreadId::MAIN_THREAD); + match exit_type { + ExitType::ExitCalled => { + // `exit` kills the current thread; we have to tell GenMC about this. + let thread_infos = self.exec_state.thread_id_manager.borrow(); + let genmc_tid = thread_infos.get_genmc_tid(thread); + self.handle.borrow_mut().pin_mut().handle_thread_kill(genmc_tid); + } + ExitType::MainThreadFinish => { + // The main thread has already exited so we don't call `handle_thread_kill` again. + assert_eq!(thread, ThreadId::MAIN_THREAD); + } } - // We continue executing now, so we store the exit status. - self.exec_state.exit_status.set(Some(exit_status)); + // To cover all possible behaviors, we have to continue execution the other threads: + // whatever they do next could also have happened before the `exit` call. So we just + // remember the exit status and use it when the other threads are done. + self.exec_state.exit_status.set(Some(ExitStatus { exit_code, exit_type })); interp_ok(()) } } diff --git a/src/tools/miri/src/concurrency/genmc/run.rs b/src/tools/miri/src/concurrency/genmc/run.rs index 6eb51e1b50cb9..2b0de62ccda5b 100644 --- a/src/tools/miri/src/concurrency/genmc/run.rs +++ b/src/tools/miri/src/concurrency/genmc/run.rs @@ -1,8 +1,10 @@ +use std::num::NonZeroI32; use std::rc::Rc; use std::sync::Arc; use std::time::Instant; use genmc_sys::EstimationResult; +use rustc_abi::Endian; use rustc_log::tracing; use rustc_middle::ty::TyCtxt; @@ -24,10 +26,15 @@ pub(super) enum GenmcMode { /// /// Returns `None` is an error is detected, or `Some(return_value)` with the return value of the last run of the program. pub fn run_genmc_mode<'tcx>( - config: &MiriConfig, - eval_entry: impl Fn(Rc) -> Option, tcx: TyCtxt<'tcx>, -) -> Option { + config: &MiriConfig, + eval_entry: impl Fn(Rc) -> Result<(), NonZeroI32>, +) -> Result<(), NonZeroI32> { + // Check for supported target. + if tcx.data_layout.endian != Endian::Little || tcx.data_layout.pointer_size().bits() != 64 { + tcx.dcx().fatal("GenMC only supports 64bit little-endian targets"); + } + let genmc_config = config.genmc_config.as_ref().unwrap(); // Run in Estimation mode if requested. if genmc_config.do_estimation { @@ -41,10 +48,10 @@ pub fn run_genmc_mode<'tcx>( fn run_genmc_mode_impl<'tcx>( config: &MiriConfig, - eval_entry: &impl Fn(Rc) -> Option, + eval_entry: &impl Fn(Rc) -> Result<(), NonZeroI32>, tcx: TyCtxt<'tcx>, mode: GenmcMode, -) -> Option { +) -> Result<(), NonZeroI32> { let time_start = Instant::now(); let genmc_config = config.genmc_config.as_ref().unwrap(); @@ -62,9 +69,9 @@ fn run_genmc_mode_impl<'tcx>( genmc_ctx.prepare_next_execution(); // Execute the program until completion to get the return value, or return if an error happens: - let Some(return_code) = eval_entry(genmc_ctx.clone()) else { + if let Err(err) = eval_entry(genmc_ctx.clone()) { genmc_ctx.print_genmc_output(genmc_config, tcx); - return None; + return Err(err); }; // We inform GenMC that the execution is complete. @@ -80,18 +87,17 @@ fn run_genmc_mode_impl<'tcx>( genmc_ctx.print_verification_output(genmc_config, elapsed_time_sec); } // Return the return code of the last execution. - return Some(return_code); + return Ok(()); } ExecutionEndResult::Error(error) => { // This can be reached for errors that affect the entire execution, not just a specific event. // For instance, linearizability checking and liveness checking report their errors this way. - // Neither are supported by Miri-GenMC at the moment though. However, GenMC also - // treats races on deallocation as global errors, so this code path is still reachable. + // Neither are supported by Miri-GenMC at the moment though. // Since we don't have any span information for the error at this point, // we just print GenMC's error string, and the full GenMC output if requested. eprintln!("(GenMC) Error detected: {error}"); genmc_ctx.print_genmc_output(genmc_config, tcx); - return None; + return Err(NonZeroI32::new(rustc_driver::EXIT_FAILURE).unwrap()); } } } diff --git a/src/tools/miri/src/concurrency/genmc/scheduling.rs b/src/tools/miri/src/concurrency/genmc/scheduling.rs index 0703e0595b338..c760126d787d1 100644 --- a/src/tools/miri/src/concurrency/genmc/scheduling.rs +++ b/src/tools/miri/src/concurrency/genmc/scheduling.rs @@ -118,14 +118,21 @@ impl GenmcCtx { // Depending on the exec_state, we either schedule the given thread, or we are finished with this execution. match result.exec_state { ExecutionState::Ok => interp_ok(Some(thread_infos.get_miri_tid(result.next_thread))), - ExecutionState::Blocked => throw_machine_stop!(TerminationInfo::GenmcBlockedExecution), + ExecutionState::Blocked => { + // This execution doesn't need further exploration. We treat this as "success, no + // leak check needed", which makes it a NOP in the big outer loop. + throw_machine_stop!(TerminationInfo::Exit { + code: 0, // success + leak_check: false, + }); + } ExecutionState::Finished => { let exit_status = self.exec_state.exit_status.get().expect( "If the execution is finished, we should have a return value from the program.", ); throw_machine_stop!(TerminationInfo::Exit { code: exit_status.exit_code, - leak_check: exit_status.do_leak_check() + leak_check: matches!(exit_status.exit_type, super::ExitType::MainThreadFinish), }); } ExecutionState::Error => { diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 7838bd214302b..c7ae335d04795 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -515,10 +515,13 @@ impl<'tcx> ThreadManager<'tcx> { &mut self.threads[self.active_thread].stack } - pub fn all_stacks( + pub fn all_blocked_stacks( &self, ) -> impl Iterator>])> { - self.threads.iter_enumerated().map(|(id, t)| (id, &t.stack[..])) + self.threads + .iter_enumerated() + .filter(|(_id, t)| matches!(t.state, ThreadState::Blocked { .. })) + .map(|(id, t)| (id, &t.stack[..])) } /// Create a new thread and returns its id. diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 2ddb3ff49d85e..bb8ba196983c4 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -33,9 +33,6 @@ pub enum TerminationInfo { }, Int2PtrWithStrictProvenance, Deadlock, - /// In GenMC mode, executions can get blocked, which stops the current execution without running any cleanup. - /// No leak checks should be performed if this happens, since they would give false positives. - GenmcBlockedExecution, MultipleSymbolDefinitions { link_name: Symbol, first: SpanData, @@ -80,8 +77,6 @@ impl fmt::Display for TerminationInfo { StackedBorrowsUb { msg, .. } => write!(f, "{msg}"), TreeBorrowsUb { title, .. } => write!(f, "{title}"), Deadlock => write!(f, "the evaluated program deadlocked"), - GenmcBlockedExecution => - write!(f, "GenMC determined that the execution got blocked (this is not an error)"), MultipleSymbolDefinitions { link_name, .. } => write!(f, "multiple definitions of symbol `{link_name}`"), SymbolShimClashing { link_name, .. } => @@ -226,19 +221,20 @@ pub fn prune_stacktrace<'tcx>( } } -/// Emit a custom diagnostic without going through the miri-engine machinery. +/// Report the result of a Miri execution. /// -/// Returns `Some` if this was regular program termination with a given exit code and a `bool` indicating whether a leak check should happen; `None` otherwise. -pub fn report_error<'tcx>( +/// Returns `Some` if this was regular program termination with a given exit code and a `bool` +/// indicating whether a leak check should happen; `None` otherwise. +pub fn report_result<'tcx>( ecx: &InterpCx<'tcx, MiriMachine<'tcx>>, - e: InterpErrorInfo<'tcx>, + res: InterpErrorInfo<'tcx>, ) -> Option<(i32, bool)> { use InterpErrorKind::*; use UndefinedBehaviorInfo::*; let mut labels = vec![]; - let (title, helps) = if let MachineStop(info) = e.kind() { + let (title, helps) = if let MachineStop(info) = res.kind() { let info = info.downcast_ref::().expect("invalid MachineStop payload"); use TerminationInfo::*; let title = match info { @@ -253,13 +249,6 @@ pub fn report_error<'tcx>( labels.push(format!("this thread got stuck here")); None } - GenmcBlockedExecution => { - // This case should only happen in GenMC mode. - assert!(ecx.machine.data_race.as_genmc_ref().is_some()); - // The program got blocked by GenMC without finishing the execution. - // No cleanup code was executed, so we don't do any leak checks. - return Some((0, false)); - } MultipleSymbolDefinitions { .. } | SymbolShimClashing { .. } => None, }; #[rustfmt::skip] @@ -334,7 +323,7 @@ pub fn report_error<'tcx>( }; (title, helps) } else { - let title = match e.kind() { + let title = match res.kind() { UndefinedBehavior(ValidationError(validation_err)) if matches!( validation_err.kind, @@ -344,7 +333,7 @@ pub fn report_error<'tcx>( ecx.handle_ice(); // print interpreter backtrace (this is outside the eval `catch_unwind`) bug!( "This validation error should be impossible in Miri: {}", - format_interp_error(ecx.tcx.dcx(), e) + format_interp_error(ecx.tcx.dcx(), res) ); } UndefinedBehavior(_) => "Undefined Behavior", @@ -363,12 +352,12 @@ pub fn report_error<'tcx>( ecx.handle_ice(); // print interpreter backtrace (this is outside the eval `catch_unwind`) bug!( "This error should be impossible in Miri: {}", - format_interp_error(ecx.tcx.dcx(), e) + format_interp_error(ecx.tcx.dcx(), res) ); } }; #[rustfmt::skip] - let helps = match e.kind() { + let helps = match res.kind() { Unsupported(_) => vec![ note!("this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support"), @@ -422,7 +411,7 @@ pub fn report_error<'tcx>( // We want to dump the allocation if this is `InvalidUninitBytes`. // Since `format_interp_error` consumes `e`, we compute the outut early. let mut extra = String::new(); - match e.kind() { + match res.kind() { UndefinedBehavior(InvalidUninitBytes(Some((alloc_id, access)))) => { writeln!( extra, @@ -448,7 +437,7 @@ pub fn report_error<'tcx>( if let Some(title) = title { write!(primary_msg, "{title}: ").unwrap(); } - write!(primary_msg, "{}", format_interp_error(ecx.tcx.dcx(), e)).unwrap(); + write!(primary_msg, "{}", format_interp_error(ecx.tcx.dcx(), res)).unwrap(); if labels.is_empty() { labels.push(format!("{} occurred here", title.unwrap_or("error"))); @@ -468,7 +457,7 @@ pub fn report_error<'tcx>( eprint!("{extra}"); // newlines are already in the string if show_all_threads { - for (thread, stack) in ecx.machine.threads.all_stacks() { + for (thread, stack) in ecx.machine.threads.all_blocked_stacks() { if thread != ecx.active_thread() { let stacktrace = Frame::generate_stacktrace_from_stack(stack); let (stacktrace, was_pruned) = prune_stacktrace(stacktrace, &ecx.machine); diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 6daee0ba69692..01a54bbb33746 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -1,6 +1,7 @@ //! Main evaluator loop and setting up the initial stack frame. use std::ffi::{OsStr, OsString}; +use std::num::NonZeroI32; use std::panic::{self, AssertUnwindSafe}; use std::path::PathBuf; use std::rc::Rc; @@ -88,8 +89,6 @@ pub struct MiriConfig { pub preemption_rate: f64, /// Report the current instruction being executed every N basic blocks. pub report_progress: Option, - /// Whether Stacked Borrows and Tree Borrows retagging should recurse into fields of datatypes. - pub retag_fields: RetagFields, /// The location of the shared object files to load when calling external functions pub native_lib: Vec, /// Whether to enable the new native lib tracing system. @@ -147,7 +146,6 @@ impl Default for MiriConfig { mute_stdout_stderr: false, preemption_rate: 0.01, // 1% report_progress: None, - retag_fields: RetagFields::Yes, native_lib: vec![], native_lib_enable_tracing: false, gc_interval: 10_000, @@ -462,7 +460,7 @@ pub fn eval_entry<'tcx>( entry_type: MiriEntryFnType, config: &MiriConfig, genmc_ctx: Option>, -) -> Option { +) -> Result<(), NonZeroI32> { // Copy setting before we move `config`. let ignore_leaks = config.ignore_leaks; @@ -482,35 +480,50 @@ pub fn eval_entry<'tcx>( ecx.handle_ice(); panic::resume_unwind(panic_payload) }); - // `Ok` can never happen; the interpreter loop always exits with an "error" - // (but that "error" might be just "regular program termination"). - let Err(err) = res.report_err(); - - // Show diagnostic, if any. - let (return_code, leak_check) = report_error(&ecx, err)?; - - // If we get here there was no fatal error. - - // Possibly check for memory leaks. - if leak_check && !ignore_leaks { - // Check for thread leaks. - if !ecx.have_all_terminated() { - tcx.dcx().err("the main thread terminated without waiting for all remaining threads"); - tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); - return None; - } - // Check for memory leaks. - info!("Additional static roots: {:?}", ecx.machine.static_roots); - let leaks = ecx.take_leaked_allocations(|ecx| &ecx.machine.static_roots); - if !leaks.is_empty() { - report_leaks(&ecx, leaks); - tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); - // Ignore the provided return code - let the reported error - // determine the return code. - return None; + // Obtain the result of the execution. This is always an `Err`, but that doesn't necessarily + // indicate an error. + let Err(res) = res.report_err(); + + // Error reporting: if we survive all checks, we return the exit code the program gave us. + 'miri_error: { + // Show diagnostic, if any. + let Some((return_code, leak_check)) = report_result(&ecx, res) else { + break 'miri_error; + }; + + // If we get here there was no fatal error -- yet. + // Possibly check for memory leaks. + if leak_check && !ignore_leaks { + // Check for thread leaks. + if !ecx.have_all_terminated() { + tcx.dcx() + .err("the main thread terminated without waiting for all remaining threads"); + tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); + break 'miri_error; + } + // Check for memory leaks. + info!("Additional static roots: {:?}", ecx.machine.static_roots); + let leaks = ecx.take_leaked_allocations(|ecx| &ecx.machine.static_roots); + if !leaks.is_empty() { + report_leaks(&ecx, leaks); + tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); + // Ignore the provided return code - let the reported error + // determine the return code. + break 'miri_error; + } } + + // The interpreter has not reported an error. + // (There could still be errors in the session if there are other interpreters.) + return match NonZeroI32::new(return_code) { + None => Ok(()), + Some(return_code) => Err(return_code), + }; } - Some(return_code) + + // The interpreter reported an error. + assert!(tcx.dcx().has_errors().is_some()); + Err(NonZeroI32::new(rustc_driver::EXIT_FAILURE).unwrap()) } /// Turns an array of arguments into a Windows command line string. diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 3137cfc8330bd..5ec2e8edd857d 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -119,7 +119,7 @@ pub use crate::borrow_tracker::stacked_borrows::{ }; pub use crate::borrow_tracker::tree_borrows::{EvalContextExt as _, Tree}; pub use crate::borrow_tracker::{ - BorTag, BorrowTrackerMethod, EvalContextExt as _, RetagFields, TreeBorrowsParams, + BorTag, BorrowTrackerMethod, EvalContextExt as _, TreeBorrowsParams, }; pub use crate::clock::{Instant, MonotonicClock}; pub use crate::concurrency::cpu_affinity::MAX_CPUS; @@ -136,7 +136,7 @@ pub use crate::concurrency::{GenmcConfig, GenmcCtx, run_genmc_mode}; pub use crate::data_structures::dedup_range_map::DedupRangeMap; pub use crate::data_structures::mono_hash_map::MonoHashMap; pub use crate::diagnostics::{ - EvalContextExt as _, NonHaltingDiagnostic, TerminationInfo, report_error, + EvalContextExt as _, NonHaltingDiagnostic, TerminationInfo, report_result, }; pub use crate::eval::{MiriConfig, MiriEntryFnType, create_ecx, eval_entry}; pub use crate::helpers::{EvalContextExt as _, ToU64 as _, ToUsize as _}; diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 69a028e485079..440388673e7d4 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -15,7 +15,7 @@ use rustc_middle::{mir, ty}; use rustc_session::config::OomStrategy; use rustc_span::Symbol; use rustc_target::callconv::FnAbi; -use rustc_target::spec::{Os, Arch}; +use rustc_target::spec::{Arch, Os}; use super::alloc::EvalContextExt as _; use super::backtrace::EvalContextExt as _; diff --git a/src/tools/miri/src/shims/native_lib/trace/parent.rs b/src/tools/miri/src/shims/native_lib/trace/parent.rs index f6ebbc469f737..5476cccc02e3b 100644 --- a/src/tools/miri/src/shims/native_lib/trace/parent.rs +++ b/src/tools/miri/src/shims/native_lib/trace/parent.rs @@ -500,9 +500,8 @@ fn handle_segfault( capstone_disassemble(&instr, addr, cs, acc_events).expect("Failed to disassemble instruction"); // Move the instr ptr into the deprotection code. - #[allow(unknown_lints)] - #[expect(clippy::as_conversions, function_casts_as_integer)] - new_regs.set_ip(mempr_off as usize); + #[expect(clippy::as_conversions)] + new_regs.set_ip(mempr_off as *const () as usize); // Don't mess up the stack by accident! new_regs.set_sp(stack_ptr); @@ -553,9 +552,8 @@ fn handle_segfault( new_regs = regs_bak; // Reprotect everything and continue. - #[allow(unknown_lints)] - #[expect(clippy::as_conversions, function_casts_as_integer)] - new_regs.set_ip(mempr_on as usize); + #[expect(clippy::as_conversions)] + new_regs.set_ip(mempr_on as *const () as usize); new_regs.set_sp(stack_ptr); ptrace::setregs(pid, new_regs).unwrap(); wait_for_signal(Some(pid), signal::SIGSTOP, InitialCont::Yes)?; diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 4f1d88b79954e..3c2e14c181682 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -10,7 +10,7 @@ use rustc_target::spec::Os; use crate::shims::files::FileDescription; use crate::shims::sig::check_min_vararg_count; -use crate::shims::unix::linux_like::epoll::EpollReadyEvents; +use crate::shims::unix::linux_like::epoll::EpollEvents; use crate::shims::unix::*; use crate::*; @@ -62,8 +62,8 @@ pub trait UnixFileDescription: FileDescription { throw_unsup_format!("cannot flock {}", self.name()); } - /// Check the readiness of file description. - fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + /// Return which epoll events are currently active. + fn epoll_active_events<'tcx>(&self) -> InterpResult<'tcx, EpollEvents> { throw_unsup_format!("{}: epoll does not support this file description", self.name()); } } diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index e2d27e5702150..a37a34f8df743 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -27,14 +27,15 @@ pub fn is_dyn_sym(name: &str, target_os: &Os) -> bool { // needed at least on macOS to avoid file-based fallback in getrandom "getentropy" | "getrandom" => true, // Give specific OSes a chance to allow their symbols. - _ => match *target_os { - Os::Android => android::is_dyn_sym(name), - Os::FreeBsd => freebsd::is_dyn_sym(name), - Os::Linux => linux::is_dyn_sym(name), - Os::MacOs => macos::is_dyn_sym(name), - Os::Solaris | Os::Illumos => solarish::is_dyn_sym(name), - _ => false, - }, + _ => + match *target_os { + Os::Android => android::is_dyn_sym(name), + Os::FreeBsd => freebsd::is_dyn_sym(name), + Os::Linux => linux::is_dyn_sym(name), + Os::MacOs => macos::is_dyn_sym(name), + Os::Solaris | Os::Illumos => solarish::is_dyn_sym(name), + _ => false, + }, } } @@ -530,7 +531,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } "pipe2" => { // Currently this function does not exist on all Unixes, e.g. on macOS. - this.check_target_os(&[Os::Linux, Os::FreeBsd, Os::Solaris, Os::Illumos], link_name)?; + this.check_target_os( + &[Os::Linux, Os::FreeBsd, Os::Solaris, Os::Illumos], + link_name, + )?; let [pipefd, flags] = this.check_shim_sig( shim_sig!(extern "C" fn(*mut _, i32) -> i32), link_name, diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 0760125917603..137e60aaba4b8 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -530,7 +530,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - if !matches!(&this.tcx.sess.target.os, Os::MacOs | Os::FreeBsd | Os::Solaris | Os::Illumos) { + if !matches!(&this.tcx.sess.target.os, Os::MacOs | Os::FreeBsd | Os::Solaris | Os::Illumos) + { panic!("`macos_fbsd_solaris_stat` should not be called on {}", this.tcx.sess.target.os); } @@ -560,7 +561,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - if !matches!(&this.tcx.sess.target.os, Os::MacOs | Os::FreeBsd | Os::Solaris | Os::Illumos) { + if !matches!(&this.tcx.sess.target.os, Os::MacOs | Os::FreeBsd | Os::Solaris | Os::Illumos) + { panic!( "`macos_fbsd_solaris_lstat` should not be called on {}", this.tcx.sess.target.os @@ -591,7 +593,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - if !matches!(&this.tcx.sess.target.os, Os::MacOs | Os::FreeBsd | Os::Solaris | Os::Illumos) { + if !matches!(&this.tcx.sess.target.os, Os::MacOs | Os::FreeBsd | Os::Solaris | Os::Illumos) + { panic!( "`macos_fbsd_solaris_fstat` should not be called on {}", this.tcx.sess.target.os @@ -904,7 +907,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn readdir64(&mut self, dirent_type: &str, dirp_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - if !matches!(&this.tcx.sess.target.os, Os::Linux | Os::Solaris | Os::Illumos | Os::FreeBsd) { + if !matches!(&this.tcx.sess.target.os, Os::Linux | Os::Solaris | Os::Illumos | Os::FreeBsd) + { panic!("`linux_solaris_readdir64` should not be called on {}", this.tcx.sess.target.os); } diff --git a/src/tools/miri/src/shims/unix/linux_like/epoll.rs b/src/tools/miri/src/shims/unix/linux_like/epoll.rs index 865cdb2533cfd..52b761fa08a1c 100644 --- a/src/tools/miri/src/shims/unix/linux_like/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux_like/epoll.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::collections::{BTreeMap, btree_map}; +use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::io; use std::time::Duration; @@ -17,19 +17,14 @@ type EpollEventKey = (FdId, FdNum); /// An `Epoll` file descriptor connects file handles and epoll events #[derive(Debug, Default)] struct Epoll { - /// A map of EpollEventInterests registered under this epoll instance. - /// Each entry is differentiated using FdId and file descriptor value. + /// A map of EpollEventInterests registered under this epoll instance. Each entry is + /// differentiated using FdId and file descriptor value. interest_list: RefCell>, - /// A map of EpollEventInstance that will be returned when `epoll_wait` is called. - /// Similar to interest_list, the entry is also differentiated using FdId - /// and file descriptor value. - /// We keep this separate from `interest_list` for two reasons: there might be many - /// interests but only a few of them ready (so with a separate list it is more efficient - /// to find a ready event), and having separate `RefCell` lets us mutate the `interest_list` - /// while unblocking threads which might mutate the `ready_list`. - ready_list: RefCell>, - /// A list of thread ids blocked on this epoll instance. - blocked_tid: RefCell>, + /// The subset of interests that is currently considered "ready". Stored separately so we + /// can access it more efficiently. + ready_set: RefCell>, + /// The queue of threads blocked on this epoll instance. + queue: RefCell>, } impl VisitProvenance for Epoll { @@ -43,33 +38,16 @@ fn range_for_id(id: FdId) -> std::ops::RangeInclusive { (id, 0)..=(id, i32::MAX) } -/// EpollEventInstance contains information that will be returned by epoll_wait. -#[derive(Debug, Default)] -pub struct EpollEventInstance { - /// Bitmask of event types that happened to the file description. - events: u32, - /// User-defined data associated with the interest that triggered this instance. - data: u64, - /// The release clock associated with this event. - clock: VClock, -} - -/// EpollEventInterest registers the file description information to an epoll -/// instance during a successful `epoll_ctl` call. It also stores additional -/// information needed to check and update readiness state for `epoll_wait`. -/// -/// `events` and `data` field matches the `epoll_event` struct defined -/// by the epoll_ctl man page. For more information -/// see the man page: -/// -/// +/// Tracks the events that this epoll is interested in for a given file descriptor. #[derive(Debug)] pub struct EpollEventInterest { - /// The events bitmask retrieved from `epoll_event`. - events: u32, - /// The way the events looked last time we checked (for edge trigger / ET detection). - prev_events: u32, - /// The data retrieved from `epoll_event`. + /// The events bitmask the epoll is interested in. + relevant_events: u32, + /// The currently active events for this file descriptor. + active_events: u32, + /// The vector clock for wakeups. + clock: VClock, + /// User-defined data associated with this interest. /// libc's data field in epoll_event can store integer or pointer, /// but only u64 is supported for now. /// @@ -78,7 +56,7 @@ pub struct EpollEventInterest { /// EpollReadyEvents reflects the readiness of a file description. #[derive(Debug)] -pub struct EpollReadyEvents { +pub struct EpollEvents { /// The associated file is available for read(2) operations, in the sense that a read will not block. /// (I.e., returning EOF is considered "ready".) pub epollin: bool, @@ -97,9 +75,9 @@ pub struct EpollReadyEvents { pub epollerr: bool, } -impl EpollReadyEvents { +impl EpollEvents { pub fn new() -> Self { - EpollReadyEvents { + EpollEvents { epollin: false, epollout: false, epollrdhup: false, @@ -197,18 +175,17 @@ impl EpollInterestTable { if let Some(epolls) = self.0.remove(&id) { for epoll in epolls.iter().filter_map(|(_id, epoll)| epoll.upgrade()) { // This is a still-live epoll with interest in this FD. Remove all - // relevent interests. + // relevent interests (including from the ready set). epoll .interest_list .borrow_mut() .extract_if(range_for_id(id), |_, _| true) // Consume the iterator. .for_each(|_| ()); - // Also remove all events from the ready list that refer to this FD. epoll - .ready_list + .ready_set .borrow_mut() - .extract_if(range_for_id(id), |_, _| true) + .extract_if(range_for_id(id), |_| true) // Consume the iterator. .for_each(|_| ()); } @@ -344,57 +321,60 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Add new interest to list. Experiments show that we need to reset all state // on `EPOLL_CTL_MOD`, including the edge tracking. let epoll_key = (id, fd); - let new_interest = EpollEventInterest { events, data, prev_events: 0 }; - let new_interest = if op == epoll_ctl_add { + if op == epoll_ctl_add { if interest_list.range(range_for_id(id)).next().is_none() { // This is the first time this FD got added to this epoll. // Remember that in the global list so we get notified about FD events. this.machine.epoll_interests.insert(id, &epfd); } - match interest_list.entry(epoll_key) { - btree_map::Entry::Occupied(_) => { - // We already had interest in this. - return this.set_last_error_and_return_i32(LibcError("EEXIST")); - } - btree_map::Entry::Vacant(e) => e.insert(new_interest), + let new_interest = EpollEventInterest { + relevant_events: events, + data, + active_events: 0, + clock: VClock::default(), + }; + if interest_list.try_insert(epoll_key, new_interest).is_err() { + // We already had interest in this. + return this.set_last_error_and_return_i32(LibcError("EEXIST")); } } else { // Modify the existing interest. let Some(interest) = interest_list.get_mut(&epoll_key) else { return this.set_last_error_and_return_i32(LibcError("ENOENT")); }; - *interest = new_interest; - interest - }; + interest.relevant_events = events; + interest.data = data; + } // Deliver events for the new interest. - let force_edge = true; // makes no difference since we reset `prev_events` - send_ready_events_to_interests( + update_readiness( this, &epfd, - fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this), - force_edge, - std::iter::once((&epoll_key, new_interest)), + fd_ref.as_unix(this).epoll_active_events()?.get_event_bitmask(this), + /* force_edge */ true, + move |callback| { + // Need to release the RefCell when this closure returns, so we have to move + // it into the closure, so we have to do a re-lookup here. + callback(epoll_key, interest_list.get_mut(&epoll_key).unwrap()) + }, )?; interp_ok(Scalar::from_i32(0)) } else if op == epoll_ctl_del { let epoll_key = (id, fd); - // Remove epoll_event_interest from interest_list. + // Remove epoll_event_interest from interest_list and ready_set. if interest_list.remove(&epoll_key).is_none() { // We did not have interest in this. return this.set_last_error_and_return_i32(LibcError("ENOENT")); }; + epfd.ready_set.borrow_mut().remove(&epoll_key); // If this was the last interest in this FD, remove us from the global list // of who is interested in this FD. if interest_list.range(range_for_id(id)).next().is_none() { this.machine.epoll_interests.remove(id, epfd.id()); } - // Remove related event instance from ready list. - epfd.ready_list.borrow_mut().remove(&epoll_key); - interp_ok(Scalar::from_i32(0)) } else { throw_unsup_format!("unsupported epoll_ctl operation: {op}"); @@ -466,10 +446,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return this.set_last_error_and_return(LibcError("EBADF"), dest); }; - // We just need to know if the ready list is empty and borrow the thread_ids out. - let ready_list_empty = epfd.ready_list.borrow().is_empty(); - if timeout == 0 || !ready_list_empty { - // If the ready list is not empty, or the timeout is 0, we can return immediately. + if timeout == 0 || !epfd.ready_set.borrow().is_empty() { + // If the timeout is 0 or there is a ready event, we can return immediately. return_ready_list(&epfd, dest, &event, this)?; } else { // Blocking @@ -486,7 +464,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } }; // Record this thread as blocked. - epfd.blocked_tid.borrow_mut().push(this.active_thread()); + epfd.queue.borrow_mut().push_back(this.active_thread()); // And block it. let dest = dest.clone(); // We keep a strong ref to the underlying `Epoll` to make sure it sticks around. @@ -504,13 +482,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { |this, unblock: UnblockKind| { match unblock { UnblockKind::Ready => { - return_ready_list(&epfd, &dest, &event, this)?; + let events = return_ready_list(&epfd, &dest, &event, this)?; + assert!(events > 0, "we got woken up with no events to deliver"); interp_ok(()) }, UnblockKind::TimedOut => { // Remove the current active thread_id from the blocked thread_id list. epfd - .blocked_tid.borrow_mut() + .queue.borrow_mut() .retain(|&id| id != this.active_thread()); this.write_int(0, &dest)?; interp_ok(()) @@ -523,13 +502,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(()) } - /// For a specific file description, get its ready events and send it to everyone who registered - /// interest in this FD. This function should be called whenever the result of - /// `get_epoll_ready_events` would change. + /// For a specific file description, get its currently active events and send it to everyone who + /// registered interest in this FD. This function must be called whenever the result of + /// `epoll_active_events` might change. /// /// If `force_edge` is set, edge-triggered interests will be triggered even if the set of /// ready events did not change. This can lead to spurious wakeups. Use with caution! - fn epoll_send_fd_ready_events( + fn update_epoll_active_events( &mut self, fd_ref: DynFileDescriptionRef, force_edge: bool, @@ -537,7 +516,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let id = fd_ref.id(); // Figure out who is interested in this. We need to clone this list since we can't prove - // that `send_ready_events_to_interest` won't mutate it. + // that `send_active_events_to_interest` won't mutate it. let Some(epolls) = this.machine.epoll_interests.get_epolls(id) else { return interp_ok(()); }; @@ -547,72 +526,61 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { .expect("someone forgot to remove the garbage from `machine.epoll_interests`") }) .collect::>(); - let event_bitmask = fd_ref.as_unix(this).get_epoll_ready_events()?.get_event_bitmask(this); + let active_events = fd_ref.as_unix(this).epoll_active_events()?.get_event_bitmask(this); for epoll in epolls { - send_ready_events_to_interests( - this, - &epoll, - event_bitmask, - force_edge, - epoll.interest_list.borrow_mut().range_mut(range_for_id(id)), - )?; + update_readiness(this, &epoll, active_events, force_edge, |callback| { + for (&key, interest) in epoll.interest_list.borrow_mut().range_mut(range_for_id(id)) + { + callback(key, interest)?; + } + interp_ok(()) + })?; } interp_ok(()) } } -/// Send the latest ready events for one particular FD (identified by `event_key`) to everyone in -/// the `interests` list, if they are interested in this kind of event. -fn send_ready_events_to_interests<'tcx, 'a>( +/// Call this when the interests denoted by `for_each_interest` have their active event set changed +/// to `active_events`. The list is provided indirectly via the `for_each_interest` closure, which +/// will call its argument closure for each relevant interest. +/// +/// Any `RefCell` should be released by the time `for_each_interest` returns since we will then +/// be waking up threads which might require access to those `RefCell`. +fn update_readiness<'tcx>( ecx: &mut MiriInterpCx<'tcx>, epoll: &Epoll, - event_bitmask: u32, + active_events: u32, force_edge: bool, - interests: impl Iterator, + for_each_interest: impl FnOnce( + &mut dyn FnMut(EpollEventKey, &mut EpollEventInterest) -> InterpResult<'tcx>, + ) -> InterpResult<'tcx>, ) -> InterpResult<'tcx> { - let mut wakeup = false; - for (&event_key, interest) in interests { - let mut ready_list = epoll.ready_list.borrow_mut(); - // This checks if any of the events specified in epoll_event_interest.events - // match those in ready_events. - let flags = interest.events & event_bitmask; - let prev = std::mem::replace(&mut interest.prev_events, flags); - if flags == 0 { - // Make sure we *remove* any previous item from the ready list, since this - // is not ready any more. - ready_list.remove(&event_key); - continue; - } - // Generate new instance, or update existing one. It is crucial that whe we are done, - // if an interest exists in the ready list, then it matches the latest events and data! - let instance = match ready_list.entry(event_key) { - btree_map::Entry::Occupied(e) => e.into_mut(), - btree_map::Entry::Vacant(e) => { - if !force_edge && flags == prev & flags { - // Every bit in `flags` was already set in `prev`, and there's currently - // no entry in the ready list for this. So there is nothing new and no - // prior entry to update; just skip it. - continue; - } - e.insert(EpollEventInstance::default()) - } - }; - instance.events = flags; - instance.data = interest.data; - ecx.release_clock(|clock| { - instance.clock.join(clock); - })?; - wakeup = true; - } - if wakeup { - // Wake up threads that may have been waiting for events on this epoll. - // Do this only once for all the interests. - // Edge-triggered notification only notify one thread even if there are - // multiple threads blocked on the same epoll. - if let Some(thread_id) = epoll.blocked_tid.borrow_mut().pop() { - ecx.unblock_thread(thread_id, BlockReason::Epoll)?; + let mut ready_set = epoll.ready_set.borrow_mut(); + for_each_interest(&mut |key, interest| { + // Update the ready events tracked in this interest. + let new_readiness = interest.relevant_events & active_events; + let prev_readiness = std::mem::replace(&mut interest.active_events, new_readiness); + if new_readiness == 0 { + // Un-trigger this, there's nothing left to report here. + ready_set.remove(&key); + } else if force_edge || new_readiness != prev_readiness & new_readiness { + // Either we force an "edge" to be detected, or there's a bit set in `new` + // that was not set in `prev`. In both cases, this is ready now. + ready_set.insert(key); + ecx.release_clock(|clock| { + interest.clock.join(clock); + })?; } + interp_ok(()) + })?; + // While there are events ready to be delivered, wake up a thread to receive them. + while !ready_set.is_empty() + && let Some(thread_id) = epoll.queue.borrow_mut().pop_front() + { + drop(ready_set); // release the "lock" so the unblocked thread can have it + ecx.unblock_thread(thread_id, BlockReason::Epoll)?; + ready_set = epoll.ready_set.borrow_mut(); } interp_ok(()) @@ -625,28 +593,40 @@ fn return_ready_list<'tcx>( dest: &MPlaceTy<'tcx>, events: &MPlaceTy<'tcx>, ecx: &mut MiriInterpCx<'tcx>, -) -> InterpResult<'tcx> { - let mut ready_list = epfd.ready_list.borrow_mut(); +) -> InterpResult<'tcx, i32> { + let mut interest_list = epfd.interest_list.borrow_mut(); + let mut ready_set = epfd.ready_set.borrow_mut(); let mut num_of_events: i32 = 0; let mut array_iter = ecx.project_array_fields(events)?; - while let Some(des) = array_iter.next(ecx)? { - if let Some((_, epoll_event_instance)) = ready_list.pop_first() { - ecx.write_int_fields_named( - &[ - ("events", epoll_event_instance.events.into()), - ("u64", epoll_event_instance.data.into()), - ], - &des.1, - )?; - // Synchronize waking thread with the event of interest. - ecx.acquire_clock(&epoll_event_instance.clock)?; - - num_of_events = num_of_events.strict_add(1); - } else { - break; + // Sanity-check to ensure that all event info is up-to-date. + if cfg!(debug_assertions) { + for (key, interest) in interest_list.iter() { + // Ensure this matches the latest readiness of this FD. + // We have to do an FD lookup by ID for this. The FdNum might be already closed. + let fd = &ecx.machine.fds.fds.values().find(|fd| fd.id() == key.0).unwrap(); + let current_active = fd.as_unix(ecx).epoll_active_events()?.get_event_bitmask(ecx); + assert_eq!(interest.active_events, current_active & interest.relevant_events); } } + + // While there is a slot to store another event, and an event to store, deliver that event. + while let Some(slot) = array_iter.next(ecx)? + && let Some(&key) = ready_set.first() + { + let interest = interest_list.get_mut(&key).expect("non-existent event in ready set"); + // Deliver event to caller. + ecx.write_int_fields_named( + &[("events", interest.active_events.into()), ("u64", interest.data.into())], + &slot.1, + )?; + num_of_events = num_of_events.strict_add(1); + // Synchronize receiving thread with the event of interest. + ecx.acquire_clock(&interest.clock)?; + // Since currently, all events are edge-triggered, we remove them from the ready set when + // they get delivered. + ready_set.remove(&key); + } ecx.write_int(num_of_events, dest)?; - interp_ok(()) + interp_ok(num_of_events) } diff --git a/src/tools/miri/src/shims/unix/linux_like/eventfd.rs b/src/tools/miri/src/shims/unix/linux_like/eventfd.rs index 1b5f1db43953b..d374a1e75f72e 100644 --- a/src/tools/miri/src/shims/unix/linux_like/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux_like/eventfd.rs @@ -6,7 +6,7 @@ use std::io::ErrorKind; use crate::concurrency::VClock; use crate::shims::files::{FdId, FileDescription, FileDescriptionRef, WeakFileDescriptionRef}; use crate::shims::unix::UnixFileDescription; -use crate::shims::unix::linux_like::epoll::{EpollReadyEvents, EvalContextExt as _}; +use crate::shims::unix::linux_like::epoll::{EpollEvents, EvalContextExt as _}; use crate::*; /// Maximum value that the eventfd counter can hold. @@ -107,14 +107,14 @@ impl FileDescription for EventFd { } impl UnixFileDescription for EventFd { - fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + fn epoll_active_events<'tcx>(&self) -> InterpResult<'tcx, EpollEvents> { // We only check the status of EPOLLIN and EPOLLOUT flags for eventfd. If other event flags // need to be supported in the future, the check should be added here. - interp_ok(EpollReadyEvents { + interp_ok(EpollEvents { epollin: self.counter.get() != 0, epollout: self.counter.get() != MAX_COUNTER, - ..EpollReadyEvents::new() + ..EpollEvents::new() }) } } @@ -220,7 +220,7 @@ fn eventfd_write<'tcx>( // Linux seems to cause spurious wakeups here, and Tokio seems to rely on that // (see // and also ). - ecx.epoll_send_fd_ready_events(eventfd, /* force_edge */ true)?; + ecx.update_epoll_active_events(eventfd, /* force_edge */ true)?; // Return how many bytes we consumed from the user-provided buffer. return finish.call(ecx, Ok(buf_place.layout.size.bytes_usize())); @@ -316,7 +316,7 @@ fn eventfd_read<'tcx>( // The state changed; we check and update the status of all supported event // types for current file description. // Linux seems to always emit do notifications here, even if we were already writable. - ecx.epoll_send_fd_ready_events(eventfd, /* force_edge */ true)?; + ecx.update_epoll_active_events(eventfd, /* force_edge */ true)?; // Tell userspace how many bytes we put into the buffer. return finish.call(ecx, Ok(buf_place.layout.size.bytes_usize())); diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index d69d373b572bd..be32ca9abd597 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -20,8 +20,14 @@ use crate::*; #[derive(Clone)] enum MacOsUnfairLock { - Active { mutex_ref: MutexRef }, - PermanentlyLocked, + Active { + mutex_ref: MutexRef, + }, + /// If a lock gets copied while being held, we put it in this state. + /// It seems like in the real implementation, the lock actually remembers who held it, + /// and still behaves as-if it was held by that thread in the new location. In Miri, we don't + /// know who actually owns this lock at the moment. + PermanentlyLockedByUnknown, } impl SyncObj for MacOsUnfairLock { @@ -93,10 +99,12 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // locks when they get released, so it got copied while locked. Unfortunately // that is something `std` needs to support (the guard could have been leaked). // On the plus side, we know nobody was queued for the lock while it got copied; - // that would have been rejected by our `on_access`. So we behave like a - // futex-based lock would in this case: any attempt to acquire the lock will - // just wait forever, since there's nobody to wake us up. - interp_ok(MacOsUnfairLock::PermanentlyLocked) + // that would have been rejected by our `on_access`. + // The real implementation would apparently remember who held the old lock, and + // consider them to hold the copy as well -- but our copies don't preserve sync + // object metadata so we instead move the lock into a "permanently locked" + // state. + interp_ok(MacOsUnfairLock::PermanentlyLockedByUnknown) } else { throw_ub_format!("`os_unfair_lock` was not properly initialized at this location, or it got overwritten"); } @@ -303,18 +311,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // Trying to get a poisoned lock. Just block forever... - this.block_thread( - BlockReason::Sleep, - None, - callback!( - @capture<'tcx> {} - |_this, _unblock: UnblockKind| { - panic!("we shouldn't wake up ever") - } - ), + // Trying to lock a perma-locked lock. On macOS this would block or abort depending + // on whether the current thread is considered to be the one holding this lock. We + // don't know who is considered to be holding the lock so we don't know what to do. + throw_unsup_format!( + "attempted to lock an os_unfair_lock that was copied while being locked" ); - return interp_ok(()); }; let mutex_ref = mutex_ref.clone(); @@ -342,15 +344,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // Trying to get a poisoned lock. That never works. + // Trying to lock a perma-locked lock. That behaves the same no matter who the owner is + // so we can implement the real behavior here. this.write_scalar(Scalar::from_bool(false), dest)?; return interp_ok(()); }; let mutex_ref = mutex_ref.clone(); if mutex_ref.owner().is_some() { - // Contrary to the blocking lock function, this does not check for - // reentrancy. + // Contrary to the blocking lock function, this does not check for reentrancy. this.write_scalar(Scalar::from_bool(false), dest)?; } else { this.mutex_lock(&mutex_ref)?; @@ -364,10 +366,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // A perma-locked lock is definitely not held by us. - throw_machine_stop!(TerminationInfo::Abort( - "attempted to unlock an os_unfair_lock not owned by the current thread".to_owned() - )); + // We don't know who the owner is so we cannot proceed. + throw_unsup_format!( + "attempted to unlock an os_unfair_lock that was copied while being locked" + ); }; let mutex_ref = mutex_ref.clone(); @@ -393,10 +395,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // A perma-locked lock is definitely not held by us. - throw_machine_stop!(TerminationInfo::Abort( - "called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread".to_owned() - )); + // We don't know who the owner is so we cannot proceed. + throw_unsup_format!( + "attempted to assert the owner of an os_unfair_lock that was copied while being locked" + ); }; let mutex_ref = mutex_ref.clone(); @@ -415,8 +417,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // A perma-locked lock is definitely not held by us. - return interp_ok(()); + // We don't know who the owner is so we cannot proceed. + throw_unsup_format!( + "attempted to assert the owner of an os_unfair_lock that was copied while being locked" + ); }; let mutex_ref = mutex_ref.clone(); diff --git a/src/tools/miri/src/shims/unix/unnamed_socket.rs b/src/tools/miri/src/shims/unix/unnamed_socket.rs index 9126090d80ba3..a320ac1316891 100644 --- a/src/tools/miri/src/shims/unix/unnamed_socket.rs +++ b/src/tools/miri/src/shims/unix/unnamed_socket.rs @@ -14,7 +14,7 @@ use crate::shims::files::{ EvalContextExt as _, FdId, FileDescription, FileDescriptionRef, WeakFileDescriptionRef, }; use crate::shims::unix::UnixFileDescription; -use crate::shims::unix::linux_like::epoll::{EpollReadyEvents, EvalContextExt as _}; +use crate::shims::unix::linux_like::epoll::{EpollEvents, EvalContextExt as _}; use crate::*; /// The maximum capacity of the socketpair buffer in bytes. @@ -99,7 +99,7 @@ impl FileDescription for AnonSocket { } } // Notify peer fd that close has happened, since that can unblock reads and writes. - ecx.epoll_send_fd_ready_events(peer_fd, /* force_edge */ false)?; + ecx.update_epoll_active_events(peer_fd, /* force_edge */ false)?; } interp_ok(Ok(())) } @@ -280,8 +280,8 @@ fn anonsocket_write<'tcx>( // Notify epoll waiters: we might be no longer writable, peer might now be readable. // The notification to the peer seems to be always sent on Linux, even if the // FD was readable before. - ecx.epoll_send_fd_ready_events(self_ref, /* force_edge */ false)?; - ecx.epoll_send_fd_ready_events(peer_fd, /* force_edge */ true)?; + ecx.update_epoll_active_events(self_ref, /* force_edge */ false)?; + ecx.update_epoll_active_events(peer_fd, /* force_edge */ true)?; return finish.call(ecx, Ok(write_size)); } @@ -378,10 +378,10 @@ fn anonsocket_read<'tcx>( // Linux seems to always notify the peer if the read buffer is now empty. // (Linux also does that if this was a "big" read, but to avoid some arbitrary // threshold, we do not match that.) - ecx.epoll_send_fd_ready_events(peer_fd, /* force_edge */ readbuf_now_empty)?; + ecx.update_epoll_active_events(peer_fd, /* force_edge */ readbuf_now_empty)?; }; // Notify epoll waiters: we might be no longer readable. - ecx.epoll_send_fd_ready_events(self_ref, /* force_edge */ false)?; + ecx.update_epoll_active_events(self_ref, /* force_edge */ false)?; return finish.call(ecx, Ok(read_size)); } @@ -389,11 +389,11 @@ fn anonsocket_read<'tcx>( } impl UnixFileDescription for AnonSocket { - fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + fn epoll_active_events<'tcx>(&self) -> InterpResult<'tcx, EpollEvents> { // We only check the status of EPOLLIN, EPOLLOUT, EPOLLHUP and EPOLLRDHUP flags. // If other event flags need to be supported in the future, the check should be added here. - let mut epoll_ready_events = EpollReadyEvents::new(); + let mut epoll_ready_events = EpollEvents::new(); // Check if it is readable. if let Some(readbuf) = &self.readbuf { diff --git a/src/tools/miri/src/shims/x86/avx2.rs b/src/tools/miri/src/shims/x86/avx2.rs index 01e1ac6de59d3..142258c6975df 100644 --- a/src/tools/miri/src/shims/x86/avx2.rs +++ b/src/tools/miri/src/shims/x86/avx2.rs @@ -6,7 +6,7 @@ use rustc_target::callconv::FnAbi; use super::{ ShiftOp, horizontal_bin_op, mask_load, mask_store, mpsadbw, packssdw, packsswb, packusdw, - packuswb, pmulhrsw, psign, shift_simd_by_scalar, shift_simd_by_simd, + packuswb, pmulhrsw, psadbw, psign, shift_simd_by_scalar, shift_simd_by_simd, }; use crate::*; @@ -241,41 +241,11 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } // Used to implement the _mm256_sad_epu8 function. - // Compute the absolute differences of packed unsigned 8-bit integers - // in `left` and `right`, then horizontally sum each consecutive 8 - // differences to produce four unsigned 16-bit integers, and pack - // these unsigned 16-bit integers in the low 16 bits of 64-bit elements - // in `dest`. - // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm256_sad_epu8 "psad.bw" => { let [left, right] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let (left, left_len) = this.project_to_simd(left)?; - let (right, right_len) = this.project_to_simd(right)?; - let (dest, dest_len) = this.project_to_simd(dest)?; - - assert_eq!(left_len, right_len); - assert_eq!(left_len, dest_len.strict_mul(8)); - - for i in 0..dest_len { - let dest = this.project_index(&dest, i)?; - - let mut acc: u16 = 0; - for j in 0..8 { - let src_index = i.strict_mul(8).strict_add(j); - - let left = this.project_index(&left, src_index)?; - let left = this.read_scalar(&left)?.to_u8()?; - - let right = this.project_index(&right, src_index)?; - let right = this.read_scalar(&right)?.to_u8()?; - - acc = acc.strict_add(left.abs_diff(right).into()); - } - - this.write_scalar(Scalar::from_u64(acc.into()), &dest)?; - } + psadbw(this, left, right, dest)? } // Used to implement the _mm256_shuffle_epi8 intrinsic. // Shuffles bytes from `left` using `right` as pattern. diff --git a/src/tools/miri/src/shims/x86/avx512.rs b/src/tools/miri/src/shims/x86/avx512.rs index e15b99beba8b4..4957b3b88cf6c 100644 --- a/src/tools/miri/src/shims/x86/avx512.rs +++ b/src/tools/miri/src/shims/x86/avx512.rs @@ -3,6 +3,7 @@ use rustc_middle::ty::Ty; use rustc_span::Symbol; use rustc_target::callconv::FnAbi; +use super::psadbw; use crate::*; impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -78,6 +79,15 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(Scalar::from_u32(r), &d_lane)?; } } + // Used to implement the _mm512_sad_epu8 function. + "psad.bw.512" => { + this.expect_target_feature_for_intrinsic(link_name, "avx512bw")?; + + let [left, right] = + this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; + + psadbw(this, left, right, dest)? + } _ => return interp_ok(EmulateItemResult::NotSupported), } interp_ok(EmulateItemResult::NeedsReturn) diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index c730609a4a57f..258ad9f8de28a 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -1038,6 +1038,54 @@ fn mpsadbw<'tcx>( interp_ok(()) } +/// Compute the absolute differences of packed unsigned 8-bit integers +/// in `left` and `right`, then horizontally sum each consecutive 8 +/// differences to produce unsigned 16-bit integers, and pack +/// these unsigned 16-bit integers in the low 16 bits of 64-bit elements +/// in `dest`. +/// +/// +/// +/// +fn psadbw<'tcx>( + ecx: &mut crate::MiriInterpCx<'tcx>, + left: &OpTy<'tcx>, + right: &OpTy<'tcx>, + dest: &MPlaceTy<'tcx>, +) -> InterpResult<'tcx, ()> { + let (left, left_len) = ecx.project_to_simd(left)?; + let (right, right_len) = ecx.project_to_simd(right)?; + let (dest, dest_len) = ecx.project_to_simd(dest)?; + + // fn psadbw(a: u8x16, b: u8x16) -> u64x2; + // fn psadbw(a: u8x32, b: u8x32) -> u64x4; + // fn vpsadbw(a: u8x64, b: u8x64) -> u64x8; + assert_eq!(left_len, right_len); + assert_eq!(left_len, left.layout.layout.size().bytes()); + assert_eq!(dest_len, left_len.strict_div(8)); + + for i in 0..dest_len { + let dest = ecx.project_index(&dest, i)?; + + let mut acc: u16 = 0; + for j in 0..8 { + let src_index = i.strict_mul(8).strict_add(j); + + let left = ecx.project_index(&left, src_index)?; + let left = ecx.read_scalar(&left)?.to_u8()?; + + let right = ecx.project_index(&right, src_index)?; + let right = ecx.read_scalar(&right)?.to_u8()?; + + acc = acc.strict_add(left.abs_diff(right).into()); + } + + ecx.write_scalar(Scalar::from_u64(acc.into()), &dest)?; + } + + interp_ok(()) +} + /// Multiplies packed 16-bit signed integer values, truncates the 32-bit /// product to the 18 most significant bits by right-shifting, and then /// divides the 18-bit value by 2 (rounding to nearest) by first adding diff --git a/src/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index 9af7f05d3b27e..8389813903073 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/src/shims/x86/sse2.rs @@ -6,7 +6,7 @@ use rustc_target::callconv::FnAbi; use super::{ FloatBinOp, ShiftOp, bin_op_simd_float_all, bin_op_simd_float_first, convert_float_to_int, - packssdw, packsswb, packuswb, shift_simd_by_scalar, + packssdw, packsswb, packuswb, psadbw, shift_simd_by_scalar, }; use crate::*; @@ -37,41 +37,11 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // vectors. match unprefixed_name { // Used to implement the _mm_sad_epu8 function. - // Computes the absolute differences of packed unsigned 8-bit integers in `a` - // and `b`, then horizontally sum each consecutive 8 differences to produce - // two unsigned 16-bit integers, and pack these unsigned 16-bit integers in - // the low 16 bits of 64-bit elements returned. - // - // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm_sad_epu8 "psad.bw" => { let [left, right] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - let (left, left_len) = this.project_to_simd(left)?; - let (right, right_len) = this.project_to_simd(right)?; - let (dest, dest_len) = this.project_to_simd(dest)?; - - // left and right are u8x16, dest is u64x2 - assert_eq!(left_len, right_len); - assert_eq!(left_len, 16); - assert_eq!(dest_len, 2); - - for i in 0..dest_len { - let dest = this.project_index(&dest, i)?; - - let mut res: u16 = 0; - let n = left_len.strict_div(dest_len); - for j in 0..n { - let op_i = j.strict_add(i.strict_mul(n)); - let left = this.read_scalar(&this.project_index(&left, op_i)?)?.to_u8()?; - let right = - this.read_scalar(&this.project_index(&right, op_i)?)?.to_u8()?; - - res = res.strict_add(left.abs_diff(right).into()); - } - - this.write_scalar(Scalar::from_u64(res.into()), &dest)?; - } + psadbw(this, left, right, dest)? } // Used to implement the _mm_{sll,srl,sra}_epi{16,32,64} functions // (except _mm_sra_epi64, which is not available in SSE2). diff --git a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs index 8406143933458..b776badceecb3 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs @@ -9,5 +9,5 @@ fn main() { let lock = lock; // This needs to either error or deadlock. unsafe { libc::os_unfair_lock_lock(lock.get()) }; - //~^ error: deadlock + //~^ error: lock an os_unfair_lock that was copied while being locked } diff --git a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr index 8e7364226eccd..fdf19a445ee74 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr +++ b/src/tools/miri/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr @@ -1,8 +1,10 @@ -error: the evaluated program deadlocked +error: unsupported operation: attempted to lock an os_unfair_lock that was copied while being locked --> tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs:LL:CC | LL | unsafe { libc::os_unfair_lock_lock(lock.get()) }; - | ^ this thread got stuck here + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.rs b/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.rs index 9dc554030c0e1..98cc80b6b4ea2 100644 --- a/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.rs +++ b/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.rs @@ -1,6 +1,4 @@ //@only-target: linux android illumos -//~^ERROR: deadlocked -//~^^ERROR: deadlocked //@compile-flags: -Zmiri-deterministic-concurrency //@error-in-other-file: deadlock diff --git a/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.stderr b/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.stderr index 4730045cfe33b..a7dfa0b6ea650 100644 --- a/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.stderr +++ b/src/tools/miri/tests/fail-dep/libc/eventfd_block_read_twice.stderr @@ -14,24 +14,13 @@ note: inside `main` LL | thread2.join().unwrap(); | ^^^^^^^^^^^^^^ -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - error: the evaluated program deadlocked --> tests/fail-dep/libc/eventfd_block_read_twice.rs:LL:CC | LL | let res: i64 = unsafe { libc::read(fd, buf.as_mut_ptr().cast(), 8).try_into().unwrap() }; | ^ this thread got stuck here -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.rs b/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.rs index 5297a3297750a..1a1d76eda2003 100644 --- a/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.rs +++ b/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.rs @@ -1,6 +1,4 @@ //@only-target: linux android illumos -//~^ERROR: deadlocked -//~^^ERROR: deadlocked //@compile-flags: -Zmiri-deterministic-concurrency //@error-in-other-file: deadlock @@ -38,7 +36,7 @@ fn main() { let thread2 = thread::spawn(move || { let sized_8_data = (u64::MAX - 1).to_ne_bytes(); - // Write u64::MAX - 1, so the all subsequent write will block. + // Write u64::MAX - 1, so that all subsequent writes will block. let res: i64 = unsafe { // This `write` will initially blocked, then get unblocked by thread3, then get blocked again // because the `write` in thread1 executes first. diff --git a/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.stderr b/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.stderr index c0e38df98d485..0e554598ecf56 100644 --- a/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.stderr +++ b/src/tools/miri/tests/fail-dep/libc/eventfd_block_write_twice.stderr @@ -14,24 +14,13 @@ note: inside `main` LL | thread2.join().unwrap(); | ^^^^^^^^^^^^^^ -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - error: the evaluated program deadlocked --> tests/fail-dep/libc/eventfd_block_write_twice.rs:LL:CC | LL | libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8).try_into().unwrap() | ^ this thread got stuck here -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.rs b/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.rs index 054cb812d9e84..8c51416f5aa93 100644 --- a/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.rs +++ b/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.rs @@ -1,11 +1,12 @@ //@compile-flags: -Zmiri-deterministic-concurrency -//~^ERROR: deadlocked -//~^^ERROR: deadlocked //@only-target: linux android illumos //@error-in-other-file: deadlock use std::convert::TryInto; -use std::thread::spawn; +use std::thread; + +#[path = "../../utils/libc.rs"] +mod libc_utils; // Using `as` cast since `EPOLLET` wraps around const EPOLL_IN_OUT_ET: u32 = (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _; @@ -49,39 +50,37 @@ fn main() { let epfd = unsafe { libc::epoll_create1(0) }; assert_ne!(epfd, -1); - // Create a socketpair instance. - let mut fds = [-1, -1]; - let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; - assert_eq!(res, 0); + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd1 = unsafe { libc::eventfd(0, flags) }; + // Make a duplicate so that we have two file descriptors for the same file description. + let fd2 = unsafe { libc::dup(fd1) }; - // Register one side of the socketpair with epoll. - let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; - let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + // Register both with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd1 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd1, &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd2 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd2, &mut ev) }; assert_eq!(res, 0); - // epoll_wait to clear notification. - let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); - let expected_value = fds[0] as u64; - check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], 0); + // Consume the initial events. + let expected = [(libc::EPOLLOUT as u32, fd1 as u64), (libc::EPOLLOUT as u32, fd2 as u64)]; + check_epoll_wait::<8>(epfd, &expected, -1); - let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); - let expected_value = fds[0] as u64; - let thread1 = spawn(move || { - check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], -1); - //~^ERROR: deadlocked + let thread1 = thread::spawn(move || { + check_epoll_wait::<2>(epfd, &expected, -1); }); - let thread2 = spawn(move || { - check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)], -1); + let thread2 = thread::spawn(move || { + check_epoll_wait::<2>(epfd, &expected, -1); + //~^ERROR: deadlocked }); + // Yield so the threads are both blocked. + thread::yield_now(); - let thread3 = spawn(move || { - // Just a single write, so we only wake up one of them. - let data = "abcde".as_bytes().as_ptr(); - let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; - assert!(res > 0 && res <= 5); - }); + // Create two events at once. + libc_utils::write_all_from_slice(fd1, &0_u64.to_ne_bytes()).unwrap(); thread1.join().unwrap(); thread2.join().unwrap(); - thread3.join().unwrap(); } diff --git a/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.stderr b/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.stderr index c3cd04c0b5e6f..6740faedb3e9d 100644 --- a/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.stderr +++ b/src/tools/miri/tests/fail-dep/libc/libc_epoll_block_two_thread.stderr @@ -1,8 +1,3 @@ -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - error: the evaluated program deadlocked --> RUSTLIB/std/src/sys/thread/PLATFORM.rs:LL:CC | @@ -16,22 +11,16 @@ LL | let ret = unsafe { libc::pthread_join(id, ptr::null_mut()) }; note: inside `main` --> tests/fail-dep/libc/libc_epoll_block_two_thread.rs:LL:CC | -LL | thread1.join().unwrap(); +LL | thread2.join().unwrap(); | ^^^^^^^^^^^^^^ error: the evaluated program deadlocked --> tests/fail-dep/libc/libc_epoll_block_two_thread.rs:LL:CC | -LL | check_epoll_wait::(epfd, &[(expected_event, expected_value)], -1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this thread got stuck here - -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +LL | check_epoll_wait::(epfd, &expected, -1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this thread got stuck here note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.rs b/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.rs index 0699dec6556d4..a975e404a9785 100644 --- a/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.rs +++ b/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.rs @@ -1,6 +1,5 @@ //! This is a regression test for : we had some //! faulty logic around `release_clock` that led to this code not reporting a data race. -//~^^ERROR: deadlock //@ignore-target: windows # no libc socketpair on Windows //@compile-flags: -Zmiri-deterministic-concurrency //@error-in-other-file: deadlock diff --git a/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.stderr b/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.stderr index ac2346f6062f1..1e802209fd795 100644 --- a/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.stderr +++ b/src/tools/miri/tests/fail-dep/libc/socketpair-close-while-blocked.stderr @@ -20,12 +20,7 @@ error: the evaluated program deadlocked LL | libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) | ^ this thread got stuck here -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors diff --git a/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.rs b/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.rs index 0fecfb8f663f9..37aa4590647b4 100644 --- a/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.rs +++ b/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.rs @@ -1,6 +1,4 @@ //@ignore-target: windows # No libc socketpair on Windows -//~^ERROR: deadlocked -//~^^ERROR: deadlocked // test_race depends on a deterministic schedule. //@compile-flags: -Zmiri-deterministic-concurrency //@error-in-other-file: deadlock diff --git a/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.stderr b/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.stderr index c599432ff13c0..3f7bbc779609f 100644 --- a/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.stderr +++ b/src/tools/miri/tests/fail-dep/libc/socketpair_block_read_twice.stderr @@ -14,24 +14,13 @@ note: inside `main` LL | thread2.join().unwrap(); | ^^^^^^^^^^^^^^ -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - error: the evaluated program deadlocked --> tests/fail-dep/libc/socketpair_block_read_twice.rs:LL:CC | LL | libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) | ^ this thread got stuck here -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.rs b/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.rs index 048938c091e3a..34eab8aaa0462 100644 --- a/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.rs +++ b/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.rs @@ -1,6 +1,4 @@ //@ignore-target: windows # No libc socketpair on Windows -//~^ERROR: deadlocked -//~^^ERROR: deadlocked // test_race depends on a deterministic schedule. //@compile-flags: -Zmiri-deterministic-concurrency //@error-in-other-file: deadlock diff --git a/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.stderr b/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.stderr index 2230d99fc6658..b8dbb513da209 100644 --- a/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.stderr +++ b/src/tools/miri/tests/fail-dep/libc/socketpair_block_write_twice.stderr @@ -14,24 +14,13 @@ note: inside `main` LL | thread2.join().unwrap(); | ^^^^^^^^^^^^^^ -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - error: the evaluated program deadlocked --> tests/fail-dep/libc/socketpair_block_write_twice.rs:LL:CC | LL | let res = unsafe { libc::write(fds[0], data.as_ptr() as *const libc::c_void, data.len()) }; | ^ this thread got stuck here -error: the evaluated program deadlocked - | - = note: this thread got stuck here - = note: (no span available) - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/src/tools/miri/tests/fail/deny_lint.rs b/src/tools/miri/tests/fail/deny_lint.rs index f49fa49d09d5e..b71c30ceac1a6 100644 --- a/src/tools/miri/tests/fail/deny_lint.rs +++ b/src/tools/miri/tests/fail/deny_lint.rs @@ -1,5 +1,3 @@ -//@error-in-other-file: miri cannot be run on programs that fail compilation - #![deny(warnings, unused)] struct Foo; diff --git a/src/tools/miri/tests/fail/deny_lint.stderr b/src/tools/miri/tests/fail/deny_lint.stderr index fe96edf934612..053a3dec5feb0 100644 --- a/src/tools/miri/tests/fail/deny_lint.stderr +++ b/src/tools/miri/tests/fail/deny_lint.stderr @@ -11,7 +11,5 @@ LL | #![deny(warnings, unused)] | ^^^^^^ = note: `#[deny(dead_code)]` implied by `#[deny(unused)]` -error: miri cannot be run on programs that fail compilation - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs index 81d4e501c43a6..c67386b4f84cf 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll-blocking.rs @@ -4,7 +4,6 @@ use std::convert::TryInto; use std::thread; -use std::thread::spawn; #[path = "../../utils/libc.rs"] mod libc_utils; @@ -17,6 +16,7 @@ fn main() { test_notification_after_timeout(); test_epoll_race(); wakeup_on_new_interest(); + multiple_events_wake_multiple_threads(); } // Using `as` cast since `EPOLLET` wraps around @@ -90,7 +90,7 @@ fn test_epoll_block_then_unblock() { // epoll_wait before triggering notification so it will block then get unblocked before timeout. let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); let expected_value = fds[0] as u64; - let thread1 = spawn(move || { + let thread1 = thread::spawn(move || { thread::yield_now(); let data = "abcde".as_bytes().as_ptr(); let res = unsafe { libc_utils::write_all(fds[1], data as *const libc::c_void, 5) }; @@ -210,3 +210,54 @@ fn wakeup_on_new_interest() { // This should wake up the thread. t.join().unwrap(); } + +/// Ensure that if a single operation triggers multiple events, we wake up enough threads +/// to consume them all. +fn multiple_events_wake_multiple_threads() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd1 = unsafe { libc::eventfd(0, flags) }; + // Make a duplicate so that we have two file descriptors for the same file description. + let fd2 = unsafe { libc::dup(fd1) }; + + // Register both with epoll. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd1 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd1, &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd2 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd2, &mut ev) }; + assert_eq!(res, 0); + + // Consume the initial events. + let expected = [(libc::EPOLLOUT as u32, fd1 as u64), (libc::EPOLLOUT as u32, fd2 as u64)]; + check_epoll_wait::<8>(epfd, &expected, -1); + + // Block two threads on the epoll, both wanting to get just one event. + let t1 = thread::spawn(move || { + let mut e = libc::epoll_event { events: 0, u64: 0 }; + let res = unsafe { libc::epoll_wait(epfd, &raw mut e, 1, -1) }; + assert!(res == 1); + (e.events, e.u64) + }); + let t2 = thread::spawn(move || { + let mut e = libc::epoll_event { events: 0, u64: 0 }; + let res = unsafe { libc::epoll_wait(epfd, &raw mut e, 1, -1) }; + assert!(res == 1); + (e.events, e.u64) + }); + // Yield so both threads are waiting now. + thread::yield_now(); + + // Trigger the eventfd. This triggers two events at once! + libc_utils::write_all_from_slice(fd1, &0_u64.to_ne_bytes()).unwrap(); + + // Both threads should have been woken up so that both events can be consumed. + let e1 = t1.join().unwrap(); + let e2 = t2.join().unwrap(); + // Ensure that across the two threads we got both events. + assert!(expected == [e1, e2] || expected == [e2, e1]); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs index 569675a5e3c4d..2c55df853abad 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs @@ -262,10 +262,8 @@ fn test_epoll_eventfd() { let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; let fd = unsafe { libc::eventfd(0, flags) }; - // Write to the eventfd instance. - let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); - let res = unsafe { libc_utils::write_all(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; - assert_eq!(res, 8); + // Write 1 to the eventfd instance. + libc_utils::write_all_from_slice(fd, &1_u64.to_ne_bytes()).unwrap(); // Create an epoll instance. let epfd = unsafe { libc::epoll_create1(0) }; @@ -281,18 +279,15 @@ fn test_epoll_eventfd() { let expected_value = u64::try_from(fd).unwrap(); check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); - // Write to the eventfd again. - let res = unsafe { libc_utils::write_all(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; - assert_eq!(res, 8); + // Write 0 to the eventfd. + libc_utils::write_all_from_slice(fd, &0_u64.to_ne_bytes()).unwrap(); // This does not change the status, so we should get no event. // However, Linux performs a spurious wakeup. check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); // Read from the eventfd. - let mut buf = [0u8; 8]; - let res = unsafe { libc_utils::read_all(fd, buf.as_mut_ptr().cast(), 8) }; - assert_eq!(res, 8); + libc_utils::read_all_into_array::<8>(fd).unwrap(); // This consumes the event, so the read status is gone. However, deactivation // does not trigger an event. @@ -355,6 +350,7 @@ fn test_epoll_socketpair_both_sides() { // The state of fds[1] does not change (was writable, is writable). // However, we force a spurious wakeup as the read buffer just got emptied. + // fds[0] lost its readability, but becoming less active is not considered an "edge". check_epoll_wait::<8>(epfd, &[(expected_event1, expected_value1)]); } diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.rs b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.rs index cf6f43dbbfa91..a3060abc39402 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.rs +++ b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.rs @@ -1,7 +1,5 @@ //@normalize-stderr-test: "::<.*>" -> "" -#![allow(function_casts_as_integer)] - #[inline(never)] fn func_a() -> Box<[*mut ()]> { func_b::() diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stdout b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stdout index b3a4beb4e80ec..5c2995e132aa6 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stdout +++ b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stdout @@ -1,5 +1,5 @@ -tests/pass/backtrace/backtrace-api-v1.rs:29:9 (func_d) -tests/pass/backtrace/backtrace-api-v1.rs:16:9 (func_c) -tests/pass/backtrace/backtrace-api-v1.rs:11:5 (func_b::) -tests/pass/backtrace/backtrace-api-v1.rs:7:5 (func_a) -tests/pass/backtrace/backtrace-api-v1.rs:36:18 (main) +tests/pass/backtrace/backtrace-api-v1.rs:27:9 (func_d) +tests/pass/backtrace/backtrace-api-v1.rs:14:9 (func_c) +tests/pass/backtrace/backtrace-api-v1.rs:9:5 (func_b::) +tests/pass/backtrace/backtrace-api-v1.rs:5:5 (func_a) +tests/pass/backtrace/backtrace-api-v1.rs:34:18 (main) diff --git a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs index c22227a8c6599..0a9bb2d315b9b 100644 --- a/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs +++ b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx512.rs @@ -15,12 +15,48 @@ fn main() { assert!(is_x86_feature_detected!("avx512vpopcntdq")); unsafe { + test_avx512(); test_avx512bitalg(); test_avx512vpopcntdq(); test_avx512ternarylogic(); } } +#[target_feature(enable = "avx512bw")] +unsafe fn test_avx512() { + #[target_feature(enable = "avx512bw")] + unsafe fn test_mm512_sad_epu8() { + let a = _mm512_set_epi8( + 71, 70, 69, 68, 67, 66, 65, 64, // + 55, 54, 53, 52, 51, 50, 49, 48, // + 47, 46, 45, 44, 43, 42, 41, 40, // + 39, 38, 37, 36, 35, 34, 33, 32, // + 31, 30, 29, 28, 27, 26, 25, 24, // + 23, 22, 21, 20, 19, 18, 17, 16, // + 15, 14, 13, 12, 11, 10, 9, 8, // + 7, 6, 5, 4, 3, 2, 1, 0, // + ); + + // `d` is the absolute difference with the corresponding row in `a`. + let b = _mm512_set_epi8( + 63, 62, 61, 60, 59, 58, 57, 56, // lane 7 (d = 8) + 62, 61, 60, 59, 58, 57, 56, 55, // lane 6 (d = 7) + 53, 52, 51, 50, 49, 48, 47, 46, // lane 5 (d = 6) + 44, 43, 42, 41, 40, 39, 38, 37, // lane 4 (d = 5) + 35, 34, 33, 32, 31, 30, 29, 28, // lane 3 (d = 4) + 26, 25, 24, 23, 22, 21, 20, 19, // lane 2 (d = 3) + 17, 16, 15, 14, 13, 12, 11, 10, // lane 1 (d = 2) + 8, 7, 6, 5, 4, 3, 2, 1, // lane 0 (d = 1) + ); + + let r = _mm512_sad_epu8(a, b); + let e = _mm512_set_epi64(64, 56, 48, 40, 32, 24, 16, 8); + + assert_eq_m512i(r, e); + } + test_mm512_sad_epu8(); +} + // Some of the constants in the tests below are just bit patterns. They should not // be interpreted as integers; signedness does not make sense for them, but // __mXXXi happens to be defined in terms of signed integers. diff --git a/src/tools/miri/tests/pass/stacked_borrows/no_field_retagging.rs b/src/tools/miri/tests/pass/stacked_borrows/no_field_retagging.rs deleted file mode 100644 index 507df068a7e11..0000000000000 --- a/src/tools/miri/tests/pass/stacked_borrows/no_field_retagging.rs +++ /dev/null @@ -1,19 +0,0 @@ -//@compile-flags: -Zmiri-retag-fields=none - -struct Newtype<'a>(#[allow(dead_code)] &'a mut i32); - -fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { - dealloc(); -} - -// Make sure that we do *not* retag the fields of `Newtype`. -fn main() { - let ptr = Box::into_raw(Box::new(0i32)); - #[rustfmt::skip] // I like my newlines - unsafe { - dealloc_while_running( - Newtype(&mut *ptr), - || drop(Box::from_raw(ptr)), - ) - }; -} diff --git a/src/tools/miri/tests/pass/stacked_borrows/non_scalar_field_retagging.rs b/src/tools/miri/tests/pass/stacked_borrows/non_scalar_field_retagging.rs deleted file mode 100644 index 92d8f3237d013..0000000000000 --- a/src/tools/miri/tests/pass/stacked_borrows/non_scalar_field_retagging.rs +++ /dev/null @@ -1,23 +0,0 @@ -//@compile-flags: -Zmiri-retag-fields=scalar - -struct Newtype<'a>( - #[allow(dead_code)] &'a mut i32, - #[allow(dead_code)] i32, - #[allow(dead_code)] i32, -); - -fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { - dealloc(); -} - -// Make sure that with -Zmiri-retag-fields=scalar, we do *not* retag the fields of `Newtype`. -fn main() { - let ptr = Box::into_raw(Box::new(0i32)); - #[rustfmt::skip] // I like my newlines - unsafe { - dealloc_while_running( - Newtype(&mut *ptr, 0, 0), - || drop(Box::from_raw(ptr)), - ) - }; -} diff --git a/src/tools/miri/tests/utils/libc.rs b/src/tools/miri/tests/utils/libc.rs index 4757a5a268c65..ceeb840f3be5c 100644 --- a/src/tools/miri/tests/utils/libc.rs +++ b/src/tools/miri/tests/utils/libc.rs @@ -22,6 +22,18 @@ pub unsafe fn read_all( return read_so_far as libc::ssize_t; } +#[track_caller] +pub fn read_all_into_array(fd: libc::c_int) -> Result<[u8; N], libc::ssize_t> { + let mut buf = [0; N]; + let res = unsafe { read_all(fd, buf.as_mut_ptr().cast(), buf.len()) }; + if res >= 0 { + assert_eq!(res as usize, buf.len()); + Ok(buf) + } else { + Err(res) + } +} + pub unsafe fn write_all( fd: libc::c_int, buf: *const libc::c_void, @@ -39,3 +51,14 @@ pub unsafe fn write_all( } return written_so_far as libc::ssize_t; } + +#[track_caller] +pub fn write_all_from_slice(fd: libc::c_int, buf: &[u8]) -> Result<(), libc::ssize_t> { + let res = unsafe { write_all(fd, buf.as_ptr().cast(), buf.len()) }; + if res >= 0 { + assert_eq!(res as usize, buf.len()); + Ok(()) + } else { + Err(res) + } +}