mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-22 14:55:26 +00:00
Auto merge of #114795 - RalfJung:cell-swap, r=dtolnay
make Cell::swap panic if the Cells partially overlap The following function ought to be sound: ```rust fn as_cell_of_array<T, const N: usize>(c: &[Cell<T>; N]) -> &Cell<[T; N]> { unsafe { transmute(c) } } ``` However, due to `Cell::swap`, it currently is not -- safe code can [cause a use-after-free](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c9415799722d985ff7d2c2c997b724ca). This PR fixes that. Fixes https://github.com/rust-lang/rust/issues/80778
This commit is contained in:
commit
f6faef4475
@ -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<T> Cell<T> {
|
||||
/// 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<T> Cell<T> {
|
||||
#[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 <https://github.com/rust-lang/rust/issues/80778> 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());
|
||||
mem::swap(&mut *self.value.get(), &mut *other.value.get());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -2567,7 +2567,7 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -
|
||||
let size = mem::size_of::<T>()
|
||||
.checked_mul(count)
|
||||
.expect("is_nonoverlapping: `size_of::<T>() * 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
|
||||
|
Loading…
Reference in New Issue
Block a user