From bec5d37ee19c33bec96ed235f0197317d3728e0a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Feb 2020 14:44:16 +0100 Subject: [PATCH 1/3] debug_assert a few more raw pointer methods --- src/libcore/intrinsics.rs | 9 +++++---- src/libcore/ptr/mod.rs | 12 +++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 2cee23a5c752c..9a23b54dfa0a5 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1423,13 +1423,14 @@ pub(crate) fn is_aligned_and_not_null(ptr: *const T) -> bool { } /// Checks whether the regions of memory starting at `src` and `dst` of size -/// `count * size_of::()` overlap. -fn overlaps(src: *const T, dst: *const T, count: usize) -> bool { +/// `count * size_of::()` do *not* overlap. +pub(crate) fn is_nonoverlapping(src: *const T, dst: *const T, count: usize) -> bool { let src_usize = src as usize; let dst_usize = dst as usize; let size = mem::size_of::().checked_mul(count).unwrap(); let diff = if src_usize > dst_usize { src_usize - dst_usize } else { dst_usize - src_usize }; - size > diff + let overlaps = size > diff; + !overlaps } /// Copies `count * size_of::()` bytes from `src` to `dst`. The source @@ -1524,7 +1525,7 @@ pub unsafe fn copy_nonoverlapping(src: *const T, dst: *mut T, count: usize) { debug_assert!(is_aligned_and_not_null(src), "attempt to copy from unaligned or null pointer"); debug_assert!(is_aligned_and_not_null(dst), "attempt to copy to unaligned or null pointer"); - debug_assert!(!overlaps(src, dst, count), "attempt to copy to overlapping memory"); + debug_assert!(is_nonoverlapping(src, dst, count), "attempt to copy to overlapping memory"); copy_nonoverlapping(src, dst, count) } diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 0ee50966f968c..ce21773165a6a 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -72,7 +72,7 @@ use crate::cmp::Ordering; use crate::fmt; use crate::hash; -use crate::intrinsics; +use crate::intrinsics::{self, is_aligned_and_not_null, is_nonoverlapping}; use crate::mem::{self, MaybeUninit}; #[stable(feature = "rust1", since = "1.0.0")] @@ -389,6 +389,10 @@ pub unsafe fn swap(x: *mut T, y: *mut T) { #[inline] #[stable(feature = "swap_nonoverlapping", since = "1.27.0")] pub unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { + debug_assert!(is_aligned_and_not_null(x), "attempt to swap unaligned or null pointer"); + debug_assert!(is_aligned_and_not_null(y), "attempt to swap unaligned or null pointer"); + debug_assert!(is_nonoverlapping(x, y, count), "attempt to swap overlapping memory"); + let x = x as *mut u8; let y = y as *mut u8; let len = mem::size_of::() * count; @@ -612,6 +616,7 @@ pub unsafe fn replace(dst: *mut T, mut src: T) -> T { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn read(src: *const T) -> T { + // `copy_nonoverlapping` takes care of debug_assert. let mut tmp = MaybeUninit::::uninit(); copy_nonoverlapping(src, tmp.as_mut_ptr(), 1); tmp.assume_init() @@ -703,6 +708,7 @@ pub unsafe fn read(src: *const T) -> T { #[inline] #[stable(feature = "ptr_unaligned", since = "1.17.0")] pub unsafe fn read_unaligned(src: *const T) -> T { + // `copy_nonoverlapping` takes care of debug_assert. let mut tmp = MaybeUninit::::uninit(); copy_nonoverlapping(src as *const u8, tmp.as_mut_ptr() as *mut u8, mem::size_of::()); tmp.assume_init() @@ -795,6 +801,7 @@ pub unsafe fn read_unaligned(src: *const T) -> T { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn write(dst: *mut T, src: T) { + debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer"); intrinsics::move_val_init(&mut *dst, src) } @@ -887,6 +894,7 @@ pub unsafe fn write(dst: *mut T, src: T) { #[inline] #[stable(feature = "ptr_unaligned", since = "1.17.0")] pub unsafe fn write_unaligned(dst: *mut T, src: T) { + // `copy_nonoverlapping` takes care of debug_assert. copy_nonoverlapping(&src as *const T as *const u8, dst as *mut u8, mem::size_of::()); mem::forget(src); } @@ -956,6 +964,7 @@ pub unsafe fn write_unaligned(dst: *mut T, src: T) { #[inline] #[stable(feature = "volatile", since = "1.9.0")] pub unsafe fn read_volatile(src: *const T) -> T { + debug_assert!(is_aligned_and_not_null(src), "attempt to read from unaligned or null pointer"); intrinsics::volatile_load(src) } @@ -1024,6 +1033,7 @@ pub unsafe fn read_volatile(src: *const T) -> T { #[inline] #[stable(feature = "volatile", since = "1.9.0")] pub unsafe fn write_volatile(dst: *mut T, src: T) { + debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer"); intrinsics::volatile_store(dst, src); } From d1d0de94db77b2165a716030adf56862a7b7617a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 27 Feb 2020 09:43:20 +0100 Subject: [PATCH 2/3] disable debug assertion in ptr::write for now --- src/libcore/ptr/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index ce21773165a6a..52af439d853d5 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -801,7 +801,9 @@ pub unsafe fn read_unaligned(src: *const T) -> T { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn write(dst: *mut T, src: T) { - debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer"); + // FIXME: the debug assertion here causes codegen test failures on some architectures. + // See . + // debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer"); intrinsics::move_val_init(&mut *dst, src) } From 8d3b3063f6a91eb4d421998aea3c7dbcefa34f11 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 27 Feb 2020 09:45:32 +0100 Subject: [PATCH 3/3] avoid a negation in is_nonoverlapping --- src/libcore/intrinsics.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 9a23b54dfa0a5..ecee33c2e6f32 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1429,8 +1429,9 @@ pub(crate) fn is_nonoverlapping(src: *const T, dst: *const T, count: usize) - let dst_usize = dst as usize; let size = mem::size_of::().checked_mul(count).unwrap(); let diff = if src_usize > dst_usize { src_usize - dst_usize } else { dst_usize - src_usize }; - let overlaps = size > diff; - !overlaps + // If the absolute distance between the ptrs is at least as big as the size of the buffer, + // they do not overlap. + diff >= size } /// Copies `count * size_of::()` bytes from `src` to `dst`. The source