Add validation for binding dedicated allocations (#2056)

vulkano/src/buffer/sys.rs

@@ -21,7 +21,7 @@ use crate::{
     device::{Device, DeviceOwned},
     memory::{
         allocator::{AllocationCreationError, MemoryAlloc},
-        ExternalMemoryHandleType, ExternalMemoryHandleTypes, MemoryRequirements,
+        DedicatedTo, ExternalMemoryHandleType, ExternalMemoryHandleTypes, MemoryRequirements,
     },
     range_map::RangeMap,
     sync::{AccessError, CurrentAccess, Sharing},
@@ -400,12 +400,12 @@ impl RawBuffer {
         }
     }
 
+    pub(crate) fn id(&self) -> NonZeroU64 {
+        self.id
+    }
+
     /// Binds device memory to this buffer.
-    ///
+    pub fn bind_memory(
-    /// # Safety
-    ///
-    /// - If `allocation` is a dedicated allocation, it must be dedicated to `self` specifically.
-    pub unsafe fn bind_memory(
         self,
         allocation: MemoryAlloc,
     ) -> Result<Buffer, (BufferError, RawBuffer, MemoryAlloc)> {
@@ -413,7 +413,7 @@ impl RawBuffer {
             return Err((err, self, allocation));
         }
 
-        self.bind_memory_unchecked(allocation)
+        unsafe { self.bind_memory_unchecked(allocation) }
             .map_err(|(err, buffer, allocation)| (err.into(), buffer, allocation))
     }
 
@@ -464,9 +464,12 @@ impl RawBuffer {
             });
         }
 
-        if allocation.is_dedicated() {
+        if let Some(dedicated_to) = memory.dedicated_to() {
             // VUID-VkBindBufferMemoryInfo-memory-01508
-            // TODO: Check that the allocation is dedicated *to this buffer specifically*.
+            match dedicated_to {
+                DedicatedTo::Buffer(id) if id == self.id => {}
+                _ => return Err(BufferError::DedicatedAllocationMismatch),
+            }
             debug_assert!(memory_offset == 0); // This should be ensured by the allocator
         } else {
             // VUID-VkBindBufferMemoryInfo-buffer-01444
@@ -1164,7 +1167,7 @@ impl<'a> DerefMut for BufferWriteGuard<'a> {
     }
 }
 
-/// Error that can happen when creating a buffer.
+/// Error that can happen in buffer functions.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum BufferError {
     VulkanError(VulkanError),
@@ -1177,6 +1180,9 @@ pub enum BufferError {
         requires_one_of: RequiresOneOf,
     },
 
+    /// The memory was created dedicated to a resource, but not to this buffer.
+    DedicatedAllocationMismatch,
+
     /// A dedicated allocation is required for this buffer, but one was not provided.
     DedicatedAllocationRequired,
 
@@ -1271,6 +1277,10 @@ impl Display for BufferError {
                 "a requirement was not met for: {}; requires one of: {}",
                 required_for, requires_one_of,
             ),
+            Self::DedicatedAllocationMismatch => write!(
+                f,
+                "the memory was created dedicated to a resource, but not to this buffer",
+            ),
             Self::DedicatedAllocationRequired => write!(
                 f,
                 "a dedicated allocation is required for this buffer, but one was not provided"

vulkano/src/image/sys.rs

@@ -29,7 +29,7 @@ use crate::{
     },
     memory::{
         allocator::{AllocationCreationError, MemoryAlloc},
-        ExternalMemoryHandleType, ExternalMemoryHandleTypes, MemoryRequirements,
+        DedicatedTo, ExternalMemoryHandleType, ExternalMemoryHandleTypes, MemoryRequirements,
     },
     range_map::RangeMap,
     swapchain::Swapchain,
@@ -1301,19 +1301,18 @@ impl RawImage {
         }
     }
 
+    pub(crate) fn id(&self) -> NonZeroU64 {
+        self.id
+    }
+
     /// Binds device memory to this image.
     ///
     /// - If `self.flags().disjoint` is not set, then `allocations` must contain exactly one
     ///   element. This element may be a dedicated allocation.
-    /// - If `self.flags().disjoint` is set,  then `allocations` must contain exactly
+    /// - If `self.flags().disjoint` is set, then `allocations` must contain exactly
     ///   `self.format().unwrap().planes().len()` elements. These elements must not be dedicated
     ///   allocations.
-    ///
+    pub fn bind_memory(
-    /// # Safety
-    ///
-    /// - If the element of `allocations` is a dedicated allocation, it must be dedicated to `self`
-    ///   specifically.
-    pub unsafe fn bind_memory(
         self,
         allocations: impl IntoIterator<Item = MemoryAlloc>,
     ) -> Result<
@@ -1330,17 +1329,16 @@ impl RawImage {
             return Err((err, self, allocations.into_iter()));
         }
 
-        self.bind_memory_unchecked(allocations)
+        unsafe { self.bind_memory_unchecked(allocations) }.map_err(|(err, image, allocations)| {
-            .map_err(|(err, image, allocations)| {
+            (
-                (
+                err.into(),
-                    err.into(),
+                image,
-                    image,
+                allocations
-                    allocations
+                    .into_iter()
-                        .into_iter()
+                    .collect::<SmallVec<[_; 3]>>()
-                        .collect::<SmallVec<[_; 3]>>()
+                    .into_iter(),
-                        .into_iter(),
+            )
-                )
+        })
-            })
     }
 
     fn validate_bind_memory(&self, allocations: &[MemoryAlloc]) -> Result<(), ImageError> {
@@ -1379,15 +1377,18 @@ impl RawImage {
             // Ensured by taking ownership of `RawImage`.
 
             // VUID-VkBindImageMemoryInfo-image-01045
-            // Currently ensured by not having sparse binding flags, but this needs to be checked once
+            // Currently ensured by not having sparse binding flags, but this needs to be checked
-            // those are enabled.
+            // once those are enabled.
 
             // VUID-VkBindImageMemoryInfo-memoryOffset-01046
             // Assume that `allocation` was created correctly.
 
-            if allocation.is_dedicated() {
+            if let Some(dedicated_to) = memory.dedicated_to() {
                 // VUID-VkBindImageMemoryInfo-memory-02628
-                // TODO: Check that the allocation is dedicated *to this buffer specifically*.
+                match dedicated_to {
+                    DedicatedTo::Image(id) if id == self.id => {}
+                    _ => return Err(ImageError::DedicatedAllocationMismatch),
+                }
                 debug_assert!(memory_offset == 0); // This should be ensured by the allocator
             } else {
                 // VUID-VkBindImageMemoryInfo-image-01445
@@ -1469,7 +1470,7 @@ impl RawImage {
     ///
     /// - If `self.flags().disjoint` is not set, then `allocations` must contain exactly one
     ///   element.
-    /// - If `self.flags().disjoint` is set,  then `allocations` must contain exactly
+    /// - If `self.flags().disjoint` is set, then `allocations` must contain exactly
     ///   `self.format().unwrap().planes().len()` elements.
     #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))]
     pub unsafe fn bind_memory_unchecked(
@@ -2770,6 +2771,9 @@ pub enum ImageError {
     /// The `cube_compatible` flag was enabled together with multisampling.
     CubeCompatibleMultisampling,
 
+    /// The memory was created dedicated to a resource, but not to this image.
+    DedicatedAllocationMismatch,
+
     /// A dedicated allocation is required for this image, but one was not provided.
     DedicatedAllocationRequired,
 
@@ -2997,6 +3001,10 @@ impl Display for ImageError {
                 f,
                 "the `cube_compatible` flag was enabled together with multisampling",
             ),
+            Self::DedicatedAllocationMismatch => write!(
+                f,
+                "the memory was created dedicated to a resource, but not to this image",
+            ),
             Self::DedicatedAllocationRequired => write!(
                 f,
                 "a dedicated allocation is required for this image, but one was not provided"

vulkano/src/memory/allocator/mod.rs

@@ -1149,28 +1149,13 @@ unsafe impl<S: Suballocator> MemoryAllocator for GenericMemoryAllocator<S> {
                 };
                 match DeviceMemory::allocate_unchecked(self.device.clone(), allocate_info, None) {
                     Ok(device_memory) => {
-                        break S::new(MemoryAlloc::new_root(device_memory)?);
+                        break S::new(MemoryAlloc::new(device_memory)?);
                     }
                     // Retry up to 3 times, halving the allocation size each time.
                     Err(VulkanError::OutOfHostMemory | VulkanError::OutOfDeviceMemory) if i < 3 => {
                         i += 1;
                     }
-                    Err(VulkanError::OutOfHostMemory) => {
+                    Err(err) => return Err(err.into()),
-                        return Err(AllocationCreationError::VulkanError(
-                            VulkanError::OutOfHostMemory,
-                        ));
-                    }
-                    Err(VulkanError::OutOfDeviceMemory) => {
-                        return Err(AllocationCreationError::VulkanError(
-                            VulkanError::OutOfDeviceMemory,
-                        ));
-                    }
-                    Err(VulkanError::TooManyObjects) => {
-                        return Err(AllocationCreationError::VulkanError(
-                            VulkanError::TooManyObjects,
-                        ));
-                    }
-                    Err(_) => unreachable!(),
                 }
             }
         };
@@ -1381,7 +1366,6 @@ unsafe impl<S: Suballocator> MemoryAllocator for GenericMemoryAllocator<S> {
             dedicated_allocation = None;
         }
 
-        let is_dedicated = dedicated_allocation.is_some();
         let allocate_info = MemoryAllocateInfo {
             allocation_size,
             memory_type_index,
@@ -1390,26 +1374,13 @@ unsafe impl<S: Suballocator> MemoryAllocator for GenericMemoryAllocator<S> {
             flags: self.flags,
             ..Default::default()
         };
-        let device_memory =
+        let mut alloc = MemoryAlloc::new(
-            DeviceMemory::allocate_unchecked(self.device.clone(), allocate_info, None).map_err(
+            DeviceMemory::allocate_unchecked(self.device.clone(), allocate_info, None)
-                |err| match err {
+                .map_err(AllocationCreationError::from)?,
-                    VulkanError::OutOfHostMemory => {
+        )?;
-                        AllocationCreationError::VulkanError(VulkanError::OutOfHostMemory)
+        alloc.set_allocation_type(self.allocation_type);
-                    }
-                    VulkanError::OutOfDeviceMemory => {
-                        AllocationCreationError::VulkanError(VulkanError::OutOfDeviceMemory)
-                    }
-                    VulkanError::TooManyObjects => {
-                        AllocationCreationError::VulkanError(VulkanError::TooManyObjects)
-                    }
-                    _ => unreachable!(),
-                },
-            )?;
 
-        MemoryAlloc::new_inner(device_memory, is_dedicated).map(|mut alloc| {
+        Ok(alloc)
-            alloc.set_allocation_type(self.allocation_type);
-            alloc
-        })
     }
 }
 
@@ -1449,7 +1420,7 @@ pub struct GenericMemoryAllocatorCreateInfo<'b, 'e> {
     /// all suballocations that the pool allocator makes inherit their allocation type from the
     /// parent allocation. For the [`FreeListAllocator`] and the [`BuddyAllocator`] this must be
     /// [`AllocationType::Unknown`] otherwise you will get panics. It does not matter what this is
-    /// for when using the [`BumpAllocator`].
+    /// when using the [`BumpAllocator`].
     ///
     /// The default value is [`AllocationType::Unknown`].
     pub allocation_type: AllocationType,

vulkano/src/memory/allocator/suballocator.rs

@@ -43,10 +43,11 @@ use std::{
 ///
 /// There's a few ways you can obtain a `MemoryAlloc` in Vulkano. Most commonly you will probably
 /// want to use a [memory allocator]. If you already have a [`DeviceMemory`] block on hand that you
-/// would like to turn into an allocation, you can use one of the constructors. Lastly, you can use
+/// would like to turn into an allocation, you can use [the constructor]. Lastly, you can use a
-/// a [suballocator] if you want to create multiple smaller allocations out of a bigger one.
+/// [suballocator] if you want to create multiple smaller allocations out of a bigger one.
 ///
 /// [memory allocator]: super::MemoryAllocator
+/// [the constructor]: Self::new
 /// [suballocator]: Suballocator
 #[derive(Debug)]
 pub struct MemoryAlloc {
@@ -89,26 +90,11 @@ unsafe impl Send for MemoryAlloc {}
 unsafe impl Sync for MemoryAlloc {}
 
 impl MemoryAlloc {
-    /// Creates a new root allocation.
+    /// Creates a new `MemoryAlloc`.
     ///
     /// The memory is mapped automatically if it's host-visible.
     #[inline]
-    pub fn new_root(device_memory: DeviceMemory) -> Result<Self, AllocationCreationError> {
+    pub fn new(device_memory: DeviceMemory) -> Result<Self, AllocationCreationError> {
-        Self::new_inner(device_memory, false)
-    }
-
-    /// Creates a new dedicated allocation.
-    ///
-    /// The memory is mapped automatically if it's host-visible.
-    #[inline]
-    pub fn new_dedicated(device_memory: DeviceMemory) -> Result<Self, AllocationCreationError> {
-        Self::new_inner(device_memory, true)
-    }
-
-    pub(super) fn new_inner(
-        device_memory: DeviceMemory,
-        dedicated: bool,
-    ) -> Result<Self, AllocationCreationError> {
         let device = device_memory.device();
         let physical_device = device.physical_device();
         let memory_type_index = device_memory.memory_type_index();
@@ -159,7 +145,7 @@ impl MemoryAlloc {
             allocation_type: AllocationType::Unknown,
             mapped_ptr,
             atom_size,
-            parent: if dedicated {
+            parent: if device_memory.is_dedicated() {
                 AllocParent::Dedicated(device_memory)
             } else {
                 AllocParent::Root(Arc::new(device_memory))
@@ -649,7 +635,7 @@ unsafe impl DeviceOwned for MemoryAlloc {
 ///     })
 ///     .unwrap() as u32;
 ///
-/// let region = MemoryAlloc::new_root(
+/// let region = MemoryAlloc::new(
 ///     DeviceMemory::allocate(
 ///         device.clone(),
 ///         MemoryAllocateInfo {
@@ -659,7 +645,8 @@ unsafe impl DeviceOwned for MemoryAlloc {
 ///         },
 ///     )
 ///     .unwrap(),
-/// );
+/// )
+/// .unwrap();
 ///
 /// // You can now feed `region` into any suballocator.
 /// ```
@@ -712,7 +699,6 @@ pub unsafe trait Suballocator: DeviceOwned {
     /// - `create_info.alignment` must be a power of two.
     ///
     /// [region]: Self#regions
-    /// [`allocate`]: Self::allocate
     #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))]
     unsafe fn allocate_unchecked(
         &self,
@@ -2746,7 +2732,7 @@ mod tests {
             )
             .unwrap();
 
-            PoolAllocator::new(MemoryAlloc::new_root(device_memory).unwrap(), 1)
+            PoolAllocator::new(MemoryAlloc::new(device_memory).unwrap(), 1)
         }
 
         let (device, _) = gfx_dev_and_queue!();
@@ -2804,7 +2790,7 @@ mod tests {
             )
             .unwrap();
 
-            PoolAllocator::<BLOCK_SIZE>::new(MemoryAlloc::new_root(device_memory).unwrap(), 1)
+            PoolAllocator::<BLOCK_SIZE>::new(MemoryAlloc::new(device_memory).unwrap(), 1)
         };
 
         // This uses the fact that block indices are inserted into the free-list in order, so
@@ -2837,7 +2823,7 @@ mod tests {
                 },
             )
             .unwrap();
-            let mut region = MemoryAlloc::new_root(device_memory).unwrap();
+            let mut region = MemoryAlloc::new(device_memory).unwrap();
             unsafe { region.set_allocation_type(allocation_type) };
 
             PoolAllocator::new(region, 256)
@@ -3096,7 +3082,7 @@ mod tests {
                 },
             )
             .unwrap();
-            let mut allocator = <$type>::new(MemoryAlloc::new_root(device_memory).unwrap());
+            let mut allocator = <$type>::new(MemoryAlloc::new(device_memory).unwrap());
             Arc::get_mut(&mut allocator)
                 .unwrap()
                 .buffer_image_granularity = $granularity;

vulkano/src/memory/device_memory.rs

@@ -7,7 +7,7 @@
 // notice may not be copied, modified, or distributed except
 // according to those terms.
 
-use super::DedicatedAllocation;
+use super::{DedicatedAllocation, DedicatedTo};
 use crate::{
     device::{Device, DeviceOwned},
     macros::{vulkan_bitflags, vulkan_enum},
@@ -56,6 +56,7 @@ pub struct DeviceMemory {
 
     allocation_size: DeviceSize,
     memory_type_index: u32,
+    dedicated_to: Option<DedicatedTo>,
     export_handle_types: ExternalMemoryHandleTypes,
     imported_handle_type: Option<ExternalMemoryHandleType>,
     flags: MemoryAllocateFlags,
@@ -97,7 +98,7 @@ impl DeviceMemory {
         let MemoryAllocateInfo {
             allocation_size,
             memory_type_index,
-            dedicated_allocation: _,
+            dedicated_allocation,
             export_handle_types,
             flags,
             _ne: _,
@@ -109,6 +110,7 @@ impl DeviceMemory {
             id: Self::next_id(),
             allocation_size,
             memory_type_index,
+            dedicated_to: dedicated_allocation.map(Into::into),
             export_handle_types,
             imported_handle_type: None,
             flags,
@@ -552,6 +554,7 @@ impl DeviceMemory {
             id: Self::next_id(),
             allocation_size,
             memory_type_index,
+            dedicated_to: dedicated_allocation.map(Into::into),
             export_handle_types,
             imported_handle_type,
             flags,
@@ -570,6 +573,18 @@ impl DeviceMemory {
         self.allocation_size
     }
 
+    /// Returns `true` if the memory is a [dedicated] to a resource.
+    ///
+    /// [dedicated]: MemoryAllocateInfo#structfield.dedicated_allocation
+    #[inline]
+    pub fn is_dedicated(&self) -> bool {
+        self.dedicated_to.is_some()
+    }
+
+    pub(crate) fn dedicated_to(&self) -> Option<DedicatedTo> {
+        self.dedicated_to
+    }
+
     /// Returns the handle types that can be exported from the memory allocation.
     #[inline]
     pub fn export_handle_types(&self) -> ExternalMemoryHandleTypes {

vulkano/src/memory/mod.rs

@@ -103,7 +103,7 @@ use crate::{
     sync::Semaphore,
     DeviceSize,
 };
-use std::sync::Arc;
+use std::{num::NonZeroU64, sync::Arc};
 
 pub mod allocator;
 mod device_memory;
@@ -323,6 +323,21 @@ pub enum DedicatedAllocation<'a> {
     Image(&'a RawImage),
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum DedicatedTo {
+    Buffer(NonZeroU64),
+    Image(NonZeroU64),
+}
+
+impl From<DedicatedAllocation<'_>> for DedicatedTo {
+    fn from(dedicated_allocation: DedicatedAllocation<'_>) -> Self {
+        match dedicated_allocation {
+            DedicatedAllocation::Buffer(buffer) => Self::Buffer(buffer.id()),
+            DedicatedAllocation::Image(image) => Self::Image(image.id()),
+        }
+    }
+}
+
 /// The properties for exporting or importing external memory, when a buffer or image is created
 /// with a specific configuration.
 #[derive(Clone, Debug, Default)]