Skip to content

Commit 8fa1ed8

Browse files
authored
Rollup merge of #97712 - RalfJung:untyped, r=scottmcm
ptr::copy and ptr::swap are doing untyped copies The consensus in #63159 seemed to be that these operations should be "untyped", i.e., they should treat the data as raw bytes, should work when these bytes violate the validity invariant of `T`, and should exactly preserve the initialization state of the bytes that are being copied. This is already somewhat implied by the description of "copying/swapping size*N bytes" (rather than "N instances of `T`"). The implementations mostly already work that way (well, for LLVM's intrinsics the documentation is not precise enough to say what exactly happens to poison, but if this ever gets clarified to something that would *not* perfectly preserve poison, then I strongly assume there will be some way to make a copy that *does* perfectly preserve poison). However, I had to adjust `swap_nonoverlapping`; after ``@scottmcm's`` [recent changes](#94212), that one (sometimes) made a typed copy. (Note that `mem::swap`, which works on mutable references, is unchanged. It is documented as "swapping the values at two mutable locations", which to me strongly indicates that it is indeed typed. It is also safe and can rely on `&mut T` pointing to a valid `T` as part of its safety invariant.) On top of adding a test (that will be run by Miri), this PR then also adjusts the documentation to indeed stably promise the untyped semantics. I assume this means the PR has to go through t-libs (and maybe t-lang?) FCP. Fixes #63159
2 parents 4045ce6 + cb7cd97 commit 8fa1ed8

File tree

3 files changed

+51
-14
lines changed

3 files changed

+51
-14
lines changed

library/core/src/intrinsics.rs

+6
Original file line numberDiff line numberDiff line change
@@ -2356,6 +2356,9 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -
23562356
/// `copy_nonoverlapping` is semantically equivalent to C's [`memcpy`], but
23572357
/// with the argument order swapped.
23582358
///
2359+
/// The copy is "untyped" in the sense that data may be uninitialized or otherwise violate the
2360+
/// requirements of `T`. The initialization state is preserved exactly.
2361+
///
23592362
/// [`memcpy`]: https://en.cppreference.com/w/c/string/byte/memcpy
23602363
///
23612364
/// # Safety
@@ -2461,6 +2464,9 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
24612464
/// order swapped. Copying takes place as if the bytes were copied from `src`
24622465
/// to a temporary array and then copied from the array to `dst`.
24632466
///
2467+
/// The copy is "untyped" in the sense that data may be uninitialized or otherwise violate the
2468+
/// requirements of `T`. The initialization state is preserved exactly.
2469+
///
24642470
/// [`memmove`]: https://en.cppreference.com/w/c/string/byte/memmove
24652471
///
24662472
/// # Safety

library/core/src/ptr/mod.rs

+20-14
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ pub const fn slice_from_raw_parts_mut<T>(data: *mut T, len: usize) -> *mut [T] {
730730
/// Swaps the values at two mutable locations of the same type, without
731731
/// deinitializing either.
732732
///
733-
/// But for the following two exceptions, this function is semantically
733+
/// But for the following exceptions, this function is semantically
734734
/// equivalent to [`mem::swap`]:
735735
///
736736
/// * It operates on raw pointers instead of references. When references are
@@ -740,6 +740,9 @@ pub const fn slice_from_raw_parts_mut<T>(data: *mut T, len: usize) -> *mut [T] {
740740
/// overlapping region of memory from `x` will be used. This is demonstrated
741741
/// in the second example below.
742742
///
743+
/// * The operation is "untyped" in the sense that data may be uninitialized or otherwise violate
744+
/// the requirements of `T`. The initialization state is preserved exactly.
745+
///
743746
/// # Safety
744747
///
745748
/// Behavior is undefined if any of the following conditions are violated:
@@ -816,6 +819,9 @@ pub const unsafe fn swap<T>(x: *mut T, y: *mut T) {
816819
/// Swaps `count * size_of::<T>()` bytes between the two regions of memory
817820
/// beginning at `x` and `y`. The two regions must *not* overlap.
818821
///
822+
/// The operation is "untyped" in the sense that data may be uninitialized or otherwise violate the
823+
/// requirements of `T`. The initialization state is preserved exactly.
824+
///
819825
/// # Safety
820826
///
821827
/// Behavior is undefined if any of the following conditions are violated:
@@ -861,15 +867,15 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
861867
if mem::align_of::<T>() >= mem::align_of::<$ChunkTy>()
862868
&& mem::size_of::<T>() % mem::size_of::<$ChunkTy>() == 0
863869
{
864-
let x: *mut MaybeUninit<$ChunkTy> = x.cast();
865-
let y: *mut MaybeUninit<$ChunkTy> = y.cast();
870+
let x: *mut $ChunkTy = x.cast();
871+
let y: *mut $ChunkTy = y.cast();
866872
let count = count * (mem::size_of::<T>() / mem::size_of::<$ChunkTy>());
867873
// SAFETY: these are the same bytes that the caller promised were
868874
// ok, just typed as `MaybeUninit<ChunkTy>`s instead of as `T`s.
869875
// The `if` condition above ensures that we're not violating
870876
// alignment requirements, and that the division is exact so
871877
// that we don't lose any bytes off the end.
872-
return unsafe { swap_nonoverlapping_simple(x, y, count) };
878+
return unsafe { swap_nonoverlapping_simple_untyped(x, y, count) };
873879
}
874880
};
875881
}
@@ -902,7 +908,7 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
902908
}
903909

904910
// SAFETY: Same preconditions as this function
905-
unsafe { swap_nonoverlapping_simple(x, y, count) }
911+
unsafe { swap_nonoverlapping_simple_untyped(x, y, count) }
906912
}
907913

908914
/// Same behaviour and safety conditions as [`swap_nonoverlapping`]
@@ -911,17 +917,17 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
911917
/// `swap_nonoverlapping` tries to use) so no need to manually SIMD it.
912918
#[inline]
913919
#[rustc_const_unstable(feature = "const_swap", issue = "83163")]
914-
const unsafe fn swap_nonoverlapping_simple<T>(x: *mut T, y: *mut T, count: usize) {
920+
const unsafe fn swap_nonoverlapping_simple_untyped<T>(x: *mut T, y: *mut T, count: usize) {
921+
let x = x.cast::<MaybeUninit<T>>();
922+
let y = y.cast::<MaybeUninit<T>>();
915923
let mut i = 0;
916924
while i < count {
917-
let x: &mut T =
918-
// SAFETY: By precondition, `i` is in-bounds because it's below `n`
919-
unsafe { &mut *x.add(i) };
920-
let y: &mut T =
921-
// SAFETY: By precondition, `i` is in-bounds because it's below `n`
922-
// and it's distinct from `x` since the ranges are non-overlapping
923-
unsafe { &mut *y.add(i) };
924-
mem::swap_simple(x, y);
925+
// SAFETY: By precondition, `i` is in-bounds because it's below `n`
926+
let x = unsafe { &mut *x.add(i) };
927+
// SAFETY: By precondition, `i` is in-bounds because it's below `n`
928+
// and it's distinct from `x` since the ranges are non-overlapping
929+
let y = unsafe { &mut *y.add(i) };
930+
mem::swap_simple::<MaybeUninit<T>>(x, y);
925931

926932
i += 1;
927933
}

library/core/tests/ptr.rs

+25
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,31 @@ fn nonnull_tagged_pointer_with_provenance() {
781781
}
782782
}
783783

784+
#[test]
785+
fn swap_copy_untyped() {
786+
// We call `{swap,copy}{,_nonoverlapping}` at `bool` type on data that is not a valid bool.
787+
// These should all do untyped copies, so this should work fine.
788+
let mut x = 5u8;
789+
let mut y = 6u8;
790+
791+
let ptr1 = &mut x as *mut u8 as *mut bool;
792+
let ptr2 = &mut y as *mut u8 as *mut bool;
793+
794+
unsafe {
795+
ptr::swap(ptr1, ptr2);
796+
ptr::swap_nonoverlapping(ptr1, ptr2, 1);
797+
}
798+
assert_eq!(x, 5);
799+
assert_eq!(y, 6);
800+
801+
unsafe {
802+
ptr::copy(ptr1, ptr2, 1);
803+
ptr::copy_nonoverlapping(ptr1, ptr2, 1);
804+
}
805+
assert_eq!(x, 5);
806+
assert_eq!(y, 5);
807+
}
808+
784809
#[test]
785810
fn test_const_copy() {
786811
const {

0 commit comments

Comments
 (0)