Handle misaligned arrays in as_slice (#525)

Handle misaligned arrays in `as_slice`

Previously, `as_slice` would only check that arrays are contiguous.
While misaligned non-empty arrays remain problematic across the library
(`get`, for example, would still be invalid), this change begins
handling this situation in a function that already returns `Result`.

As a convenience, the empty slice is returned for zero-length arrays,
regardless of the alignment of the underlying pointer.  Misaligned
zero-length pointers can fairly easily arise from allocator
optimisations. For example, CPython reliably returned an odd-address
pointer in `bytearray()` in (at least) CPython 3.14.0 on Linux x86-64,
which caused empty Numpy arrays passing through Pickle protocol 5 (with
the default handling of its `PickleBuffer`s) to be backed by a
misaligned pointer for multi-byte aligned dtypes.

Author: jakelishman

CHANGELOG.md

@@ -2,6 +2,7 @@
 - v0.28.0
   - Fix mismatched behavior between `PyArrayLike1` and `PyArrayLike2` when used with floats ([#520](https://github.com/PyO3/rust-numpy/pull/520))
   - Add ownership-moving conversions into `PyReadonlyArray` and `PyReadwriteArray` ([#524](https://github.com/PyO3/rust-numpy/pull/524))
+  - Fix UB when calling `PyArray::as_slice` and `as_slice_mut` on misaligned arrays ([#525](https://github.com/PyO3/rust-numpy/pull/525))
 
 - v0.27.1
   - Bump ndarray dependency to v0.17. ([#516](https://github.com/PyO3/rust-numpy/pull/516))

src/array.rs

@@ -26,7 +26,7 @@ use crate::cold;
 use crate::convert::{ArrayExt, IntoPyArray, NpyIndex, ToNpyDims, ToPyArray};
 use crate::dtype::{Element, PyArrayDescrMethods};
 use crate::error::{
-    BorrowError, DimensionalityError, FromVecError, IgnoreError, NotContiguousError, TypeError,
+    AsSliceError, BorrowError, DimensionalityError, FromVecError, IgnoreError, TypeError,
     DIMENSIONALITY_MISMATCH_ERR, MAX_DIMENSIONALITY_ERR,
 };
 use crate::npyffi::{self, npy_intp, NPY_ORDER, PY_ARRAY_API};
@@ -739,15 +739,20 @@ pub trait PyArrayMethods<'py, T, D>: PyUntypedArrayMethods<'py> + Sized {
     /// or concurrently modified by Python or other native code.
     ///
     /// Please consider the safe alternative [`PyReadonlyArray::as_slice`].
-    unsafe fn as_slice(&self) -> Result<&[T], NotContiguousError>
+    unsafe fn as_slice(&self) -> Result<&[T], AsSliceError>
     where
         T: Element,
         D: Dimension,
     {
-        if self.is_contiguous() {
-            Ok(slice::from_raw_parts(self.data(), self.len()))
+        let len = self.len();
+        if len == 0 {
+            // We can still produce a slice over zero objects regardless of whether
+            // the underlying pointer is aligned or not.
+            Ok(&[])
+        } else if self.is_aligned() && self.is_contiguous() {
+            Ok(slice::from_raw_parts(self.data(), len))
         } else {
-            Err(NotContiguousError)
+            Err(AsSliceError)
         }
     }
 
@@ -761,15 +766,20 @@ pub trait PyArrayMethods<'py, T, D>: PyUntypedArrayMethods<'py> + Sized {
     ///
     /// Please consider the safe alternative [`PyReadwriteArray::as_slice_mut`].
     #[allow(clippy::mut_from_ref)]
-    unsafe fn as_slice_mut(&self) -> Result<&mut [T], NotContiguousError>
+    unsafe fn as_slice_mut(&self) -> Result<&mut [T], AsSliceError>
     where
         T: Element,
         D: Dimension,
     {
-        if self.is_contiguous() {
-            Ok(slice::from_raw_parts_mut(self.data(), self.len()))
+        let len = self.len();
+        if len == 0 {
+            // We can still produce a slice over zero objects regardless of whether
+            // the underlying pointer is aligned or not.
+            Ok(&mut [])
+        } else if self.is_aligned() && self.is_contiguous() {
+            Ok(slice::from_raw_parts_mut(self.data(), len))
         } else {
-            Err(NotContiguousError)
+            Err(AsSliceError)
         }
     }
 
@@ -951,7 +961,7 @@ pub trait PyArrayMethods<'py, T, D>: PyUntypedArrayMethods<'py> + Sized {
     /// })
     /// # }
     /// ```
-    fn to_vec(&self) -> Result<Vec<T>, NotContiguousError>
+    fn to_vec(&self) -> Result<Vec<T>, AsSliceError>
     where
         T: Element,
         D: Dimension;
@@ -1503,7 +1513,7 @@ impl<'py, T, D> PyArrayMethods<'py, T, D> for Bound<'py, PyArray<T, D>> {
         unsafe { self.cast_unchecked() }
     }
 
-    fn to_vec(&self) -> Result<Vec<T>, NotContiguousError>
+    fn to_vec(&self) -> Result<Vec<T>, AsSliceError>
     where
         T: Element,
         D: Dimension,

src/borrow/mod.rs

@@ -180,7 +180,7 @@ use pyo3::{Borrowed, Bound, CastError, FromPyObject, PyAny, PyResult};
 use crate::array::{PyArray, PyArrayMethods};
 use crate::convert::NpyIndex;
 use crate::dtype::Element;
-use crate::error::{BorrowError, NotContiguousError};
+use crate::error::{AsSliceError, BorrowError};
 use crate::npyffi::flags;
 use crate::untyped_array::PyUntypedArrayMethods;
 
@@ -268,7 +268,7 @@ where
 
     /// Provide an immutable slice view of the interior of the NumPy array if it is contiguous.
     #[inline(always)]
-    pub fn as_slice(&self) -> Result<&[T], NotContiguousError> {
+    pub fn as_slice(&self) -> Result<&[T], AsSliceError> {
         // SAFETY: Global borrow flags ensure aliasing discipline.
         unsafe { self.array.as_slice() }
     }
@@ -511,7 +511,7 @@ where
 
     /// Provide a mutable slice view of the interior of the NumPy array if it is contiguous.
     #[inline(always)]
-    pub fn as_slice_mut(&mut self) -> Result<&mut [T], NotContiguousError> {
+    pub fn as_slice_mut(&mut self) -> Result<&mut [T], AsSliceError> {
         // SAFETY: Global borrow flags ensure aliasing discipline.
         unsafe { self.array.as_slice_mut() }
     }

src/error.rs

@@ -141,17 +141,15 @@ impl fmt::Display for FromVecError {
 
 impl_pyerr!(FromVecError);
 
-/// Represents that the given array is not contiguous.
+/// Represents that the given array is not compatible with viewing as a Rust slice.
 #[derive(Debug)]
-pub struct NotContiguousError;
-
-impl fmt::Display for NotContiguousError {
+pub struct AsSliceError;
+impl fmt::Display for AsSliceError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "The given array is not contiguous")
+        write!(f, "The given array is not contiguous or is misaligned.")
     }
 }
-
-impl_pyerr!(NotContiguousError);
+impl_pyerr!(AsSliceError);
 
 /// Inidcates why borrowing an array failed.
 #[derive(Debug)]

src/lib.rs

@@ -106,7 +106,7 @@ pub use crate::borrow::{
 };
 pub use crate::convert::{IntoPyArray, NpyIndex, ToNpyDims, ToPyArray};
 pub use crate::dtype::{dtype, Complex32, Complex64, Element, PyArrayDescr, PyArrayDescrMethods};
-pub use crate::error::{BorrowError, FromVecError, NotContiguousError};
+pub use crate::error::{AsSliceError, BorrowError, FromVecError};
 pub use crate::npyffi::{PY_ARRAY_API, PY_UFUNC_API};
 pub use crate::strings::{PyFixedString, PyFixedUnicode};
 pub use crate::sum_products::{dot, einsum, inner};
@@ -130,6 +130,11 @@ pub mod prelude {
     pub use crate::untyped_array::PyUntypedArrayMethods;
 }
 
+/// Deprecated type alias to [`AsSliceError`].  The new name is preferred because arrays might also
+/// fail to view as a slice due to misalignment.
+#[deprecated(note = "use AsSliceError instead", since = "0.28.0")]
+pub type NonContiguousError = AsSliceError;
+
 #[cfg(doctest)]
 mod doctest {
     macro_rules! doc_comment {

src/untyped_array.rs

@@ -102,6 +102,39 @@ pub trait PyUntypedArrayMethods<'py>: Sealed {
     /// [PyArray_DTYPE]: https://numpy.org/doc/stable/reference/c-api/array.html#c.PyArray_DTYPE
     fn dtype(&self) -> Bound<'py, PyArrayDescr>;
 
+    /// Returns `true` if the internal data of the array is aligned for the dtype.
+    ///
+    /// Note that NumPy considers zero-length data to be aligned regardless of the base pointer,
+    /// which is a weaker condition than Rust's slice guarantees.  [PyArrayMethods::as_slice] will
+    /// safely handle the case of a misaligned zero-length array.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// use numpy::{PyArray1, PyUntypedArrayMethods};
+    /// use pyo3::{types::{IntoPyDict, PyAnyMethods}, Python, ffi::c_str};
+    ///
+    /// # fn main() -> pyo3::PyResult<()> {
+    /// Python::attach(|py| {
+    ///     let array = PyArray1::<u16>::zeros(py, 8, false);
+    ///     assert!(array.is_aligned());
+    ///
+    ///     let view = py
+    ///         .eval(
+    ///             c_str!("array.view('u1')[1:-1].view('u2')"),
+    ///             None,
+    ///             Some(&[("array", array)].into_py_dict(py)?),
+    ///         )?
+    ///         .cast_into::<PyArray1<u16>>()?;
+    ///     assert!(!view.is_aligned());
+    /// #   Ok(())
+    /// })
+    /// # }
+    /// ```
+    fn is_aligned(&self) -> bool {
+        unsafe { check_flags(&*self.as_array_ptr(), npyffi::NPY_ARRAY_ALIGNED) }
+    }
+
     /// Returns `true` if the internal data of the array is contiguous,
     /// indepedently of whether C-style/row-major or Fortran-style/column-major.
     ///

tests/array.rs

@@ -32,6 +32,17 @@ fn not_contiguous_array(py: Python<'_>) -> Bound<'_, PyArray1<i32>> {
     .unwrap()
 }
 
+fn not_aligned_array(py: Python<'_>) -> Bound<'_, PyArray1<u16>> {
+    py.eval(
+        c_str!("np.zeros(8, dtype='u2').view('u1')[1:-1].view('u2')"),
+        None,
+        Some(&get_np_locals(py)),
+    )
+    .unwrap()
+    .cast_into()
+    .unwrap()
+}
+
 #[test]
 fn new_c_order() {
     Python::attach(|py| {
@@ -51,6 +62,7 @@ fn new_c_order() {
         assert!(arr.is_contiguous());
         assert!(arr.is_c_contiguous());
         assert!(!arr.is_fortran_contiguous());
+        assert!(arr.is_aligned());
     });
 }
 
@@ -73,6 +85,7 @@ fn new_fortran_order() {
         assert!(arr.is_contiguous());
         assert!(!arr.is_c_contiguous());
         assert!(arr.is_fortran_contiguous());
+        assert!(arr.is_aligned());
     });
 }
 
@@ -179,7 +192,52 @@ fn as_slice() {
 
         let not_contiguous = not_contiguous_array(py);
         let err = not_contiguous.readonly().as_slice().unwrap_err();
-        assert_eq!(err.to_string(), "The given array is not contiguous");
+        assert_eq!(
+            err.to_string(),
+            "The given array is not contiguous or is misaligned."
+        );
+        let err = not_contiguous.readwrite().as_slice_mut().unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "The given array is not contiguous or is misaligned."
+        );
+
+        let not_aligned = not_aligned_array(py);
+        assert!(!not_aligned.is_aligned());
+        let err = not_aligned.readonly().as_slice().unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "The given array is not contiguous or is misaligned."
+        );
+        let err = not_aligned.readwrite().as_slice_mut().unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "The given array is not contiguous or is misaligned."
+        );
+
+        let misaligned_empty: Bound<'_, PyArray1<u16>> = {
+            let arr = not_aligned_array(py);
+            py.eval(
+                c_str!("arr[0:0]"),
+                None,
+                Some(&[("arr", arr)].into_py_dict(py).unwrap()),
+            )
+            .unwrap()
+            .cast_into()
+            .unwrap()
+        };
+        assert_eq!(
+            misaligned_empty.readonly().as_slice().unwrap(),
+            &[] as &[u16]
+        );
+        assert_eq!(
+            misaligned_empty.readwrite().as_slice_mut().unwrap(),
+            &mut [] as &mut [u16]
+        );
+        assert_eq!(
+            misaligned_empty.readonly().to_vec().unwrap(),
+            Vec::<u16>::new()
+        );
     });
 }