Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove alignment restriction on RawArrayView/Mut #738

Merged
merged 1 commit into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 22 additions & 21 deletions src/impl_raw_views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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::<A, _>(&dim, &strides).unwrap();
}
RawArrayView::new_(ptr, dim, strides)
Expand All @@ -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)
}

Expand Down Expand Up @@ -130,12 +134,6 @@ where
"size mismatch in raw view cast"
);
let ptr = self.ptr.cast::<B>();
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) }
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::<A, _>(&dim, &strides).unwrap();
}
RawArrayViewMut::new_(ptr, dim, strides)
Expand All @@ -220,19 +217,29 @@ 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)
}

/// Converts to a mutable view of the array.
///
/// **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)
}

Expand Down Expand Up @@ -267,12 +274,6 @@ where
"size mismatch in raw view cast"
);
let ptr = self.ptr.cast::<B>();
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) }
}
}
21 changes: 11 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -1177,13 +1179,13 @@ pub type Ixs = isize;
// `.offset()` at all, even by zero bytes, but the implementation of
// `Vec<A>` 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
Expand All @@ -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<S::Elem>,
/// The lengths of the axes.
dim: D,
Expand Down Expand Up @@ -1331,7 +1333,7 @@ pub type ArrayViewMut<'a, A, D> = ArrayBase<ViewRepr<&'a mut A>, 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
///
Expand All @@ -1356,8 +1358,7 @@ pub type RawArrayView<A, D> = ArrayBase<RawViewRepr<*const A>, 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
///
Expand Down
43 changes: 20 additions & 23 deletions tests/raw_views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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::<A>() * (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::<B>() == 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::<B>();
#[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);
}