From 6df86a3a778a6c849ee0b72576e259d3bb2ea135 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Thu, 23 Dec 2021 19:11:48 -0500 Subject: [PATCH] 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. --- src/impl_views/conversions.rs | 21 +++++++++++++ src/iterators/chunks.rs | 55 +++++++++++++++++++++++------------ src/iterators/macros.rs | 2 +- src/iterators/windows.rs | 25 +++++++++++----- src/lib.rs | 4 --- 5 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/impl_views/conversions.rs b/src/impl_views/conversions.rs index 1e9a1499f..ca571a761 100644 --- a/src/impl_views/conversions.rs +++ b/src/impl_views/conversions.rs @@ -183,6 +183,27 @@ where } } +/// Private raw array view methods +impl RawArrayView +where + D: Dimension, +{ + #[inline] + pub(crate) fn into_base_iter(self) -> Baseiter { + unsafe { Baseiter::new(self.ptr.as_ptr(), self.dim, self.strides) } + } +} + +impl RawArrayViewMut +where + D: Dimension, +{ + #[inline] + pub(crate) fn into_base_iter(self) -> Baseiter { + 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 index ba0e789fb..9cf06b55f 100644 --- 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, + 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(mut a: ArrayView<'a, A, D>, chunk: E) -> Self + pub(crate) fn new(a: ArrayView<'a, A, D>, chunk: E) -> Self where E: IntoDimension, { + 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, + 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, + 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(mut a: ArrayViewMut<'a, A, D>, chunk: E) -> Self + pub(crate) fn new(a: ArrayViewMut<'a, A, D>, chunk: E) -> Self where E: IntoDimension, { + 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, + 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 index ff4b3bb93..7fbe410fe 100644 --- 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 index 6238d0ed1..9140f43b9 100644 --- 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, + 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, + 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 index e80f2651a..a651ae58c 100644 --- 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 { let d = self.dim.try_remove_axis(axis);