From 6b388101d05e124217b46d04ec0a6332fa028adf Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Wed, 2 Oct 2019 11:11:18 -0400 Subject: [PATCH] Remove alignment restriction on RawArrayView/Mut --- src/impl_raw_views.rs | 43 ++++++++++++++++++++++--------------------- src/lib.rs | 21 +++++++++++---------- tests/raw_views.rs | 43 ++++++++++++++++++++----------------------- 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/src/impl_raw_views.rs b/src/impl_raw_views.rs index 643753e7d..d10c7909e 100644 --- a/src/impl_raw_views.rs +++ b/src/impl_raw_views.rs @@ -33,8 +33,8 @@ where /// /// Unsafe because caller is responsible for ensuring all of the following: /// - /// * `ptr` must be non-null and aligned, and it must be safe to - /// [`.offset()`] `ptr` by zero. + /// * `ptr` must be non-null, and it must be safe to [`.offset()`] `ptr` by + /// zero. /// /// * It must be safe to [`.offset()`] the pointer repeatedly along all /// axes and calculate the `count`s for the `.offset()` calls without @@ -70,7 +70,6 @@ where let strides = shape.strides; if cfg!(debug_assertions) { assert!(!ptr.is_null(), "The pointer must be non-null."); - assert!(is_aligned(ptr), "The pointer must be aligned."); dimension::max_abs_offset_check_overflow::(&dim, &strides).unwrap(); } RawArrayView::new_(ptr, dim, strides) @@ -80,9 +79,14 @@ where /// /// **Warning** from a safety standpoint, this is equivalent to /// dereferencing a raw pointer for every element in the array. You must - /// ensure that all of the data is valid and choose the correct lifetime. + /// ensure that all of the data is valid, ensure that the pointer is + /// aligned, and choose the correct lifetime. #[inline] pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> { + debug_assert!( + is_aligned(self.ptr.as_ptr()), + "The pointer must be aligned." + ); ArrayView::new(self.ptr, self.dim, self.strides) } @@ -130,12 +134,6 @@ where "size mismatch in raw view cast" ); let ptr = self.ptr.cast::(); - debug_assert!( - is_aligned(ptr.as_ptr()), - "alignment mismatch in raw view cast" - ); - /* Alignment checked with debug assertion: alignment could be dynamically correct, - * and we don't have a check that compiles out for that. */ unsafe { RawArrayView::new(ptr, self.dim, self.strides) } } } @@ -167,8 +165,8 @@ where /// /// Unsafe because caller is responsible for ensuring all of the following: /// - /// * `ptr` must be non-null and aligned, and it must be safe to - /// [`.offset()`] `ptr` by zero. + /// * `ptr` must be non-null, and it must be safe to [`.offset()`] `ptr` by + /// zero. /// /// * It must be safe to [`.offset()`] the pointer repeatedly along all /// axes and calculate the `count`s for the `.offset()` calls without @@ -204,7 +202,6 @@ where let strides = shape.strides; if cfg!(debug_assertions) { assert!(!ptr.is_null(), "The pointer must be non-null."); - assert!(is_aligned(ptr), "The pointer must be aligned."); dimension::max_abs_offset_check_overflow::(&dim, &strides).unwrap(); } RawArrayViewMut::new_(ptr, dim, strides) @@ -220,9 +217,14 @@ where /// /// **Warning** from a safety standpoint, this is equivalent to /// dereferencing a raw pointer for every element in the array. You must - /// ensure that all of the data is valid and choose the correct lifetime. + /// ensure that all of the data is valid, ensure that the pointer is + /// aligned, and choose the correct lifetime. #[inline] pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> { + debug_assert!( + is_aligned(self.ptr.as_ptr()), + "The pointer must be aligned." + ); ArrayView::new(self.ptr, self.dim, self.strides) } @@ -230,9 +232,14 @@ where /// /// **Warning** from a safety standpoint, this is equivalent to /// dereferencing a raw pointer for every element in the array. You must - /// ensure that all of the data is valid and choose the correct lifetime. + /// ensure that all of the data is valid, ensure that the pointer is + /// aligned, and choose the correct lifetime. #[inline] pub unsafe fn deref_into_view_mut<'a>(self) -> ArrayViewMut<'a, A, D> { + debug_assert!( + is_aligned(self.ptr.as_ptr()), + "The pointer must be aligned." + ); ArrayViewMut::new(self.ptr, self.dim, self.strides) } @@ -267,12 +274,6 @@ where "size mismatch in raw view cast" ); let ptr = self.ptr.cast::(); - debug_assert!( - is_aligned(ptr.as_ptr()), - "alignment mismatch in raw view cast" - ); - /* Alignment checked with debug assertion: alignment could be dynamically correct, - * and we don't have a check that compiles out for that. */ unsafe { RawArrayViewMut::new(ptr, self.dim, self.strides) } } } diff --git a/src/lib.rs b/src/lib.rs index a164bee93..35d1e1aab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1106,10 +1106,12 @@ pub type Ixs = isize; // `dim`, and `strides` must be exclusively borrowed and not aliased by // multiple indices. // -// 2. `ptr` must be non-null and aligned, and it must be safe to [`.offset()`] -// `ptr` by zero. +// 2. If the type of `data` implements `Data`, then `ptr` must be aligned. // -// 3. It must be safe to [`.offset()`] the pointer repeatedly along all axes +// 3. `ptr` must be non-null, and it must be safe to [`.offset()`] `ptr` by +// zero. +// +// 4. It must be safe to [`.offset()`] the pointer repeatedly along all axes // and calculate the `count`s for the `.offset()` calls without overflow, // even if the array is empty or the elements are zero-sized. // @@ -1177,13 +1179,13 @@ pub type Ixs = isize; // `.offset()` at all, even by zero bytes, but the implementation of // `Vec` does this, so we can too. See rust-lang/rust#54857 for details.) // -// 4. The product of non-zero axis lengths must not exceed `isize::MAX`. (This +// 5. The product of non-zero axis lengths must not exceed `isize::MAX`. (This // also implies that the length of any individual axis must not exceed // `isize::MAX`, and an array can contain at most `isize::MAX` elements.) // This constraint makes various calculations easier because they don't have // to worry about overflow and axis lengths can be freely cast to `isize`. // -// Constraints 2–4 are carefully designed such that if they're upheld for the +// Constraints 2–5 are carefully designed such that if they're upheld for the // array, they're also upheld for any subset of axes of the array as well as // slices/subviews/reshapes of the array. This is important for iterators that // produce subviews (and other similar cases) to be safe without extra (easy to @@ -1209,8 +1211,8 @@ where /// Data buffer / ownership information. (If owned, contains the data /// buffer; if borrowed, contains the lifetime and mutability.) data: S, - /// A non-null and aligned pointer into the buffer held by `data`; may - /// point anywhere in its range. + /// A non-null pointer into the buffer held by `data`; may point anywhere + /// in its range. If `S: Data`, this pointer must be aligned. ptr: std::ptr::NonNull, /// The lengths of the axes. dim: D, @@ -1331,7 +1333,7 @@ pub type ArrayViewMut<'a, A, D> = ArrayBase, D>; /// conversion into an [`ArrayView`]. The relationship between `RawArrayView` /// and [`ArrayView`] is somewhat analogous to the relationship between `*const /// T` and `&T`, but `RawArrayView` has additional requirements that `*const T` -/// does not, such as alignment and non-nullness. +/// does not, such as non-nullness. /// /// [`ArrayView`]: type.ArrayView.html /// @@ -1356,8 +1358,7 @@ pub type RawArrayView = ArrayBase, D>; /// unsafe conversion into an [`ArrayViewMut`]. The relationship between /// `RawArrayViewMut` and [`ArrayViewMut`] is somewhat analogous to the /// relationship between `*mut T` and `&mut T`, but `RawArrayViewMut` has -/// additional requirements that `*mut T` does not, such as alignment and -/// non-nullness. +/// additional requirements that `*mut T` does not, such as non-nullness. /// /// [`ArrayViewMut`]: type.ArrayViewMut.html /// diff --git a/tests/raw_views.rs b/tests/raw_views.rs index 09e01aebd..b63e42926 100644 --- a/tests/raw_views.rs +++ b/tests/raw_views.rs @@ -2,8 +2,6 @@ use ndarray::prelude::*; use ndarray::Zip; use std::cell::Cell; -#[cfg(debug_assertions)] -use std::mem; #[test] fn raw_view_cast_cell() { @@ -59,28 +57,27 @@ fn raw_view_mut_invalid_size_cast() { } #[test] -#[cfg(debug_assertions)] -#[should_panic = "alignment mismatch"] -fn raw_view_invalid_align_cast() { - #[derive(Copy, Clone, Debug)] - #[repr(transparent)] - struct A([u8; 16]); - #[derive(Copy, Clone, Debug)] - #[repr(transparent)] - struct B([f64; 2]); - +fn raw_view_misaligned() { + let data: [u16; 2] = [0x0011, 0x2233]; + let ptr: *const u16 = data.as_ptr(); unsafe { - const LEN: usize = 16; - let mut buffer = [0u8; mem::size_of::() * (LEN + 1)]; - // Take out a slice of buffer as &[A] which is misaligned for B - let mut ptr = buffer.as_mut_ptr(); - if ptr as usize % mem::align_of::() == 0 { - ptr = ptr.add(1); - } - - let view = RawArrayViewMut::from_shape_ptr(LEN, ptr as *mut A); + let misaligned_ptr = (ptr as *const u8).add(1) as *const u16; + RawArrayView::from_shape_ptr(1, misaligned_ptr); + } +} - // misaligned cast - test debug assertion - view.cast::(); +#[test] +#[cfg(debug_assertions)] +#[should_panic = "The pointer must be aligned."] +fn raw_view_deref_into_view_misaligned() { + fn misaligned_deref(data: &[u16; 2]) -> ArrayView1<'_, u16> { + let ptr: *const u16 = data.as_ptr(); + unsafe { + let misaligned_ptr = (ptr as *const u8).add(1) as *const u16; + let raw_view = RawArrayView::from_shape_ptr(1, misaligned_ptr); + raw_view.deref_into_view() + } } + let data: [u16; 2] = [0x0011, 0x2233]; + misaligned_deref(&data); }