Fix Miri errors for WindowsIter and ExactChunksIter/Mut

Before this commit, running `MIRIFLAGS="-Zmiri-tag-raw-pointers" cargo miri test` caused Miri to report undefined behavior for code using the `WindowsIter`, `ExactChunksIter`, and `ExactChunksIterMut` iterators. This commit fixes the underlying issue. Basically, Miri doesn't like code which uses a reference to an element to access other elements. Before this commit, these iterators wrapped the `ElementsBase` and `ElementsBaseMut` iterators, and they created views from the references returned by those inner iterators. Accessing elements within those views (other than the first element) led to the Miri error, since the view's pointer was derived from a reference to a single element. Now, the iterators wrap `Baseiter` instead, which produces raw pointers instead of references. Although not necessary to satisfy Miri, this commit also changes the `Windows`, `ExactChunks`, and `ExactChunksMut` producers to wrap raw views instead of normal views. This avoids potential confusion regarding which elements are accessible through the views produced by these producers.
rust-ndarray · Mar 9, 2024 · 6df86a3 · 6df86a3
1 parent f124a16
commit 6df86a3
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 31 deletions.
diff --git a/src/impl_views/conversions.rs b/src/impl_views/conversions.rs
@@ -183,6 +183,27 @@ where
     }
 }
 
+/// Private raw array view methods
+impl<A, D> RawArrayView<A, D>
+where
+    D: Dimension,
+{
+    #[inline]
+    pub(crate) fn into_base_iter(self) -> Baseiter<A, D> {
+        unsafe { Baseiter::new(self.ptr.as_ptr(), self.dim, self.strides) }
+    }
+}
+
+impl<A, D> RawArrayViewMut<A, D>
+where
+    D: Dimension,
+{
+    #[inline]
+    pub(crate) fn into_base_iter(self) -> Baseiter<A, D> {
+        unsafe { Baseiter::new(self.ptr.as_ptr(), self.dim, self.strides) }
+    }
+}
+
 /// Private array view methods
 impl<'a, A, D> ArrayView<'a, A, D>
 where

diff --git a/src/iterators/chunks.rs b/src/iterators/chunks.rs
@@ -1,6 +1,7 @@
+use std::marker::PhantomData;
+
 use crate::imp_prelude::*;
-use crate::ElementsBase;
-use crate::ElementsBaseMut;
+use crate::Baseiter;
 use crate::IntoDimension;
 use crate::{Layout, NdProducer};
 
@@ -9,6 +10,7 @@ impl_ndproducer! {
     [Clone => 'a, A, D: Clone ]
     ExactChunks {
         base,
+        life,
         chunk,
         inner_strides,
     }
@@ -23,16 +25,14 @@ impl_ndproducer! {
     }
 }
 
-type BaseProducerRef<'a, A, D> = ArrayView<'a, A, D>;
-type BaseProducerMut<'a, A, D> = ArrayViewMut<'a, A, D>;
-
 /// Exact chunks producer and iterable.
 ///
 /// See [`.exact_chunks()`](ArrayBase::exact_chunks) for more
 /// information.
 //#[derive(Debug)]
 pub struct ExactChunks<'a, A, D> {
-    base: BaseProducerRef<'a, A, D>,
+    base: RawArrayView<A, D>,
+    life: PhantomData<&'a A>,
     chunk: D,
     inner_strides: D,
 }
@@ -41,10 +41,11 @@ impl<'a, A, D: Dimension> ExactChunks<'a, A, D> {
     /// Creates a new exact chunks producer.
     ///
     /// **Panics** if any chunk dimension is zero
-    pub(crate) fn new<E>(mut a: ArrayView<'a, A, D>, chunk: E) -> Self
+    pub(crate) fn new<E>(a: ArrayView<'a, A, D>, chunk: E) -> Self
     where
         E: IntoDimension<Dim = D>,
     {
+        let mut a = a.into_raw_view();
         let chunk = chunk.into_dimension();
         ndassert!(
             a.ndim() == chunk.ndim(),
@@ -59,11 +60,12 @@ impl<'a, A, D: Dimension> ExactChunks<'a, A, D> {
         for i in 0..a.ndim() {
             a.dim[i] /= chunk[i];
         }
-        let inner_strides = a.raw_strides();
+        let inner_strides = a.strides.clone();
         a.strides *= &chunk;
 
         ExactChunks {
             base: a,
+            life: PhantomData,
             chunk,
             inner_strides,
         }
@@ -79,7 +81,8 @@ where
     type IntoIter = ExactChunksIter<'a, A, D>;
     fn into_iter(self) -> Self::IntoIter {
         ExactChunksIter {
-            iter: self.base.into_elements_base(),
+            iter: self.base.into_base_iter(),
+            life: self.life,
             chunk: self.chunk,
             inner_strides: self.inner_strides,
         }
@@ -91,7 +94,8 @@ where
 /// See [`.exact_chunks()`](ArrayBase::exact_chunks) for more
 /// information.
 pub struct ExactChunksIter<'a, A, D> {
-    iter: ElementsBase<'a, A, D>,
+    iter: Baseiter<A, D>,
+    life: PhantomData<&'a A>,
     chunk: D,
     inner_strides: D,
 }
@@ -101,6 +105,7 @@ impl_ndproducer! {
     [Clone => ]
     ExactChunksMut {
         base,
+        life,
         chunk,
         inner_strides,
     }
@@ -122,7 +127,8 @@ impl_ndproducer! {
 /// for more information.
 //#[derive(Debug)]
 pub struct ExactChunksMut<'a, A, D> {
-    base: BaseProducerMut<'a, A, D>,
+    base: RawArrayViewMut<A, D>,
+    life: PhantomData<&'a mut A>,
     chunk: D,
     inner_strides: D,
 }
@@ -131,10 +137,11 @@ impl<'a, A, D: Dimension> ExactChunksMut<'a, A, D> {
     /// Creates a new exact chunks producer.
     ///
     /// **Panics** if any chunk dimension is zero
-    pub(crate) fn new<E>(mut a: ArrayViewMut<'a, A, D>, chunk: E) -> Self
+    pub(crate) fn new<E>(a: ArrayViewMut<'a, A, D>, chunk: E) -> Self
     where
         E: IntoDimension<Dim = D>,
     {
+        let mut a = a.into_raw_view_mut();
         let chunk = chunk.into_dimension();
         ndassert!(
             a.ndim() == chunk.ndim(),
@@ -149,11 +156,12 @@ impl<'a, A, D: Dimension> ExactChunksMut<'a, A, D> {
         for i in 0..a.ndim() {
             a.dim[i] /= chunk[i];
         }
-        let inner_strides = a.raw_strides();
+        let inner_strides = a.strides.clone();
         a.strides *= &chunk;
 
         ExactChunksMut {
             base: a,
+            life: PhantomData,
             chunk,
             inner_strides,
         }
@@ -169,7 +177,8 @@ where
     type IntoIter = ExactChunksIterMut<'a, A, D>;
     fn into_iter(self) -> Self::IntoIter {
         ExactChunksIterMut {
-            iter: self.base.into_elements_base(),
+            iter: self.base.into_base_iter(),
+            life: self.life,
             chunk: self.chunk,
             inner_strides: self.inner_strides,
         }
@@ -181,16 +190,17 @@ impl_iterator! {
     [Clone => 'a, A, D: Clone]
     ExactChunksIter {
         iter,
+        life,
         chunk,
         inner_strides,
     }
     ExactChunksIter<'a, A, D> {
         type Item = ArrayView<'a, A, D>;
 
-        fn item(&mut self, elt) {
+        fn item(&mut self, ptr) {
             unsafe {
                 ArrayView::new_(
-                    elt,
+                    ptr,
                     self.chunk.clone(),
                     self.inner_strides.clone())
             }
@@ -209,10 +219,10 @@ impl_iterator! {
     ExactChunksIterMut<'a, A, D> {
         type Item = ArrayViewMut<'a, A, D>;
 
-        fn item(&mut self, elt) {
+        fn item(&mut self, ptr) {
             unsafe {
                 ArrayViewMut::new_(
-                    elt,
+                    ptr,
                     self.chunk.clone(),
                     self.inner_strides.clone())
             }
@@ -225,7 +235,14 @@ impl_iterator! {
 /// See [`.exact_chunks_mut()`](ArrayBase::exact_chunks_mut)
 /// for more information.
 pub struct ExactChunksIterMut<'a, A, D> {
-    iter: ElementsBaseMut<'a, A, D>,
+    iter: Baseiter<A, D>,
+    life: PhantomData<&'a mut A>,
     chunk: D,
     inner_strides: D,
 }
+
+send_sync_read_only!(ExactChunks);
+send_sync_read_only!(ExactChunksIter);
+
+send_sync_read_write!(ExactChunksMut);
+send_sync_read_write!(ExactChunksIterMut);
diff --git a/src/iterators/macros.rs b/src/iterators/macros.rs
@@ -84,7 +84,7 @@ impl<$($typarm)*> NdProducer for $fulltype {
     }
 
     unsafe fn uget_ptr(&self, i: &Self::Dim) -> *mut A {
-        self.$base.uget_ptr(i)
+        self.$base.uget_ptr(i) as *mut _
     }
 
     fn stride_of(&self, axis: Axis) -> isize {

diff --git a/src/iterators/windows.rs b/src/iterators/windows.rs
@@ -1,4 +1,6 @@
-use super::ElementsBase;
+use std::marker::PhantomData;
+
+use super::Baseiter;
 use crate::imp_prelude::*;
 use crate::IntoDimension;
 use crate::Layout;
@@ -10,7 +12,8 @@ use crate::Slice;
 /// See [`.windows()`](ArrayBase::windows) for more
 /// information.
 pub struct Windows<'a, A, D> {
-    base: ArrayView<'a, A, D>,
+    base: RawArrayView<A, D>,
+    life: PhantomData<&'a A>,
     window: D,
     strides: D,
 }
@@ -74,7 +77,8 @@ impl<'a, A, D: Dimension> Windows<'a, A, D> {
         });
 
         Windows {
-            base,
+            base: base.into_raw_view(),
+            life: PhantomData,
             window,
             strides: window_strides,
         }
@@ -86,6 +90,7 @@ impl_ndproducer! {
     [Clone => 'a, A, D: Clone ]
     Windows {
         base,
+        life,
         window,
         strides,
     }
@@ -109,7 +114,8 @@ where
     type IntoIter = WindowsIter<'a, A, D>;
     fn into_iter(self) -> Self::IntoIter {
         WindowsIter {
-            iter: self.base.into_elements_base(),
+            iter: self.base.into_base_iter(),
+            life: self.life,
             window: self.window,
             strides: self.strides,
         }
@@ -121,7 +127,8 @@ where
 /// See [`.windows()`](ArrayBase::windows) for more
 /// information.
 pub struct WindowsIter<'a, A, D> {
-    iter: ElementsBase<'a, A, D>,
+    iter: Baseiter<A, D>,
+    life: PhantomData<&'a A>,
     window: D,
     strides: D,
 }
@@ -131,19 +138,23 @@ impl_iterator! {
     [Clone => 'a, A, D: Clone]
     WindowsIter {
         iter,
+        life,
         window,
         strides,
     }
     WindowsIter<'a, A, D> {
         type Item = ArrayView<'a, A, D>;
 
-        fn item(&mut self, elt) {
+        fn item(&mut self, ptr) {
             unsafe {
                 ArrayView::new_(
-                    elt,
+                    ptr,
                     self.window.clone(),
                     self.strides.clone())
             }
         }
     }
 }
+
+send_sync_read_only!(Windows);
+send_sync_read_only!(WindowsIter);
diff --git a/src/lib.rs b/src/lib.rs
@@ -1559,10 +1559,6 @@ where
         unsafe { ArrayView::new(ptr, dim, strides) }
     }
 
-    fn raw_strides(&self) -> D {
-        self.strides.clone()
-    }
-
     /// Remove array axis `axis` and return the result.
     fn try_remove_axis(self, axis: Axis) -> ArrayBase<S, D::Smaller> {
         let d = self.dim.try_remove_axis(axis);