Skip to content

Commit

Permalink
Fix undefined behavior reported by miri when reading or writing slices (
Browse files Browse the repository at this point in the history
#351)

Many methods in the slice or array api take a pointer to the first
element and then use that pointer to read or write multiple lanes.

This is undefined behavior according to miri since pointer is only valid
for a single element. The fix is to use the `as_ptr`/`as_mut_ptr`
methods of the slice, which gives a pointer to the whole slice and also
leads to nicer code.
  • Loading branch information
jhorstmann authored Jul 28, 2023
1 parent 76932d4 commit 4d4d27a
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 16 deletions.
1 change: 1 addition & 0 deletions src/api/from/from_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ macro_rules! impl_from_array {
mod [<$id _from>] {
use super::*;
#[test]
#[cfg_attr(miri, ignore)]
fn array() {
let vec: $id = Default::default();

Expand Down
16 changes: 6 additions & 10 deletions src/api/minimal/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ macro_rules! impl_minimal_p {
pub fn from_slice_aligned(slice: &[$elem_ty]) -> Self {
unsafe {
assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked(0) as *const $elem_ty;
let target_ptr = slice.as_ptr();
assert!(
target_ptr.align_offset(crate::mem::align_of::<Self>())
== 0
Expand Down Expand Up @@ -615,7 +615,7 @@ macro_rules! impl_minimal_p {
pub unsafe fn from_slice_aligned_unchecked(slice: &[$elem_ty])
-> Self {
#[allow(clippy::cast_ptr_alignment)]
*(slice.get_unchecked(0) as *const $elem_ty as *const Self)
*(slice.as_ptr().cast())
}

/// Instantiates a new vector with the values of the `slice`.
Expand All @@ -628,8 +628,7 @@ macro_rules! impl_minimal_p {
slice: &[$elem_ty],
) -> Self {
use crate::mem::size_of;
let target_ptr =
slice.get_unchecked(0) as *const $elem_ty as *const u8;
let target_ptr = slice.as_ptr().cast();
let mut x = Self::splat(crate::ptr::null_mut() as $elem_ty);
let self_ptr = &mut x as *mut Self as *mut u8;
crate::ptr::copy_nonoverlapping(
Expand Down Expand Up @@ -798,8 +797,7 @@ macro_rules! impl_minimal_p {
pub fn write_to_slice_aligned(self, slice: &mut [$elem_ty]) {
unsafe {
assert!(slice.len() >= $elem_count);
let target_ptr =
slice.get_unchecked_mut(0) as *mut $elem_ty;
let target_ptr = slice.as_mut_ptr();
assert!(
target_ptr.align_offset(crate::mem::align_of::<Self>())
== 0
Expand Down Expand Up @@ -833,8 +831,7 @@ macro_rules! impl_minimal_p {
self, slice: &mut [$elem_ty],
) {
#[allow(clippy::cast_ptr_alignment)]
*(slice.get_unchecked_mut(0) as *mut $elem_ty as *mut Self) =
self;
*(slice.as_mut_ptr().cast()) = self;
}

/// Writes the values of the vector to the `slice`.
Expand All @@ -846,8 +843,7 @@ macro_rules! impl_minimal_p {
pub unsafe fn write_to_slice_unaligned_unchecked(
self, slice: &mut [$elem_ty],
) {
let target_ptr =
slice.get_unchecked_mut(0) as *mut $elem_ty as *mut u8;
let target_ptr = slice.as_mut_ptr().cast();
let self_ptr = &self as *const Self as *const u8;
crate::ptr::copy_nonoverlapping(
self_ptr,
Expand Down
6 changes: 3 additions & 3 deletions src/api/slice/from_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ macro_rules! impl_slice_from_slice {
pub fn from_slice_aligned(slice: &[$elem_ty]) -> Self {
unsafe {
assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked(0) as *const $elem_ty;
let target_ptr = slice.as_ptr();
assert_eq!(target_ptr.align_offset(crate::mem::align_of::<Self>()), 0);
Self::from_slice_aligned_unchecked(slice)
}
Expand Down Expand Up @@ -41,7 +41,7 @@ macro_rules! impl_slice_from_slice {
#[inline]
pub unsafe fn from_slice_aligned_unchecked(slice: &[$elem_ty]) -> Self {
debug_assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked(0) as *const $elem_ty;
let target_ptr = slice.as_ptr();
debug_assert_eq!(target_ptr.align_offset(crate::mem::align_of::<Self>()), 0);

#[allow(clippy::cast_ptr_alignment)]
Expand All @@ -57,7 +57,7 @@ macro_rules! impl_slice_from_slice {
pub unsafe fn from_slice_unaligned_unchecked(slice: &[$elem_ty]) -> Self {
use crate::mem::size_of;
debug_assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked(0) as *const $elem_ty as *const u8;
let target_ptr = slice.as_ptr().cast();
let mut x = Self::splat(0 as $elem_ty);
let self_ptr = &mut x as *mut Self as *mut u8;
crate::ptr::copy_nonoverlapping(target_ptr, self_ptr, size_of::<Self>());
Expand Down
6 changes: 3 additions & 3 deletions src/api/slice/write_to_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ macro_rules! impl_slice_write_to_slice {
pub fn write_to_slice_aligned(self, slice: &mut [$elem_ty]) {
unsafe {
assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked_mut(0) as *mut $elem_ty;
let target_ptr = slice.as_mut_ptr();
assert_eq!(target_ptr.align_offset(crate::mem::align_of::<Self>()), 0);
self.write_to_slice_aligned_unchecked(slice);
}
Expand Down Expand Up @@ -42,7 +42,7 @@ macro_rules! impl_slice_write_to_slice {
#[inline]
pub unsafe fn write_to_slice_aligned_unchecked(self, slice: &mut [$elem_ty]) {
debug_assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked_mut(0) as *mut $elem_ty;
let target_ptr = slice.as_mut_ptr();
debug_assert_eq!(target_ptr.align_offset(crate::mem::align_of::<Self>()), 0);

#[allow(clippy::cast_ptr_alignment)]
Expand All @@ -60,7 +60,7 @@ macro_rules! impl_slice_write_to_slice {
#[inline]
pub unsafe fn write_to_slice_unaligned_unchecked(self, slice: &mut [$elem_ty]) {
debug_assert!(slice.len() >= $elem_count);
let target_ptr = slice.get_unchecked_mut(0) as *mut $elem_ty as *mut u8;
let target_ptr = slice.as_mut_ptr().cast();
let self_ptr = &self as *const Self as *const u8;
crate::ptr::copy_nonoverlapping(self_ptr, target_ptr, crate::mem::size_of::<Self>());
}
Expand Down

0 comments on commit 4d4d27a

Please sign in to comment.