Skip to content

Commit

Permalink
Fix Miri errors for WindowsIter and ExactChunksIter/Mut
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jturner314 committed Dec 24, 2021
1 parent a7d1fd6 commit 97c00b9
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 31 deletions.
21 changes: 21 additions & 0 deletions src/impl_views/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 36 additions & 19 deletions src/iterators/chunks.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -9,6 +10,7 @@ impl_ndproducer! {
[Clone => 'a, A, D: Clone ]
ExactChunks {
base,
life,
chunk,
inner_strides,
}
Expand All @@ -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,
}
Expand All @@ -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(),
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -101,6 +105,7 @@ impl_ndproducer! {
[Clone => ]
ExactChunksMut {
base,
life,
chunk,
inner_strides,
}
Expand All @@ -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,
}
Expand All @@ -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(),
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -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())
}
Expand All @@ -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())
}
Expand All @@ -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);
2 changes: 1 addition & 1 deletion src/iterators/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<$($typarm)*> NdProducer for $fulltype {

#[doc(hidden)]
unsafe fn uget_ptr(&self, i: &Self::Dim) -> *mut A {
self.$base.uget_ptr(i)
self.$base.uget_ptr(i) as *mut _
}

#[doc(hidden)]
Expand Down
25 changes: 18 additions & 7 deletions src/iterators/windows.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::ElementsBase;
use std::marker::PhantomData;

use super::Baseiter;
use crate::imp_prelude::*;
use crate::IntoDimension;
use crate::Layout;
Expand All @@ -9,7 +11,8 @@ use crate::NdProducer;
/// 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,
}
Expand Down Expand Up @@ -41,7 +44,8 @@ impl<'a, A, D: Dimension> Windows<'a, A, D> {

unsafe {
Windows {
base: ArrayView::new(a.ptr, size, a.strides),
base: RawArrayView::new(a.ptr, size, a.strides),
life: PhantomData,
window,
strides: window_strides,
}
Expand All @@ -54,6 +58,7 @@ impl_ndproducer! {
[Clone => 'a, A, D: Clone ]
Windows {
base,
life,
window,
strides,
}
Expand All @@ -77,7 +82,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,
}
Expand All @@ -89,7 +95,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,
}
Expand All @@ -99,19 +106,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);
4 changes: 0 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1554,10 +1554,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);
Expand Down

0 comments on commit 97c00b9

Please sign in to comment.