From 1842257814919fa62e81bdecd5e8f95be2839dbb Mon Sep 17 00:00:00 2001 From: Alex Abdugafarov Date: Tue, 17 Oct 2023 20:11:58 +0500 Subject: [PATCH 1/5] Fixed `Hole::get` marked as unsafe in `exception-safety.md` (#427) --- src/exception-safety.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/exception-safety.md b/src/exception-safety.md index 8404bb8..762b38b 100644 --- a/src/exception-safety.md +++ b/src/exception-safety.md @@ -172,7 +172,7 @@ impl<'a, T> Hole<'a, T> { fn removed(&self) -> &T { self.elt.as_ref().unwrap() } - unsafe fn get(&self, index: usize) -> &T { &self.data[index] } + fn get(&self, index: usize) -> &T { &self.data[index] } unsafe fn move_to(&mut self, index: usize) { let index_ptr: *const _ = &self.data[index]; From 0e589061c89ef5aa85d6b04a2bfe95a9d6543add Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sun, 19 Nov 2023 00:36:09 -0600 Subject: [PATCH 2/5] Reword the section on general race conditions The section on preventing general race conditions is a bit hand wavy. Change wording to be more concrete, and add an example of Rust preventing general races in a very specific case. --- src/races.md | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/races.md b/src/races.md index cb78ac6..d5f1ea0 100644 --- a/src/races.md +++ b/src/races.md @@ -6,26 +6,28 @@ Safe Rust guarantees an absence of data races, which are defined as: * one or more of them is a write * one or more of them is unsynchronized -A data race has Undefined Behavior, and is therefore impossible to perform -in Safe Rust. Data races are *mostly* prevented through Rust's ownership system: +A data race has Undefined Behavior, and is therefore impossible to perform in +Safe Rust. Data races are *mostly* prevented through Rust's ownership system: it's impossible to alias a mutable reference, so it's impossible to perform a data race. Interior mutability makes this more complicated, which is largely why -we have the Send and Sync traits (see below). +we have the Send and Sync traits (see the next section for more on this). **However Rust does not prevent general race conditions.** -This is pretty fundamentally impossible, and probably honestly undesirable. Your -hardware is racy, your OS is racy, the other programs on your computer are racy, -and the world this all runs in is racy. Any system that could genuinely claim to -prevent *all* race conditions would be pretty awful to use, if not just -incorrect. +This is mathematically impossible in situations where you do not control the +scheduler, which is true for the normal OS environment. If you do control +preemption, it _can be_ possible to prevent general races - this technique is +used by frameworks such as [RTIC](https://github.com/rtic-rs/rtic). However, +actually having control over scheduling is a very uncommon case. -So it's perfectly "fine" for a Safe Rust program to get deadlocked or do -something nonsensical with incorrect synchronization. Obviously such a program -isn't very good, but Rust can only hold your hand so far. Still, a race -condition can't violate memory safety in a Rust program on its own. Only in -conjunction with some other unsafe code can a race condition actually violate -memory safety. For instance: +For this reason, it is considered "safe" for Rust to get deadlocked or do +something nonsensical with incorrect synchronization: this is known as a general +race condition or resource race. Obviously such a program isn't very good, but +Rust of course cannot prevent all logic errors. + +In any case, a race condition cannot violate memory safety in a Rust program on +its own. Only in conjunction with some other unsafe code can a race condition +actually violate memory safety. For instance, a correct program looks like this: ```rust,no_run use std::thread; @@ -58,6 +60,9 @@ thread::spawn(move || { println!("{}", data[idx.load(Ordering::SeqCst)]); ``` +We can cause a data race if we instead do the bound check in advance, and then +unsafely access the data with an unchecked value: + ```rust,no_run use std::thread; use std::sync::atomic::{AtomicUsize, Ordering}; From 360a768c6d4fc5910e9c849d793c82f9f3bf3936 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Sun, 10 Dec 2023 04:18:32 +0100 Subject: [PATCH 3/5] Improve the `PhantomData` table (#417) --- src/phantom-data.md | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/phantom-data.md b/src/phantom-data.md index 449d9e7..cd2428d 100644 --- a/src/phantom-data.md +++ b/src/phantom-data.md @@ -106,7 +106,14 @@ that that `Vec` _owns_ values of type `T` (more precisely: may use values of in its `Drop` implementation), and Rust will thus not allow them to _dangle_ should a `Vec` be dropped. -**Adding an extra `_owns_T: PhantomData` field is thus _superfluous_ and accomplishes nothing**. +When a type already has a `Drop impl`, **adding an extra `_owns_T: PhantomData` field +is thus _superfluous_ and accomplishes nothing**, dropck-wise (it still affects variance +and auto-traits). + + - (advanced edge case: if the type containing the `PhantomData` has no `Drop` impl at all, + but still has drop glue (by having _another_ field with drop glue), then the + dropck/`#[may_dangle]` considerations mentioned herein do apply as well: a `PhantomData` + field will then require `T` to be droppable whenever the containing type goes out of scope). ___ @@ -234,14 +241,18 @@ standard library made a utility for itself called `Unique` which: Here’s a table of all the wonderful ways `PhantomData` could be used: -| Phantom type | `'a` | `T` | `Send` | `Sync` | -|-----------------------------|-----------|-----------------------------|-----------|-----------| -| `PhantomData` | - | covariant (with drop check) | `T: Send` | `T: Sync` | -| `PhantomData<&'a T>` | covariant | covariant | `T: Sync` | `T: Sync` | -| `PhantomData<&'a mut T>` | covariant | invariant | `T: Send` | `T: Sync` | -| `PhantomData<*const T>` | - | covariant | - | - | -| `PhantomData<*mut T>` | - | invariant | - | - | -| `PhantomData` | - | contravariant | `Send` | `Sync` | -| `PhantomData T>` | - | covariant | `Send` | `Sync` | -| `PhantomData T>` | - | invariant | `Send` | `Sync` | -| `PhantomData>` | invariant | - | `Send` | - | +| Phantom type | variance of `'a` | variance of `T` | `Send`/`Sync`
(or lack thereof) | dangling `'a` or `T` in drop glue
(_e.g._, `#[may_dangle] Drop`) | +|-----------------------------|:----------------:|:-----------------:|:-----------------------------------------:|:------------------------------------------------:| +| `PhantomData` | - | **cov**ariant | inherited | disallowed ("owns `T`") | +| `PhantomData<&'a T>` | **cov**ariant | **cov**ariant | `Send + Sync`
requires
`T : Sync` | allowed | +| `PhantomData<&'a mut T>` | **cov**ariant | **inv**ariant | inherited | allowed | +| `PhantomData<*const T>` | - | **cov**ariant | `!Send + !Sync` | allowed | +| `PhantomData<*mut T>` | - | **inv**ariant | `!Send + !Sync` | allowed | +| `PhantomData` | - | **contra**variant | `Send + Sync` | allowed | +| `PhantomData T>` | - | **cov**ariant | `Send + Sync` | allowed | +| `PhantomData T>` | - | **inv**ariant | `Send + Sync` | allowed | +| `PhantomData>` | **inv**ariant | - | `Send + !Sync` | allowed | + + - Note: opting out of the `Unpin` auto-trait requires the dedicated [`PhantomPinned`] type instead. + +[`PhantomPinned`]: ../core/marker/struct.PhantomPinned.html From f6bd083c4ccfc4ce6699b8b4154e3c45c5a27a8c Mon Sep 17 00:00:00 2001 From: Eva Pace Date: Sun, 10 Dec 2023 00:19:24 -0300 Subject: [PATCH 4/5] Minor improvements to Vec (#415) --- src/vec/vec-drain.md | 20 ++++++++--------- src/vec/vec-final.md | 43 ++++++++++++++++++------------------ src/vec/vec-insert-remove.md | 5 +++-- src/vec/vec-into-iter.md | 22 +++++++++--------- src/vec/vec-layout.md | 1 - src/vec/vec-raw.md | 32 +++++++++++++-------------- 6 files changed, 59 insertions(+), 64 deletions(-) diff --git a/src/vec/vec-drain.md b/src/vec/vec-drain.md index 7a0e7f8..763c82a 100644 --- a/src/vec/vec-drain.md +++ b/src/vec/vec-drain.md @@ -93,7 +93,7 @@ impl IntoIterator for Vec { mem::forget(self); IntoIter { - iter: iter, + iter, _buf: buf, } } @@ -135,18 +135,16 @@ impl<'a, T> Drop for Drain<'a, T> { impl Vec { pub fn drain(&mut self) -> Drain { - unsafe { - let iter = RawValIter::new(&self); + let iter = unsafe { RawValIter::new(&self) }; - // this is a mem::forget safety thing. If Drain is forgotten, we just - // leak the whole Vec's contents. Also we need to do this *eventually* - // anyway, so why not do it now? - self.len = 0; + // this is a mem::forget safety thing. If Drain is forgotten, we just + // leak the whole Vec's contents. Also we need to do this *eventually* + // anyway, so why not do it now? + self.len = 0; - Drain { - iter: iter, - vec: PhantomData, - } + Drain { + iter, + vec: PhantomData, } } } diff --git a/src/vec/vec-final.md b/src/vec/vec-final.md index e680e0d..1f73036 100644 --- a/src/vec/vec-final.md +++ b/src/vec/vec-final.md @@ -127,7 +127,7 @@ impl Vec { pub fn insert(&mut self, index: usize, elem: T) { assert!(index <= self.len, "index out of bounds"); - if self.cap() == self.len { + if self.len == self.cap() { self.buf.grow(); } @@ -138,14 +138,17 @@ impl Vec { self.len - index, ); ptr::write(self.ptr().add(index), elem); - self.len += 1; } + + self.len += 1; } pub fn remove(&mut self, index: usize) -> T { assert!(index < self.len, "index out of bounds"); + + self.len -= 1; + unsafe { - self.len -= 1; let result = ptr::read(self.ptr().add(index)); ptr::copy( self.ptr().add(index + 1), @@ -157,18 +160,16 @@ impl Vec { } pub fn drain(&mut self) -> Drain { - unsafe { - let iter = RawValIter::new(&self); + let iter = unsafe { RawValIter::new(&self) }; - // this is a mem::forget safety thing. If Drain is forgotten, we just - // leak the whole Vec's contents. Also we need to do this *eventually* - // anyway, so why not do it now? - self.len = 0; + // this is a mem::forget safety thing. If Drain is forgotten, we just + // leak the whole Vec's contents. Also we need to do this *eventually* + // anyway, so why not do it now? + self.len = 0; - Drain { - iter: iter, - vec: PhantomData, - } + Drain { + iter, + vec: PhantomData, } } } @@ -197,15 +198,15 @@ impl IntoIterator for Vec { type Item = T; type IntoIter = IntoIter; fn into_iter(self) -> IntoIter { - unsafe { - let iter = RawValIter::new(&self); - let buf = ptr::read(&self.buf); - mem::forget(self); + let (iter, buf) = unsafe { + (RawValIter::new(&self), ptr::read(&self.buf)) + }; - IntoIter { - iter: iter, - _buf: buf, - } + mem::forget(self); + + IntoIter { + iter, + _buf: buf, } } } diff --git a/src/vec/vec-insert-remove.md b/src/vec/vec-insert-remove.md index 722e20c..2acee65 100644 --- a/src/vec/vec-insert-remove.md +++ b/src/vec/vec-insert-remove.md @@ -18,7 +18,7 @@ pub fn insert(&mut self, index: usize, elem: T) { // Note: `<=` because it's valid to insert after everything // which would be equivalent to push. assert!(index <= self.len, "index out of bounds"); - if self.cap == self.len { self.grow(); } + if self.len == self.cap { self.grow(); } unsafe { // ptr::copy(src, dest, len): "copy from src to dest len elems" @@ -28,8 +28,9 @@ pub fn insert(&mut self, index: usize, elem: T) { self.len - index, ); ptr::write(self.ptr.as_ptr().add(index), elem); - self.len += 1; } + + self.len += 1; } ``` diff --git a/src/vec/vec-into-iter.md b/src/vec/vec-into-iter.md index a3a3c8c..ad22ff9 100644 --- a/src/vec/vec-into-iter.md +++ b/src/vec/vec-into-iter.md @@ -68,18 +68,16 @@ impl IntoIterator for Vec { let cap = vec.cap; let len = vec.len; - unsafe { - IntoIter { - buf: ptr, - cap: cap, - start: ptr.as_ptr(), - end: if cap == 0 { - // can't offset off this pointer, it's not allocated! - ptr.as_ptr() - } else { - ptr.as_ptr().add(len) - }, - } + IntoIter { + buf: ptr, + cap, + start: ptr.as_ptr(), + end: if cap == 0 { + // can't offset off this pointer, it's not allocated! + ptr.as_ptr() + } else { + unsafe { ptr.as_ptr().add(len) } + }, } } } diff --git a/src/vec/vec-layout.md b/src/vec/vec-layout.md index 9129952..695485f 100644 --- a/src/vec/vec-layout.md +++ b/src/vec/vec-layout.md @@ -40,7 +40,6 @@ we get the same results as using `Unique`: ```rust use std::ptr::NonNull; -use std::marker::PhantomData; pub struct Vec { ptr: NonNull, diff --git a/src/vec/vec-raw.md b/src/vec/vec-raw.md index 0bca2da..a251b4a 100644 --- a/src/vec/vec-raw.md +++ b/src/vec/vec-raw.md @@ -131,23 +131,21 @@ impl IntoIterator for Vec { type Item = T; type IntoIter = IntoIter; fn into_iter(self) -> IntoIter { - unsafe { - // need to use ptr::read to unsafely move the buf out since it's - // not Copy, and Vec implements Drop (so we can't destructure it). - let buf = ptr::read(&self.buf); - let len = self.len; - mem::forget(self); - - IntoIter { - start: buf.ptr.as_ptr(), - end: if buf.cap == 0 { - // can't offset off of a pointer unless it's part of an allocation - buf.ptr.as_ptr() - } else { - buf.ptr.as_ptr().add(len) - }, - _buf: buf, - } + // need to use ptr::read to unsafely move the buf out since it's + // not Copy, and Vec implements Drop (so we can't destructure it). + let buf = unsafe { ptr::read(&self.buf) }; + let len = self.len; + mem::forget(self); + + IntoIter { + start: buf.ptr.as_ptr(), + end: if buf.cap == 0 { + // can't offset off of a pointer unless it's part of an allocation + buf.ptr.as_ptr() + } else { + unsafe { buf.ptr.as_ptr().add(len) } + }, + _buf: buf, } } } From 6bc2415218d4dd0cb01433d8320f5ccf79c343a1 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 2 Jan 2024 22:01:04 -0600 Subject: [PATCH 5/5] Update an example of `thread_local` to use `local_key_cell_methods` (#438) --- src/subtyping.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/subtyping.md b/src/subtyping.md index f63b532..4c45b2d 100644 --- a/src/subtyping.md +++ b/src/subtyping.md @@ -310,9 +310,7 @@ thread_local! { /// saves the input given into a thread local `Vec<&'static str>` fn store(input: &'static str) { - StaticVecs.with(|v| { - v.borrow_mut().push(input); - }) + StaticVecs.with_borrow_mut(|v| v.push(input)); } /// Calls the function with it's input (must have the same lifetime!) @@ -332,9 +330,8 @@ fn main() { demo(&smuggle, store); } - StaticVecs.with(|v| { - println!("{:?}", v.borrow()); // use after free 😿 - }); + // use after free 😿 + StaticVecs.with_borrow(|v| println!("{v:?}")); } ```