Skip to content

Commit

Permalink
Remove alignment restriction on RawArrayView/Mut
Browse files Browse the repository at this point in the history
  • Loading branch information
jturner314 authored and bluss committed Oct 9, 2019
1 parent ecb7643 commit 6b38810
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 54 deletions.
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);
}

0 comments on commit 6b38810

Please sign in to comment.