Skip to content

Fix potential undoundness with Storage::as_slice and StorageMut::as_mut_slice#905

Merged
sebcrozet merged 1 commit into
devfrom
unsound_as_slice
Jun 17, 2021
Merged

Fix potential undoundness with Storage::as_slice and StorageMut::as_mut_slice#905
sebcrozet merged 1 commit into
devfrom
unsound_as_slice

Conversation

@sebcrozet
Copy link
Copy Markdown
Member

@sebcrozet sebcrozet commented Jun 6, 2021

Fix #828

  • Add the safe methods: ContiguousStorage::as_slice and ContiguousStorage::as_mut_slice.
  • The trait methods: Storage::as_slice, StorageMut::as_mut_slice have been renamed to Storage::as_slice_unchecked and StorageMut::as_mut_slice_unchecked and are marked as unsafe.
  • The trait method Storage::is_contiguous is now marked as unsafe.

Not marking them as unsafe was invalid because they could result in invalid aliasing when calling as_mut_slice on two matrix slices obtained from the same matrix, but with overlapping (but non-contiguous) internal storage. For example, the following was allowed, but is actually unsound:

let (mut slice1, mut slice2) = matrix.rows_range_pair_mut(0, 1)
// Unsound because both mutable references point to the same element.
some_function(&mut slice1.data.as_mut_slice[1], &mut slice2.data.as_mut_slice[0]);

@sebcrozet sebcrozet changed the title Fix potential undoundness with Storage::as_slice and Storage::as_mut_slice Fix potential undoundness with Storage::as_slice and StorageMut::as_mut_slice Jun 6, 2021
@sebcrozet sebcrozet added breaking change Fixing this issue, or merging this pull-request is likely to require breaking changes bug P-high High priority labels Jun 6, 2021
@sebcrozet sebcrozet merged commit 38add0b into dev Jun 17, 2021
@sebcrozet sebcrozet deleted the unsound_as_slice branch June 17, 2021 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Fixing this issue, or merging this pull-request is likely to require breaking changes bug P-high High priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storage::as_slice and StorageMut::as_mut_slice may result in unsoundness in the future

1 participant