From 9a5443fe21bfaba77a4c872ec76b8e828bb265c0 Mon Sep 17 00:00:00 2001 From: Raoul Strackx Date: Mon, 15 May 2023 15:10:25 +0200 Subject: [PATCH 1/8] `waitqueue` clarifications for SGX platform --- library/std/src/sys/sgx/waitqueue/mod.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/sgx/waitqueue/mod.rs b/library/std/src/sys/sgx/waitqueue/mod.rs index 5e1d859ee99c3..32328123f730e 100644 --- a/library/std/src/sys/sgx/waitqueue/mod.rs +++ b/library/std/src/sys/sgx/waitqueue/mod.rs @@ -148,6 +148,8 @@ impl WaitQueue { /// until a wakeup event. /// /// This function does not return until this thread has been awoken. + /// + /// Safety: `before_wait` must not panic pub fn wait(mut guard: SpinMutexGuard<'_, WaitVariable>, before_wait: F) { // very unsafe: check requirements of UnsafeList::push unsafe { @@ -159,6 +161,9 @@ impl WaitQueue { drop(guard); before_wait(); while !entry.lock().wake { + // `entry.wake` is only set in `notify_one` and `notify_all` functions. Both ensure + // the entry is removed from the queue _before_ setting this bool. There are no + // other references to `entry`. // don't panic, this would invalidate `entry` during unwinding let eventset = rtunwrap!(Ok, usercalls::wait(EV_UNPARK, WAIT_INDEFINITE)); rtassert!(eventset & EV_UNPARK == EV_UNPARK); @@ -169,6 +174,7 @@ impl WaitQueue { /// Adds the calling thread to the `WaitVariable`'s wait queue, then wait /// until a wakeup event or timeout. If event was observed, returns true. /// If not, it will remove the calling thread from the wait queue. + /// Safety: `before_wait` must not panic pub fn wait_timeout( lock: &SpinMutex>, timeout: Duration, @@ -183,7 +189,9 @@ impl WaitQueue { let entry_lock = lock.lock().queue.inner.push(&mut entry); before_wait(); usercalls::wait_timeout(EV_UNPARK, timeout, || entry_lock.lock().wake); - // acquire the wait queue's lock first to avoid deadlock. + // acquire the wait queue's lock first to avoid deadlock + // and ensure no other function can simultaneously access the list + // (e.g., `notify_one` or `notify_all`) let mut guard = lock.lock(); let success = entry_lock.lock().wake; if !success { @@ -204,8 +212,8 @@ impl WaitQueue { ) -> Result, SpinMutexGuard<'_, WaitVariable>> { // SAFETY: lifetime of the pop() return value is limited to the map // closure (The closure return value is 'static). The underlying - // stack frame won't be freed until after the WaitGuard created below - // is dropped. + // stack frame won't be freed until after the lock on the queue is released + // (i.e., `guard` is dropped). unsafe { let tcs = guard.queue.inner.pop().map(|entry| -> Tcs { let mut entry_guard = entry.lock(); @@ -231,7 +239,7 @@ impl WaitQueue { ) -> Result, SpinMutexGuard<'_, WaitVariable>> { // SAFETY: lifetime of the pop() return values are limited to the // while loop body. The underlying stack frames won't be freed until - // after the WaitGuard created below is dropped. + // after the lock on the queue is released (i.e., `guard` is dropped). unsafe { let mut count = 0; while let Some(entry) = guard.queue.inner.pop() { From 9baab45e2f2bc3218737613bda074e5b04d2034b Mon Sep 17 00:00:00 2001 From: Raoul Strackx Date: Mon, 31 Jul 2023 13:28:44 +0200 Subject: [PATCH 2/8] Aborting when `before_wait` function panics --- library/std/src/sys/sgx/waitqueue/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/sgx/waitqueue/mod.rs b/library/std/src/sys/sgx/waitqueue/mod.rs index 32328123f730e..25eca61d67b66 100644 --- a/library/std/src/sys/sgx/waitqueue/mod.rs +++ b/library/std/src/sys/sgx/waitqueue/mod.rs @@ -18,6 +18,7 @@ mod unsafe_list; use crate::num::NonZeroUsize; use crate::ops::{Deref, DerefMut}; +use crate::panic::{self, AssertUnwindSafe}; use crate::time::Duration; use super::abi::thread; @@ -147,9 +148,8 @@ impl WaitQueue { /// Adds the calling thread to the `WaitVariable`'s wait queue, then wait /// until a wakeup event. /// - /// This function does not return until this thread has been awoken. - /// - /// Safety: `before_wait` must not panic + /// This function does not return until this thread has been awoken. When `before_wait` panics, + /// this function will abort. pub fn wait(mut guard: SpinMutexGuard<'_, WaitVariable>, before_wait: F) { // very unsafe: check requirements of UnsafeList::push unsafe { @@ -159,7 +159,9 @@ impl WaitQueue { })); let entry = guard.queue.inner.push(&mut entry); drop(guard); - before_wait(); + if let Err(_e) = panic::catch_unwind(AssertUnwindSafe(|| before_wait())) { + rtabort!("Panic before wait on wakeup event") + } while !entry.lock().wake { // `entry.wake` is only set in `notify_one` and `notify_all` functions. Both ensure // the entry is removed from the queue _before_ setting this bool. There are no @@ -174,7 +176,7 @@ impl WaitQueue { /// Adds the calling thread to the `WaitVariable`'s wait queue, then wait /// until a wakeup event or timeout. If event was observed, returns true. /// If not, it will remove the calling thread from the wait queue. - /// Safety: `before_wait` must not panic + /// When `before_wait` panics, this function will abort. pub fn wait_timeout( lock: &SpinMutex>, timeout: Duration, @@ -187,7 +189,9 @@ impl WaitQueue { wake: false, })); let entry_lock = lock.lock().queue.inner.push(&mut entry); - before_wait(); + if let Err(_e) = panic::catch_unwind(AssertUnwindSafe(|| before_wait())) { + rtabort!("Panic before wait on wakeup event or timeout") + } usercalls::wait_timeout(EV_UNPARK, timeout, || entry_lock.lock().wake); // acquire the wait queue's lock first to avoid deadlock // and ensure no other function can simultaneously access the list From dfdab8fc629da55ec434d8838daaba9906a61445 Mon Sep 17 00:00:00 2001 From: vwkd <33468089+vwkd@users.noreply.github.com> Date: Fri, 1 Sep 2023 21:58:40 +0200 Subject: [PATCH 3/8] Update mod.rs --- library/core/src/str/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index 23cebdb6c3b90..ab6dfb0a72940 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -806,7 +806,7 @@ impl str { /// assert_eq!(Some((0, 'y')), char_indices.next()); // not (0, 'y̆') /// assert_eq!(Some((1, '\u{0306}')), char_indices.next()); /// - /// // note the 3 here - the last character took up two bytes + /// // note the 3 here - the previous character took up two bytes /// assert_eq!(Some((3, 'e')), char_indices.next()); /// assert_eq!(Some((4, 's')), char_indices.next()); /// From a16622f62f10b76af9b61d5177f6e3bb8bdb09b5 Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Sun, 3 Sep 2023 18:12:27 -0700 Subject: [PATCH 4/8] Clarify ManuallyDrop bit validity Clarify that `ManuallyDrop` has the same bit validity as `T`. --- library/core/src/mem/manually_drop.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/core/src/mem/manually_drop.rs b/library/core/src/mem/manually_drop.rs index 5f3d66e3773f1..98cff3493a7ea 100644 --- a/library/core/src/mem/manually_drop.rs +++ b/library/core/src/mem/manually_drop.rs @@ -4,12 +4,12 @@ use crate::ptr; /// A wrapper to inhibit compiler from automatically calling `T`’s destructor. /// This wrapper is 0-cost. /// -/// `ManuallyDrop` is guaranteed to have the same layout as `T`, and is subject -/// to the same layout optimizations as `T`. As a consequence, it has *no effect* -/// on the assumptions that the compiler makes about its contents. For example, -/// initializing a `ManuallyDrop<&mut T>` with [`mem::zeroed`] is undefined -/// behavior. If you need to handle uninitialized data, use [`MaybeUninit`] -/// instead. +/// `ManuallyDrop` is guaranteed to have the same layout and bit validity as +/// `T`, and is subject to the same layout optimizations as `T`. As a consequence, +/// it has *no effect* on the assumptions that the compiler makes about its +/// contents. For example, initializing a `ManuallyDrop<&mut T>` with [`mem::zeroed`] +/// is undefined behavior. If you need to handle uninitialized data, use +/// [`MaybeUninit`] instead. /// /// Note that accessing the value inside a `ManuallyDrop` is safe. /// This means that a `ManuallyDrop` whose content has been dropped must not From 71429f5fd2388ff6332889c193409a88c762e7c1 Mon Sep 17 00:00:00 2001 From: July Tikhonov Date: Tue, 5 Sep 2023 19:46:18 +0300 Subject: [PATCH 5/8] fix a comment in std::iter::successors The `unfold` function have since been renamed to `from_fn`. --- library/core/src/iter/sources/successors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/sources/successors.rs b/library/core/src/iter/sources/successors.rs index 6a6cbe905e464..7f7b2c7756628 100644 --- a/library/core/src/iter/sources/successors.rs +++ b/library/core/src/iter/sources/successors.rs @@ -17,7 +17,7 @@ where F: FnMut(&T) -> Option, { // If this function returned `impl Iterator` - // it could be based on `unfold` and not need a dedicated type. + // it could be based on `from_fn` and not need a dedicated type. // However having a named `Successors` type allows it to be `Clone` when `T` and `F` are. Successors { next: first, succ } } From ec2e00c4045f49f8efc5b1490b6276ecb364b2dd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 2 Oct 2023 08:34:10 +0200 Subject: [PATCH 6/8] update some comments around swap() --- library/core/src/mem/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index 5244e478018e9..4ec49a6d0b9d3 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -728,10 +728,6 @@ pub const fn swap(x: &mut T, y: &mut T) { // reinterpretation of values as (chunkable) byte arrays, and the loop in the // block optimization in `swap_slice` is hard to rewrite back // into the (unoptimized) direct swapping implementation, so we disable it. - // FIXME(eddyb) the block optimization also prevents MIR optimizations from - // understanding `mem::replace`, `Option::take`, etc. - a better overall - // solution might be to make `ptr::swap_nonoverlapping` into an intrinsic, which - // a backend can choose to implement using the block optimization, or not. #[cfg(not(any(target_arch = "spirv")))] { // For types that are larger multiples of their alignment, the simple way @@ -768,11 +764,14 @@ pub(crate) const fn swap_simple(x: &mut T, y: &mut T) { // And LLVM actually optimizes it to 3×memcpy if called with // a type larger than it's willing to keep in a register. // Having typed reads and writes in MIR here is also good as - // it lets MIRI and CTFE understand them better, including things + // it lets Miri and CTFE understand them better, including things // like enforcing type validity for them. // Importantly, read+copy_nonoverlapping+write introduces confusing // asymmetry to the behaviour where one value went through read+write // whereas the other was copied over by the intrinsic (see #94371). + // Furthermore, using only read+write here benefits limited backends + // such as SPIR-V that work on an underlying *typed* view of memory, + // and thus have trouble with Rust's untyped memory operations. // SAFETY: exclusive references are always valid to read/write, // including being aligned, and nothing here panics so it's drop-safe. From bfc0f23acb49adbd55a04ed84fba4badc0a1be04 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 2 Oct 2023 08:35:08 +0200 Subject: [PATCH 7/8] MIRI -> Miri --- compiler/rustc_middle/src/mir/syntax.rs | 2 +- library/core/tests/array.rs | 4 ++-- tests/ui/consts/const-eval/nrvo.rs | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 55f895f73b48b..b190963704b20 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -996,7 +996,7 @@ pub type AssertMessage<'tcx> = AssertKind>; /// /// [UCG#319]: https://github.com/rust-lang/unsafe-code-guidelines/issues/319 /// -/// Rust currently requires that every place obey those two rules. This is checked by MIRI and taken +/// Rust currently requires that every place obey those two rules. This is checked by Miri and taken /// advantage of by codegen (via `gep inbounds`). That is possibly subject to change. #[derive(Copy, Clone, PartialEq, Eq, Hash, TyEncodable, HashStable, TypeFoldable, TypeVisitable)] pub struct Place<'tcx> { diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 982d7853f6936..81da75d32a1c9 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -663,7 +663,7 @@ fn array_mixed_equality_nans() { #[test] fn array_into_iter_fold() { - // Strings to help MIRI catch if we double-free or something + // Strings to help Miri catch if we double-free or something let a = ["Aa".to_string(), "Bb".to_string(), "Cc".to_string()]; let mut s = "s".to_string(); a.into_iter().for_each(|b| s += &b); @@ -679,7 +679,7 @@ fn array_into_iter_fold() { #[test] fn array_into_iter_rfold() { - // Strings to help MIRI catch if we double-free or something + // Strings to help Miri catch if we double-free or something let a = ["Aa".to_string(), "Bb".to_string(), "Cc".to_string()]; let mut s = "s".to_string(); a.into_iter().rev().for_each(|b| s += &b); diff --git a/tests/ui/consts/const-eval/nrvo.rs b/tests/ui/consts/const-eval/nrvo.rs index 1d2c6acc06cd5..22da96a3fc1ea 100644 --- a/tests/ui/consts/const-eval/nrvo.rs +++ b/tests/ui/consts/const-eval/nrvo.rs @@ -1,7 +1,8 @@ // run-pass // When the NRVO is applied, the return place (`_0`) gets treated like a normal local. For example, -// its address may be taken and it may be written to indirectly. Ensure that MIRI can handle this. +// its address may be taken and it may be written to indirectly. Ensure that the const-eval +// interpreter can handle this. #![feature(const_mut_refs)] From 1eb2a766417c755f6787358bfc36385a26c4ed31 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 5 Oct 2023 22:32:37 -0700 Subject: [PATCH 8/8] rustdoc-search: fix bug with multi-item impl trait --- src/librustdoc/html/static/js/search.js | 2 +- tests/rustdoc-js/impl-trait.js | 19 ++++++++++++++----- tests/rustdoc-js/impl-trait.rs | 6 ++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 2f0cae0a48e21..7a282a99e9ce9 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -1555,7 +1555,7 @@ function initSearch(rawSearchIndex) { return false; } } - } else if (fnType.id !== null) { + } else { if (queryElem.id === typeNameIdOfArrayOrSlice && (fnType.id === typeNameIdOfSlice || fnType.id === typeNameIdOfArray) ) { diff --git a/tests/rustdoc-js/impl-trait.js b/tests/rustdoc-js/impl-trait.js index 710e594b54774..00d67d639bd08 100644 --- a/tests/rustdoc-js/impl-trait.js +++ b/tests/rustdoc-js/impl-trait.js @@ -1,3 +1,4 @@ +// exact-check // ignore-order const EXPECTED = [ @@ -20,9 +21,16 @@ const EXPECTED = [ { 'query': '-> Aaaaaaa', 'others': [ - { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' }, - { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' }, { 'path': 'impl_trait', 'name': 'bbbbbbb' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'ggggggg' }, + ], + }, + { + 'query': '-> Bbbbbbb', + 'others': [ + { 'path': 'impl_trait::Ccccccc', 'name': 'ggggggg' }, ], }, { @@ -31,13 +39,14 @@ const EXPECTED = [ { 'path': 'impl_trait', 'name': 'Aaaaaaa' }, ], 'in_args': [ - { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' }, { 'path': 'impl_trait::Ccccccc', 'name': 'eeeeeee' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' }, ], 'returned': [ - { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' }, - { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' }, { 'path': 'impl_trait', 'name': 'bbbbbbb' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' }, + { 'path': 'impl_trait::Ccccccc', 'name': 'ggggggg' }, ], }, ]; diff --git a/tests/rustdoc-js/impl-trait.rs b/tests/rustdoc-js/impl-trait.rs index fb8869b46f3d4..d20fdd60ec965 100644 --- a/tests/rustdoc-js/impl-trait.rs +++ b/tests/rustdoc-js/impl-trait.rs @@ -1,6 +1,9 @@ pub trait Aaaaaaa {} +pub trait Bbbbbbb {} + impl Aaaaaaa for () {} +impl Bbbbbbb for () {} pub fn bbbbbbb() -> impl Aaaaaaa { () @@ -18,4 +21,7 @@ impl Ccccccc { pub fn fffffff(&self, x: impl Aaaaaaa) -> impl Aaaaaaa { x } + pub fn ggggggg(&self) -> impl Aaaaaaa + Bbbbbbb { + () + } }