From 888cd2abc48c76f9233a51fcdd1e32a19b42b728 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Tue, 18 Oct 2022 15:58:59 +0200 Subject: [PATCH] vulkan: Mark `fn mapped_(mut_)slice()` as `unsafe` As discussed long ago, and recently in #138, it is undefined behaviour to create or transmute to `&[u8]` when the underlying data is possibly uninit. This also holds true for transmuting arbitrary `T: Copy` structures to `&[u8]` where eventual padding bytes are considered uninitialized, hence invalid for `u8`. Instead of coming up with a massive safety API that distinguishes between uninitialized and initialized buffers - which turn out to be really easy to invalidate by copying structures with padding bytes - place the onus on the user to keep track of initialization status by only ever providing mapped slices in an `unsafe` context. Users are expected to initialize the buffer using `ptr::copy(_nonoverlapping)()` when used from a CPU context instead of calling `.mapped_mut_slice()`, or switch to the new [presser] API from #138. [presser]: https://crates.io/crates/presser --- src/vulkan/mod.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/vulkan/mod.rs b/src/vulkan/mod.rs index 92e295ac..c309c9e8 100644 --- a/src/vulkan/mod.rs +++ b/src/vulkan/mod.rs @@ -92,18 +92,24 @@ impl Allocation { /// Returns a valid mapped slice if the memory is host visible, otherwise it will return None. /// The slice already references the exact memory region of the allocation, so no offset needs to be applied. - pub fn mapped_slice(&self) -> Option<&[u8]> { - self.mapped_ptr().map(|ptr| unsafe { - std::slice::from_raw_parts(ptr.cast().as_ptr(), self.size as usize) - }) + /// + /// # Safety + /// Only to be called when the memory is known to be _fully_ initialized. + pub unsafe fn mapped_slice(&self) -> Option<&[u8]> { + self.mapped_ptr() + .map(|ptr| std::slice::from_raw_parts(ptr.cast().as_ptr(), self.size as usize)) } /// Returns a valid mapped mutable slice if the memory is host visible, otherwise it will return None. /// The slice already references the exact memory region of the allocation, so no offset needs to be applied. - pub fn mapped_slice_mut(&mut self) -> Option<&mut [u8]> { - self.mapped_ptr().map(|ptr| unsafe { - std::slice::from_raw_parts_mut(ptr.cast().as_ptr(), self.size as usize) - }) + /// + /// # Safety + /// Only to be called when the memory is known to be _fully_ initialized. Use [`std::ptr::copy()`] or + /// [`std::ptr::copy_nonoverlapping()`] on [`mapped_ptr()`][Self::mapped_ptr()] to initialize this buffer + /// from the CPU instead. + pub unsafe fn mapped_mut_slice(&mut self) -> Option<&mut [u8]> { + self.mapped_ptr() + .map(|ptr| std::slice::from_raw_parts_mut(ptr.cast().as_ptr(), self.size as usize)) } pub fn is_null(&self) -> bool {