From 4cb4013464b3ec594e42dbed4d80110ee1817988 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 14 Aug 2023 09:48:13 +0200 Subject: [PATCH 1/3] make Cell::swap panic if the Cells partially overlap --- library/core/src/cell.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index bf4c682d33e5a..69d4c3768db30 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -237,6 +237,7 @@ use crate::cmp::Ordering; use crate::fmt::{self, Debug, Display}; +use crate::intrinsics::is_nonoverlapping; use crate::marker::{PhantomData, Unsize}; use crate::mem; use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn}; @@ -415,6 +416,12 @@ impl Cell { /// Swaps the values of two `Cell`s. /// Difference with `std::mem::swap` is that this function doesn't require `&mut` reference. /// + /// # Panics + /// + /// This function will panic if `self` and `other` are different `Cell`s that partially overlap. + /// (Using just standard library methods, it is impossible to create such partially overlapping `Cell`s. + /// However, unsafe code is allowed to e.g. create two `&Cell<[i32; 2]>` that partially overlap.) + /// /// # Examples /// /// ``` @@ -430,14 +437,20 @@ impl Cell { #[stable(feature = "move_cell", since = "1.17.0")] pub fn swap(&self, other: &Self) { if ptr::eq(self, other) { + // Swapping wouldn't change anything. return; } + if !is_nonoverlapping(self, other, 1) { + // See for why we need to stop here. + panic!("`Cell::swap` on overlapping non-identical `Cell`s"); + } // SAFETY: This can be risky if called from separate threads, but `Cell` // is `!Sync` so this won't happen. This also won't invalidate any // pointers since `Cell` makes sure nothing else will be pointing into - // either of these `Cell`s. + // either of these `Cell`s. We also excluded shenanigans like partially overlapping `Cell`s, + // so `swap` will just properly copy two full values of type `T` back and forth. unsafe { - ptr::swap(self.value.get(), other.value.get()); + ptr::swap_nonoverlapping(self.value.get(), other.value.get(), 1); } } From 26cfd211fb00f7f9076bf12eb3eef4da341720d3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 14 Aug 2023 09:53:53 +0200 Subject: [PATCH 2/3] simplify is_nonoverlapping a bit --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 9ef2c7cde02eb..0442e1639bac4 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2568,7 +2568,7 @@ pub(crate) fn is_nonoverlapping(src: *const T, dst: *const T, count: usize) - let size = mem::size_of::() .checked_mul(count) .expect("is_nonoverlapping: `size_of::() * count` overflows a usize"); - let diff = if src_usize > dst_usize { src_usize - dst_usize } else { dst_usize - src_usize }; + let diff = src_usize.abs_diff(dst_usize); // If the absolute distance between the ptrs is at least as big as the size of the buffer, // they do not overlap. diff >= size From e7a1e4271d3c65fb707518d89890195faab29efe Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 16 Aug 2023 08:47:06 +0200 Subject: [PATCH 3/3] use mem::swap instead of ptr::swap_nonoverlapping --- library/core/src/cell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 69d4c3768db30..1d572ede9e429 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -450,7 +450,7 @@ impl Cell { // either of these `Cell`s. We also excluded shenanigans like partially overlapping `Cell`s, // so `swap` will just properly copy two full values of type `T` back and forth. unsafe { - ptr::swap_nonoverlapping(self.value.get(), other.value.get(), 1); + mem::swap(&mut *self.value.get(), &mut *other.value.get()); } }