diff --git a/CHANGELOG.md b/CHANGELOG.md index a5815358b..1efb05ef3 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/src/array.rs b/src/array.rs index 737101e9a..b00e3f01c 100644 --- a/src/array.rs +++ b/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, NotContiguousError> + fn to_vec(&self) -> Result, AsSliceError> where T: Element, D: Dimension; @@ -1503,7 +1513,7 @@ impl<'py, T, D> PyArrayMethods<'py, T, D> for Bound<'py, PyArray> { unsafe { self.cast_unchecked() } } - fn to_vec(&self) -> Result, NotContiguousError> + fn to_vec(&self) -> Result, AsSliceError> where T: Element, D: Dimension, diff --git a/src/borrow/mod.rs b/src/borrow/mod.rs index c08b74017..a22544d7f 100644 --- a/src/borrow/mod.rs +++ b/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() } } diff --git a/src/error.rs b/src/error.rs index dbe852c61..1c471eec9 100644 --- a/src/error.rs +++ b/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)] diff --git a/src/lib.rs b/src/lib.rs index 446615e99..2b27dce67 100644 --- a/src/lib.rs +++ b/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 { diff --git a/src/untyped_array.rs b/src/untyped_array.rs index df443ec6c..2db8c1e6a 100644 --- a/src/untyped_array.rs +++ b/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::::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::>()?; + /// 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. /// diff --git a/tests/array.rs b/tests/array.rs index 4603d3097..eadce8c9d 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -32,6 +32,17 @@ fn not_contiguous_array(py: Python<'_>) -> Bound<'_, PyArray1> { .unwrap() } +fn not_aligned_array(py: Python<'_>) -> Bound<'_, PyArray1> { + 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> = { + 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::::new() + ); }); }