From 82f2f2f488d37770b851ce8571e5b1da9da2cdd2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 19:10:33 +0100 Subject: [PATCH 01/14] simplify no-std tests set panic=abort so that we do not need this eh_personality thing --- src/tools/miri/tests/fail/alloc/no_global_allocator.rs | 6 ++---- src/tools/miri/tests/fail/panic/no_std.rs | 6 ++---- src/tools/miri/tests/pass/miri-alloc.rs | 6 ++---- src/tools/miri/tests/pass/no_std.rs | 6 ++---- src/tools/miri/tests/pass/overloaded-calls-simple.rs | 2 +- 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/tools/miri/tests/fail/alloc/no_global_allocator.rs b/src/tools/miri/tests/fail/alloc/no_global_allocator.rs index 624ad1bda582f..0952b2c46bac6 100644 --- a/src/tools/miri/tests/fail/alloc/no_global_allocator.rs +++ b/src/tools/miri/tests/fail/alloc/no_global_allocator.rs @@ -1,7 +1,8 @@ +//@compile-flags: -Cpanic=abort //@normalize-stderr-test: "OS `.*`" -> "$$OS" // Make sure we pretend the allocation symbols don't exist when there is no allocator -#![feature(lang_items, start)] +#![feature(start)] #![no_std] extern "Rust" { @@ -21,6 +22,3 @@ fn start(_: isize, _: *const *const u8) -> isize { fn panic_handler(_: &core::panic::PanicInfo) -> ! { loop {} } - -#[lang = "eh_personality"] -fn eh_personality() {} diff --git a/src/tools/miri/tests/fail/panic/no_std.rs b/src/tools/miri/tests/fail/panic/no_std.rs index 589f843cf8270..bad425804dc0a 100644 --- a/src/tools/miri/tests/fail/panic/no_std.rs +++ b/src/tools/miri/tests/fail/panic/no_std.rs @@ -1,5 +1,6 @@ -#![feature(lang_items, start, core_intrinsics)] +#![feature(start, core_intrinsics)] #![no_std] +//@compile-flags: -Cpanic=abort // windows tls dtors go through libstd right now, thus this test // cannot pass. When windows tls dtors go through the special magic // windows linker section, we can run this test on windows again. @@ -36,6 +37,3 @@ fn panic_handler(panic_info: &core::panic::PanicInfo) -> ! { writeln!(HostErr, "{panic_info}").ok(); core::intrinsics::abort(); //~ ERROR: the program aborted execution } - -#[lang = "eh_personality"] -fn eh_personality() {} diff --git a/src/tools/miri/tests/pass/miri-alloc.rs b/src/tools/miri/tests/pass/miri-alloc.rs index f6464b5bd0181..8f1724754181f 100644 --- a/src/tools/miri/tests/pass/miri-alloc.rs +++ b/src/tools/miri/tests/pass/miri-alloc.rs @@ -1,5 +1,6 @@ -#![feature(lang_items, start)] +#![feature(start)] #![no_std] +//@compile-flags: -Cpanic=abort // windows tls dtors go through libstd right now, thus this test // cannot pass. When windows tls dtors go through the special magic // windows linker section, we can run this test on windows again. @@ -24,6 +25,3 @@ fn start(_: isize, _: *const *const u8) -> isize { fn panic_handler(_: &core::panic::PanicInfo) -> ! { loop {} } - -#[lang = "eh_personality"] -fn eh_personality() {} diff --git a/src/tools/miri/tests/pass/no_std.rs b/src/tools/miri/tests/pass/no_std.rs index 3bece7783f798..3c98ee50aa9c0 100644 --- a/src/tools/miri/tests/pass/no_std.rs +++ b/src/tools/miri/tests/pass/no_std.rs @@ -1,4 +1,5 @@ -#![feature(lang_items, start)] +//@compile-flags: -Cpanic=abort +#![feature(start)] #![no_std] // Plumbing to let us use `writeln!` to host stdout: @@ -32,6 +33,3 @@ fn start(_: isize, _: *const *const u8) -> isize { fn panic_handler(_: &core::panic::PanicInfo) -> ! { loop {} } - -#[lang = "eh_personality"] -fn eh_personality() {} diff --git a/src/tools/miri/tests/pass/overloaded-calls-simple.rs b/src/tools/miri/tests/pass/overloaded-calls-simple.rs index 9fcf7d4a819a5..d13bf34fc5efd 100644 --- a/src/tools/miri/tests/pass/overloaded-calls-simple.rs +++ b/src/tools/miri/tests/pass/overloaded-calls-simple.rs @@ -1,4 +1,4 @@ -#![feature(lang_items, unboxed_closures, fn_traits)] +#![feature(unboxed_closures, fn_traits)] struct S3 { x: i32, From bfbf1d3ac41be54f236902a8afdd4a90695dc197 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 20:01:52 +0100 Subject: [PATCH 02/14] windows: remove support for slim rwlock Since https://github.com/rust-lang/rust/pull/121956 we don't need it any more, and we are generally short on Windows staff so reducing the amount of code we have to test and maintain sounds like a good idea. The InitOnce stuff is still used by `thread_local_key::StaticKey`. --- .../miri/src/shims/windows/foreign_items.rs | 45 --- src/tools/miri/src/shims/windows/sync.rs | 271 ------------------ .../concurrency/windows_condvar_shared.rs | 226 --------------- .../concurrency/windows_condvar_shared.stdout | 60 ---- 4 files changed, 602 deletions(-) delete mode 100644 src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.rs delete mode 100644 src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.stdout diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index 9e0baf86cbcf6..f942b72f0e2ae 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -297,32 +297,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } // Synchronization primitives - "AcquireSRWLockExclusive" => { - let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - this.AcquireSRWLockExclusive(ptr)?; - } - "ReleaseSRWLockExclusive" => { - let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - this.ReleaseSRWLockExclusive(ptr)?; - } - "TryAcquireSRWLockExclusive" => { - let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - let ret = this.TryAcquireSRWLockExclusive(ptr)?; - this.write_scalar(ret, dest)?; - } - "AcquireSRWLockShared" => { - let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - this.AcquireSRWLockShared(ptr)?; - } - "ReleaseSRWLockShared" => { - let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - this.ReleaseSRWLockShared(ptr)?; - } - "TryAcquireSRWLockShared" => { - let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - let ret = this.TryAcquireSRWLockShared(ptr)?; - this.write_scalar(ret, dest)?; - } "InitOnceBeginInitialize" => { let [ptr, flags, pending, context] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; @@ -335,25 +309,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.InitOnceComplete(ptr, flags, context)?; this.write_scalar(result, dest)?; } - "SleepConditionVariableSRW" => { - let [condvar, lock, timeout, flags] = - this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - - let result = this.SleepConditionVariableSRW(condvar, lock, timeout, flags, dest)?; - this.write_scalar(result, dest)?; - } - "WakeConditionVariable" => { - let [condvar] = - this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - - this.WakeConditionVariable(condvar)?; - } - "WakeAllConditionVariable" => { - let [condvar] = - this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - - this.WakeAllConditionVariable(condvar)?; - } "WaitOnAddress" => { let [ptr_op, compare_op, size_op, timeout_op] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 5edc18af482ea..f02939f888ec3 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -3,52 +3,14 @@ use std::time::Duration; use rustc_target::abi::Size; use crate::concurrency::init_once::InitOnceStatus; -use crate::concurrency::sync::{CondvarLock, RwLockMode}; use crate::concurrency::thread::MachineCallback; use crate::*; impl<'mir, 'tcx> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { - /// Try to reacquire the lock associated with the condition variable after we - /// were signaled. - fn reacquire_cond_lock( - &mut self, - thread: ThreadId, - lock: RwLockId, - mode: RwLockMode, - ) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - this.unblock_thread(thread); - - match mode { - RwLockMode::Read => - if this.rwlock_is_write_locked(lock) { - this.rwlock_enqueue_and_block_reader(lock, thread); - } else { - this.rwlock_reader_lock(lock, thread); - }, - RwLockMode::Write => - if this.rwlock_is_locked(lock) { - this.rwlock_enqueue_and_block_writer(lock, thread); - } else { - this.rwlock_writer_lock(lock, thread); - }, - } - - Ok(()) - } - // Windows sync primitives are pointer sized. // We only use the first 4 bytes for the id. - fn srwlock_get_id( - &mut self, - rwlock_op: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, RwLockId> { - let this = self.eval_context_mut(); - this.rwlock_get_or_create_id(rwlock_op, this.windows_ty_layout("SRWLOCK"), 0) - } - fn init_once_get_id( &mut self, init_once_op: &OpTy<'tcx, Provenance>, @@ -56,117 +18,11 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); this.init_once_get_or_create_id(init_once_op, this.windows_ty_layout("INIT_ONCE"), 0) } - - fn condvar_get_id( - &mut self, - condvar_op: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, CondvarId> { - let this = self.eval_context_mut(); - this.condvar_get_or_create_id(condvar_op, this.windows_ty_layout("CONDITION_VARIABLE"), 0) - } } impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} #[allow(non_snake_case)] pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { - fn AcquireSRWLockExclusive(&mut self, lock_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let id = this.srwlock_get_id(lock_op)?; - let active_thread = this.get_active_thread(); - - if this.rwlock_is_locked(id) { - // Note: this will deadlock if the lock is already locked by this - // thread in any way. - // - // FIXME: Detect and report the deadlock proactively. (We currently - // report the deadlock only when no thread can continue execution, - // but we could detect that this lock is already locked and report - // an error.) - this.rwlock_enqueue_and_block_writer(id, active_thread); - } else { - this.rwlock_writer_lock(id, active_thread); - } - - Ok(()) - } - - fn TryAcquireSRWLockExclusive( - &mut self, - lock_op: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - let id = this.srwlock_get_id(lock_op)?; - let active_thread = this.get_active_thread(); - - if this.rwlock_is_locked(id) { - // Lock is already held. - Ok(Scalar::from_u8(0)) - } else { - this.rwlock_writer_lock(id, active_thread); - Ok(Scalar::from_u8(1)) - } - } - - fn ReleaseSRWLockExclusive(&mut self, lock_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let id = this.srwlock_get_id(lock_op)?; - let active_thread = this.get_active_thread(); - - if !this.rwlock_writer_unlock(id, active_thread) { - // The docs do not say anything about this case, but it seems better to not allow it. - throw_ub_format!( - "calling ReleaseSRWLockExclusive on an SRWLock that is not exclusively locked by the current thread" - ); - } - - Ok(()) - } - - fn AcquireSRWLockShared(&mut self, lock_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let id = this.srwlock_get_id(lock_op)?; - let active_thread = this.get_active_thread(); - - if this.rwlock_is_write_locked(id) { - this.rwlock_enqueue_and_block_reader(id, active_thread); - } else { - this.rwlock_reader_lock(id, active_thread); - } - - Ok(()) - } - - fn TryAcquireSRWLockShared( - &mut self, - lock_op: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - let id = this.srwlock_get_id(lock_op)?; - let active_thread = this.get_active_thread(); - - if this.rwlock_is_write_locked(id) { - Ok(Scalar::from_u8(0)) - } else { - this.rwlock_reader_lock(id, active_thread); - Ok(Scalar::from_u8(1)) - } - } - - fn ReleaseSRWLockShared(&mut self, lock_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let id = this.srwlock_get_id(lock_op)?; - let active_thread = this.get_active_thread(); - - if !this.rwlock_reader_unlock(id, active_thread) { - // The docs do not say anything about this case, but it seems better to not allow it. - throw_ub_format!( - "calling ReleaseSRWLockShared on an SRWLock that is not locked by the current thread" - ); - } - - Ok(()) - } - fn InitOnceBeginInitialize( &mut self, init_once_op: &OpTy<'tcx, Provenance>, @@ -399,131 +255,4 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(()) } - - fn SleepConditionVariableSRW( - &mut self, - condvar_op: &OpTy<'tcx, Provenance>, - lock_op: &OpTy<'tcx, Provenance>, - timeout_op: &OpTy<'tcx, Provenance>, - flags_op: &OpTy<'tcx, Provenance>, - dest: &MPlaceTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let condvar_id = this.condvar_get_id(condvar_op)?; - let lock_id = this.srwlock_get_id(lock_op)?; - let timeout_ms = this.read_scalar(timeout_op)?.to_u32()?; - let flags = this.read_scalar(flags_op)?.to_u32()?; - - let timeout_time = if timeout_ms == this.eval_windows_u32("c", "INFINITE") { - None - } else { - let duration = Duration::from_millis(timeout_ms.into()); - Some(this.machine.clock.now().checked_add(duration).unwrap()) - }; - - let shared_mode = 0x1; // CONDITION_VARIABLE_LOCKMODE_SHARED is not in std - let mode = if flags == 0 { - RwLockMode::Write - } else if flags == shared_mode { - RwLockMode::Read - } else { - throw_unsup_format!("unsupported `Flags` {flags} in `SleepConditionVariableSRW`"); - }; - - let active_thread = this.get_active_thread(); - - let was_locked = match mode { - RwLockMode::Read => this.rwlock_reader_unlock(lock_id, active_thread), - RwLockMode::Write => this.rwlock_writer_unlock(lock_id, active_thread), - }; - - if !was_locked { - throw_ub_format!( - "calling SleepConditionVariableSRW with an SRWLock that is not locked by the current thread" - ); - } - - this.block_thread(active_thread); - this.condvar_wait(condvar_id, active_thread, CondvarLock::RwLock { id: lock_id, mode }); - - if let Some(timeout_time) = timeout_time { - struct Callback<'tcx> { - thread: ThreadId, - condvar_id: CondvarId, - lock_id: RwLockId, - mode: RwLockMode, - dest: MPlaceTy<'tcx, Provenance>, - } - - impl<'tcx> VisitProvenance for Callback<'tcx> { - fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let Callback { thread: _, condvar_id: _, lock_id: _, mode: _, dest } = self; - dest.visit_provenance(visit); - } - } - - impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback<'tcx> { - fn call(&self, this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> { - this.reacquire_cond_lock(self.thread, self.lock_id, self.mode)?; - - this.condvar_remove_waiter(self.condvar_id, self.thread); - - let error_timeout = this.eval_windows("c", "ERROR_TIMEOUT"); - this.set_last_error(error_timeout)?; - this.write_scalar(this.eval_windows("c", "FALSE"), &self.dest)?; - Ok(()) - } - } - - this.register_timeout_callback( - active_thread, - Time::Monotonic(timeout_time), - Box::new(Callback { - thread: active_thread, - condvar_id, - lock_id, - mode, - dest: dest.clone(), - }), - ); - } - - Ok(this.eval_windows("c", "TRUE")) - } - - fn WakeConditionVariable(&mut self, condvar_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let condvar_id = this.condvar_get_id(condvar_op)?; - - if let Some((thread, lock)) = this.condvar_signal(condvar_id) { - if let CondvarLock::RwLock { id, mode } = lock { - this.reacquire_cond_lock(thread, id, mode)?; - this.unregister_timeout_callback_if_exists(thread); - } else { - panic!("mutexes should not exist on windows"); - } - } - - Ok(()) - } - - fn WakeAllConditionVariable( - &mut self, - condvar_op: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let condvar_id = this.condvar_get_id(condvar_op)?; - - while let Some((thread, lock)) = this.condvar_signal(condvar_id) { - if let CondvarLock::RwLock { id, mode } = lock { - this.reacquire_cond_lock(thread, id, mode)?; - this.unregister_timeout_callback_if_exists(thread); - } else { - panic!("mutexes should not exist on windows"); - } - } - - Ok(()) - } } diff --git a/src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.rs b/src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.rs deleted file mode 100644 index 5bff9098a58bd..0000000000000 --- a/src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.rs +++ /dev/null @@ -1,226 +0,0 @@ -//@only-target-windows: Uses win32 api functions -// We are making scheduler assumptions here. -//@compile-flags: -Zmiri-preemption-rate=0 - -use std::ptr::null_mut; -use std::thread; - -use windows_sys::Win32::System::Threading::{ - AcquireSRWLockExclusive, AcquireSRWLockShared, ReleaseSRWLockExclusive, ReleaseSRWLockShared, - SleepConditionVariableSRW, WakeAllConditionVariable, CONDITION_VARIABLE, - CONDITION_VARIABLE_LOCKMODE_SHARED, INFINITE, SRWLOCK, -}; - -// not in windows-sys -const SRWLOCK_INIT: SRWLOCK = SRWLOCK { Ptr: null_mut() }; -const CONDITION_VARIABLE_INIT: CONDITION_VARIABLE = CONDITION_VARIABLE { Ptr: null_mut() }; - -#[derive(Copy, Clone)] -struct SendPtr(*mut T); - -unsafe impl Send for SendPtr {} - -/// threads should be able to reacquire the lock while it is locked by multiple other threads in shared mode -fn all_shared() { - println!("all_shared"); - - let mut lock = SRWLOCK_INIT; - let mut condvar = CONDITION_VARIABLE_INIT; - - let lock_ptr = SendPtr(&mut lock); - let condvar_ptr = SendPtr(&mut condvar); - - let mut handles = Vec::with_capacity(10); - - // waiters - for i in 0..5 { - handles.push(thread::spawn(move || { - let condvar_ptr = condvar_ptr; // avoid field capture - let lock_ptr = lock_ptr; // avoid field capture - unsafe { - AcquireSRWLockShared(lock_ptr.0); - } - println!("exclusive waiter {i} locked"); - - let r = unsafe { - SleepConditionVariableSRW( - condvar_ptr.0, - lock_ptr.0, - INFINITE, - CONDITION_VARIABLE_LOCKMODE_SHARED, - ) - }; - assert_ne!(r, 0); - - println!("exclusive waiter {i} reacquired lock"); - - // unlocking is unnecessary because the lock is never used again - })); - } - - // ensures each waiter is waiting by this point - thread::yield_now(); - - // readers - for i in 0..5 { - handles.push(thread::spawn(move || { - let lock_ptr = lock_ptr; // avoid field capture - unsafe { - AcquireSRWLockShared(lock_ptr.0); - } - println!("reader {i} locked"); - - // switch to next reader or main thread - thread::yield_now(); - - unsafe { - ReleaseSRWLockShared(lock_ptr.0); - } - println!("reader {i} unlocked"); - })); - } - - // ensures each reader has acquired the lock - thread::yield_now(); - - unsafe { - WakeAllConditionVariable(condvar_ptr.0); - } - - for handle in handles { - handle.join().unwrap(); - } -} - -// reacquiring a lock should wait until the lock is not exclusively locked -fn shared_sleep_and_exclusive_lock() { - println!("shared_sleep_and_exclusive_lock"); - - let mut lock = SRWLOCK_INIT; - let mut condvar = CONDITION_VARIABLE_INIT; - - let lock_ptr = SendPtr(&mut lock); - let condvar_ptr = SendPtr(&mut condvar); - - let mut waiters = Vec::with_capacity(5); - for i in 0..5 { - waiters.push(thread::spawn(move || { - let lock_ptr = lock_ptr; // avoid field capture - let condvar_ptr = condvar_ptr; // avoid field capture - unsafe { - AcquireSRWLockShared(lock_ptr.0); - } - println!("shared waiter {i} locked"); - - let r = unsafe { - SleepConditionVariableSRW( - condvar_ptr.0, - lock_ptr.0, - INFINITE, - CONDITION_VARIABLE_LOCKMODE_SHARED, - ) - }; - assert_ne!(r, 0); - - println!("shared waiter {i} reacquired lock"); - - // unlocking is unnecessary because the lock is never used again - })); - } - - // ensures each waiter is waiting by this point - thread::yield_now(); - - unsafe { - AcquireSRWLockExclusive(lock_ptr.0); - } - println!("main locked"); - - unsafe { - WakeAllConditionVariable(condvar_ptr.0); - } - - // waiters are now waiting for the lock to be unlocked - thread::yield_now(); - - unsafe { - ReleaseSRWLockExclusive(lock_ptr.0); - } - println!("main unlocked"); - - for handle in waiters { - handle.join().unwrap(); - } -} - -// threads reacquiring locks should wait for all locks to be released first -fn exclusive_sleep_and_shared_lock() { - println!("exclusive_sleep_and_shared_lock"); - - let mut lock = SRWLOCK_INIT; - let mut condvar = CONDITION_VARIABLE_INIT; - - let lock_ptr = SendPtr(&mut lock); - let condvar_ptr = SendPtr(&mut condvar); - - let mut handles = Vec::with_capacity(10); - for i in 0..5 { - handles.push(thread::spawn(move || { - let lock_ptr = lock_ptr; // avoid field capture - let condvar_ptr = condvar_ptr; // avoid field capture - unsafe { - AcquireSRWLockExclusive(lock_ptr.0); - } - - println!("exclusive waiter {i} locked"); - - let r = unsafe { SleepConditionVariableSRW(condvar_ptr.0, lock_ptr.0, INFINITE, 0) }; - assert_ne!(r, 0); - - println!("exclusive waiter {i} reacquired lock"); - - // switch to next waiter or main thread - thread::yield_now(); - - unsafe { - ReleaseSRWLockExclusive(lock_ptr.0); - } - println!("exclusive waiter {i} unlocked"); - })); - } - - for i in 0..5 { - handles.push(thread::spawn(move || { - let lock_ptr = lock_ptr; // avoid field capture - unsafe { - AcquireSRWLockShared(lock_ptr.0); - } - println!("reader {i} locked"); - - // switch to next reader or main thread - thread::yield_now(); - - unsafe { - ReleaseSRWLockShared(lock_ptr.0); - } - println!("reader {i} unlocked"); - })); - } - - // ensures each reader has acquired the lock - thread::yield_now(); - - unsafe { - WakeAllConditionVariable(condvar_ptr.0); - } - - for handle in handles { - handle.join().unwrap(); - } -} - -fn main() { - all_shared(); - shared_sleep_and_exclusive_lock(); - exclusive_sleep_and_shared_lock(); -} diff --git a/src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.stdout b/src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.stdout deleted file mode 100644 index 918b54668f201..0000000000000 --- a/src/tools/miri/tests/pass-dep/concurrency/windows_condvar_shared.stdout +++ /dev/null @@ -1,60 +0,0 @@ -all_shared -exclusive waiter 0 locked -exclusive waiter 1 locked -exclusive waiter 2 locked -exclusive waiter 3 locked -exclusive waiter 4 locked -reader 0 locked -reader 1 locked -reader 2 locked -reader 3 locked -reader 4 locked -exclusive waiter 0 reacquired lock -exclusive waiter 1 reacquired lock -exclusive waiter 2 reacquired lock -exclusive waiter 3 reacquired lock -exclusive waiter 4 reacquired lock -reader 0 unlocked -reader 1 unlocked -reader 2 unlocked -reader 3 unlocked -reader 4 unlocked -shared_sleep_and_exclusive_lock -shared waiter 0 locked -shared waiter 1 locked -shared waiter 2 locked -shared waiter 3 locked -shared waiter 4 locked -main locked -main unlocked -shared waiter 0 reacquired lock -shared waiter 1 reacquired lock -shared waiter 2 reacquired lock -shared waiter 3 reacquired lock -shared waiter 4 reacquired lock -exclusive_sleep_and_shared_lock -exclusive waiter 0 locked -exclusive waiter 1 locked -exclusive waiter 2 locked -exclusive waiter 3 locked -exclusive waiter 4 locked -reader 0 locked -reader 1 locked -reader 2 locked -reader 3 locked -reader 4 locked -reader 0 unlocked -reader 1 unlocked -reader 2 unlocked -reader 3 unlocked -reader 4 unlocked -exclusive waiter 0 reacquired lock -exclusive waiter 0 unlocked -exclusive waiter 1 reacquired lock -exclusive waiter 1 unlocked -exclusive waiter 2 reacquired lock -exclusive waiter 2 unlocked -exclusive waiter 3 reacquired lock -exclusive waiter 3 unlocked -exclusive waiter 4 reacquired lock -exclusive waiter 4 unlocked From 1d279f179052882860231ffeb32e0f06982bfe36 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 10 Mar 2024 04:57:09 +0000 Subject: [PATCH 03/14] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index e23293ea558b7..243bc4a053651 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -4d4bb491b65c300835442f6cb4f34fc9a5685c26 +768408af123a455fb27ad8af8055becd5b95d36f From 2ecfbaef0a68fe6fb3aa7758d9a1bad531490365 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 10 Mar 2024 05:05:28 +0000 Subject: [PATCH 04/14] fmt --- .../miri/tests/pass/box-custom-alloc-aliasing.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs b/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs index 541e5f244db08..6daa033835ba1 100644 --- a/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs +++ b/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs @@ -10,9 +10,9 @@ use std::{ alloc::{AllocError, Allocator, Layout}, cell::{Cell, UnsafeCell}, + mem, ptr::{self, addr_of, NonNull}, thread::{self, ThreadId}, - mem, }; const BIN_SIZE: usize = 8; @@ -33,7 +33,7 @@ impl MyBin { } // Cast the *entire* thing to a raw pointer to not restrict its provenance. let bin = self as *const MyBin; - let base_ptr = UnsafeCell::raw_get(unsafe{ addr_of!((*bin).memory )}).cast::(); + let base_ptr = UnsafeCell::raw_get(unsafe { addr_of!((*bin).memory) }).cast::(); let ptr = unsafe { NonNull::new_unchecked(base_ptr.add(top)) }; self.top.set(top + 1); Some(ptr.cast()) @@ -64,22 +64,14 @@ impl MyAllocator { MyAllocator { thread_id, bins: Box::new( - [MyBin { - top: Cell::new(0), - thread_id, - memory: UnsafeCell::default(), - }; 1], + [MyBin { top: Cell::new(0), thread_id, memory: UnsafeCell::default() }; 1], ), } } // Pretends to be expensive finding a suitable bin for the layout. fn find_bin(&self, layout: Layout) -> Option<&MyBin> { - if layout == Layout::new::() { - Some(&self.bins[0]) - } else { - None - } + if layout == Layout::new::() { Some(&self.bins[0]) } else { None } } } From 5531a80e56b8b58a3b39caeae7dcf45c410868d5 Mon Sep 17 00:00:00 2001 From: tgolang Date: Mon, 11 Mar 2024 15:40:39 +0800 Subject: [PATCH 05/14] chore: remove repetitive word Signed-off-by: tgolang --- src/tools/miri/cargo-miri/src/phases.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 315f7a23a912a..81ff68545ccd3 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -95,7 +95,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { let target = get_arg_flag_value("--target"); let target = target.as_ref().unwrap_or(host); - // If cleaning the the target directory & sysroot cache, + // If cleaning the target directory & sysroot cache, // delete them then exit. There is no reason to setup a new // sysroot in this execution. if let MiriCommand::Clean = subcommand { From 9123df0300897fdcd718f0be193ac4ca4a30aa41 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 12 Mar 2024 05:38:23 +0000 Subject: [PATCH 06/14] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 243bc4a053651..632c0845ec01d 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -768408af123a455fb27ad8af8055becd5b95d36f +5aad51d015b8d3f6d823a6bf9dbc8ae3b9fd10c5 From 48a19ffd5e46bb93a030d62b009953900f37f350 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Wed, 13 Mar 2024 11:18:41 -0400 Subject: [PATCH 07/14] Improve sysroots notification --- src/tools/miri/.github/workflows/sysroots.yml | 21 +++++++++++++++---- src/tools/miri/ci/build-all-targets.sh | 7 +++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/.github/workflows/sysroots.yml b/src/tools/miri/.github/workflows/sysroots.yml index 96d7d4d111851..60d9ef5bb3757 100644 --- a/src/tools/miri/.github/workflows/sysroots.yml +++ b/src/tools/miri/.github/workflows/sysroots.yml @@ -21,6 +21,13 @@ jobs: ./miri install python3 -m pip install beautifulsoup4 ./ci/build-all-targets.sh + - name: Upload build errors + # We don't want to skip this step on failure + if: always() + uses: actions/upload-artifact@v4 + with: + name: failures + path: failures.tar.gz sysroots-cron-fail-notify: name: sysroots cronjob failure notification @@ -28,6 +35,11 @@ jobs: needs: [sysroots] if: failure() || cancelled() steps: + # Download our build error logs + - name: Download build errors + uses: actions/download-artifact@v4 + with: + name: failures # Send a Zulip notification - name: Install zulip-send run: pip3 install zulip @@ -36,11 +48,12 @@ jobs: ZULIP_BOT_EMAIL: ${{ secrets.ZULIP_BOT_EMAIL }} ZULIP_API_TOKEN: ${{ secrets.ZULIP_API_TOKEN }} run: | + tar xf failures.tar.gz + ls failures ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \ - --stream miri --subject "Cron Job Failure (miri, $(date -u +%Y-%m))" \ - --message 'Dear @*T-miri*, - - It would appear that the [Miri sysroots cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed. + --stream miri --subject "Sysroot Build Errors (miri, $(date -u +%Y-%m))" \ + --message 'It would appear that the [Miri sysroots cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed to build these targets: + $(ls failures) Would you mind investigating this issue? diff --git a/src/tools/miri/ci/build-all-targets.sh b/src/tools/miri/ci/build-all-targets.sh index fdb645c3ae182..b93d4f5eec431 100755 --- a/src/tools/miri/ci/build-all-targets.sh +++ b/src/tools/miri/ci/build-all-targets.sh @@ -3,6 +3,7 @@ set -eu set -o pipefail +# .github/workflows/sysroots.yml relies on this name this to report which sysroots didn't build FAILS_DIR=failures rm -rf $FAILS_DIR @@ -13,14 +14,16 @@ PLATFORM_SUPPORT_FILE=$(rustc +miri --print sysroot)/share/doc/rust/html/rustc/p for target in $(python3 ci/scrape-targets.py $PLATFORM_SUPPORT_FILE); do # Wipe the cache before every build to minimize disk usage cargo +miri miri clean - if cargo +miri miri setup --target $target 2>&1 | tee failures/$target; then + if cargo +miri miri setup --target $target 2>&1 | tee $FAILS_DIR/$target; then # If the build succeeds, delete its output. If we have output, a build failed. rm $FAILS_DIR/$target fi done +tar czf $FAILS_DIR.tar.gz $FAILS_DIR + # If the sysroot for any target fails to build, we will have a file in FAILS_DIR. -if [[ $(ls failures | wc -l) -ne 0 ]]; then +if [[ $(ls $FAILS_DIR | wc -l) -ne 0 ]]; then echo "Sysroots for the following targets failed to build:" ls $FAILS_DIR exit 1 From f5bb34f4605bc83c02c2c0a7a128d706d6031f11 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 14 Mar 2024 04:54:37 +0000 Subject: [PATCH 08/14] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 632c0845ec01d..721d6df0f1104 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -5aad51d015b8d3f6d823a6bf9dbc8ae3b9fd10c5 +5ac0b2d0219de2fd6fef86c69ef0cfa1e6c36f3b From 4cd673b4c6b6c633ea768b7d5ff1dbfead53c153 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 14 Mar 2024 05:02:48 +0000 Subject: [PATCH 09/14] fmt --- src/tools/miri/src/concurrency/data_race.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 4a1c3ac868e14..127d97bd5af1f 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -1074,7 +1074,12 @@ impl VClockAlloc { size: Size, machine: &mut MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { - self.unique_access(alloc_id, alloc_range(Size::ZERO, size), NaWriteType::Deallocate, machine) + self.unique_access( + alloc_id, + alloc_range(Size::ZERO, size), + NaWriteType::Deallocate, + machine, + ) } } From 915bb5b0820f82d20f8c17bc644f1d5ce38e60b5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Mar 2024 07:53:11 +0100 Subject: [PATCH 10/14] make cron job topic names more consistent --- src/tools/miri/.github/workflows/ci.yml | 2 +- src/tools/miri/.github/workflows/sysroots.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 49ed7ffce22fa..c70005c2c582a 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -165,7 +165,7 @@ jobs: ZULIP_API_TOKEN: ${{ secrets.ZULIP_API_TOKEN }} run: | ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \ - --stream miri --subject "Cron Job Failure (miri, $(date -u +%Y-%m))" \ + --stream miri --subject "Miri Build Failure ($(date -u +%Y-%m))" \ --message 'Dear @*T-miri*, It would appear that the [Miri cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed. diff --git a/src/tools/miri/.github/workflows/sysroots.yml b/src/tools/miri/.github/workflows/sysroots.yml index 60d9ef5bb3757..456c47f9fb7e8 100644 --- a/src/tools/miri/.github/workflows/sysroots.yml +++ b/src/tools/miri/.github/workflows/sysroots.yml @@ -51,7 +51,7 @@ jobs: tar xf failures.tar.gz ls failures ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \ - --stream miri --subject "Sysroot Build Errors (miri, $(date -u +%Y-%m))" \ + --stream miri --subject "Sysroot Build Errors ($(date -u +%Y-%m))" \ --message 'It would appear that the [Miri sysroots cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed to build these targets: $(ls failures) From 1bc4d62f81e0b665b00e4f8ff16f24eab8ee02bd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Mar 2024 07:51:47 +0100 Subject: [PATCH 11/14] explain time-with-isolation test better --- .../miri/tests/pass/shims/time-with-isolation.rs | 13 ++++++++++++- .../tests/pass/shims/time-with-isolation.stdout | 2 ++ .../miri/tests/pass/shims/time-with-isolation2.rs | 8 -------- .../tests/pass/shims/time-with-isolation2.stdout | 1 - 4 files changed, 14 insertions(+), 10 deletions(-) create mode 100644 src/tools/miri/tests/pass/shims/time-with-isolation.stdout delete mode 100644 src/tools/miri/tests/pass/shims/time-with-isolation2.rs delete mode 100644 src/tools/miri/tests/pass/shims/time-with-isolation2.stdout diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation.rs b/src/tools/miri/tests/pass/shims/time-with-isolation.rs index a0c3c6bbaa92e..645d42ad975da 100644 --- a/src/tools/miri/tests/pass/shims/time-with-isolation.rs +++ b/src/tools/miri/tests/pass/shims/time-with-isolation.rs @@ -24,7 +24,8 @@ fn test_time_passes() { assert_eq!(now2 - diff, now1); // The virtual clock is deterministic and I got 15ms on a 64-bit Linux machine. However, this // changes according to the platform so we use an interval to be safe. This should be updated - // if `NANOSECONDS_PER_BASIC_BLOCK` changes. + // if `NANOSECONDS_PER_BASIC_BLOCK` changes. It may also need updating if the standard library + // code that runs in the loop above changes. assert!(diff.as_millis() > 5); assert!(diff.as_millis() < 20); } @@ -37,8 +38,18 @@ fn test_block_for_one_second() { while Instant::now() < end {} } +/// Ensures that we get the same behavior across all targets. +fn test_deterministic() { + let begin = Instant::now(); + for _ in 0..100_000 {} + let time = begin.elapsed(); + println!("The loop took around {}s", time.as_secs()); + println!("(It's fine for this number to change when you `--bless` this test.)") +} + fn main() { test_time_passes(); test_block_for_one_second(); test_sleep(); + test_deterministic(); } diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation.stdout b/src/tools/miri/tests/pass/shims/time-with-isolation.stdout new file mode 100644 index 0000000000000..f3d071e001e52 --- /dev/null +++ b/src/tools/miri/tests/pass/shims/time-with-isolation.stdout @@ -0,0 +1,2 @@ +The loop took around 7s +(It's fine for this number to change when you `--bless` this test.) diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation2.rs b/src/tools/miri/tests/pass/shims/time-with-isolation2.rs deleted file mode 100644 index 24e72e22fd895..0000000000000 --- a/src/tools/miri/tests/pass/shims/time-with-isolation2.rs +++ /dev/null @@ -1,8 +0,0 @@ -use std::time::Instant; - -fn main() { - let begin = Instant::now(); - for _ in 0..100_000 {} - let time = begin.elapsed(); - println!("The loop took around {}s", time.as_secs()); -} diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout b/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout deleted file mode 100644 index c68b40b744bf2..0000000000000 --- a/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout +++ /dev/null @@ -1 +0,0 @@ -The loop took around 7s From 22f6193ac1f26c199418e364efe5883ba1b988c8 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 14 Mar 2024 18:43:04 -0400 Subject: [PATCH 12/14] Apply the same shell quoting trick we use in the URL to --- src/tools/miri/.github/workflows/sysroots.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/.github/workflows/sysroots.yml b/src/tools/miri/.github/workflows/sysroots.yml index 456c47f9fb7e8..73a10768db908 100644 --- a/src/tools/miri/.github/workflows/sysroots.yml +++ b/src/tools/miri/.github/workflows/sysroots.yml @@ -53,7 +53,7 @@ jobs: ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \ --stream miri --subject "Sysroot Build Errors ($(date -u +%Y-%m))" \ --message 'It would appear that the [Miri sysroots cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed to build these targets: - $(ls failures) + '"$(ls failures)"' Would you mind investigating this issue? From 3cc4059a6e8a6aebcd020b0d3a0e2eb33d926c2c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 15 Mar 2024 07:55:46 +0100 Subject: [PATCH 13/14] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 721d6df0f1104..e32968d8178e8 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -5ac0b2d0219de2fd6fef86c69ef0cfa1e6c36f3b +ee03c286cfdca26fa5b2a4ee40957625d2c826ff From 32c734b73cf6eada0b330337e24fee2c6039cc84 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 15 Mar 2024 08:09:46 +0100 Subject: [PATCH 14/14] fmt --- src/tools/miri/src/diagnostics.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 6e612ea34a70f..03428b081c569 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -527,8 +527,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { pub fn emit_diagnostic(&self, e: NonHaltingDiagnostic) { use NonHaltingDiagnostic::*; - let stacktrace = - Frame::generate_stacktrace_from_stack(self.threads.active_thread_stack()); + let stacktrace = Frame::generate_stacktrace_from_stack(self.threads.active_thread_stack()); let (stacktrace, _was_pruned) = prune_stacktrace(stacktrace, self); let (title, diag_level) = match &e {