layers: Clean up no memory bound checks

When memory is freed make sure to clear bindings in associated objects.

Also clean up reporting of no memory bound errors. Old assumption was that
no memory had every been bound, but it's also possible to hit such errors
if the bound memory was freed prior to operation of interest.
In descriptor error cases where no memory is bound, add a final error string
as clarification since descriptor errors are built up of multiple strings
with details from each function call in the tree.

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index df0e180..88d816c 100644 (file)
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -633,8 +633,8 @@ static void clear_cmd_buf_and_mem_references(layer_data *dev_data, const VkComma
     clear_cmd_buf_and_mem_references(dev_data, getCBNode(dev_data, cb));
 }
 
-// For given MemObjInfo, report Obj & CB bindings
-static bool reportMemReferencesAndCleanUp(layer_data *dev_data, DEVICE_MEM_INFO *pMemObjInfo) {
+// For given MemObjInfo, report Obj & CB bindings. Clear any object bindings.
+static bool ReportMemReferencesAndCleanUp(layer_data *dev_data, DEVICE_MEM_INFO *pMemObjInfo) {
     bool skip_call = false;
     size_t cmdBufRefCount = pMemObjInfo->command_buffer_bindings.size();
     size_t objRefCount = pMemObjInfo->obj_bindings.size();
@@ -662,6 +662,24 @@ static bool reportMemReferencesAndCleanUp(layer_data *dev_data, DEVICE_MEM_INFO
             log_msg(dev_data->report_data, VK_DEBUG_REPORT_INFORMATION_BIT_EXT, obj.type, obj.handle, __LINE__,
                     MEMTRACK_FREED_MEM_REF, "MEM", "VK Object 0x%" PRIxLEAST64 " still has a reference to mem obj 0x%" PRIxLEAST64,
                     obj.handle, (uint64_t)pMemObjInfo->mem);
+            // Clear mem binding for bound objects
+            switch (obj.type) {
+            case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT: {
+                auto image_node = getImageNode(dev_data, reinterpret_cast<VkImage &>(obj.handle));
+                assert(image_node); // Any destroyed images should already be removed from bindings
+                image_node->mem = VK_NULL_HANDLE;
+                break;
+            }
+            case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT: {
+                auto buff_node = getBufferNode(dev_data, reinterpret_cast<VkBuffer &>(obj.handle));
+                assert(buff_node); // Any destroyed buffers should already be removed from bindings
+                buff_node->mem = VK_NULL_HANDLE;
+                break;
+            }
+            default:
+                // Should only have buffer or image objects bound to memory
+                assert(0);
+            }
         }
         // Clear the list of hanging references
         pMemObjInfo->obj_bindings.clear();
@@ -688,10 +706,9 @@ static bool freeMemObjInfo(layer_data *dev_data, void *object, VkDeviceMemory me
                 clear_cmd_buf_and_mem_references(dev_data, cb);
             }
         }
-
-        // Now verify that no references to this mem obj remain and remove bindings
+        // Now check for any remaining references to this mem obj and remove bindings
         if (pInfo->command_buffer_bindings.size() || pInfo->obj_bindings.size()) {
-            skip_call |= reportMemReferencesAndCleanUp(dev_data, pInfo);
+            skip_call |= ReportMemReferencesAndCleanUp(dev_data, pInfo);
         }
         // Delete mem obj info
         dev_data->memObjMap.erase(dev_data->memObjMap.find(mem));
@@ -742,8 +759,9 @@ bool ValidateMemoryIsBoundToImage(const layer_data *dev_data, const IMAGE_NODE *
         if (0 == image_node->mem) {
             result = log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT,
                              reinterpret_cast<const uint64_t &>(image_node->image), __LINE__, MEMTRACK_OBJECT_NOT_BOUND, "MEM",
-                             "%s: VkImage object 0x%" PRIxLEAST64 " used without first calling vkBindImageMemory.", api_name,
-                             reinterpret_cast<const uint64_t &>(image_node->image));
+                             "%s: VkImage object 0x%" PRIxLEAST64 " used with no memory bound. Memory should be bound by calling "
+                             "vkBindImageMemory() and then the bound memory must not be freed prior to this operation.",
+                             api_name, reinterpret_cast<const uint64_t &>(image_node->image));
         }
     }
     return result;
@@ -756,8 +774,9 @@ bool ValidateMemoryIsBoundToBuffer(const layer_data *dev_data, const BUFFER_NODE
         if (0 == buffer_node->mem) {
             result = log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT,
                              reinterpret_cast<const uint64_t &>(buffer_node->buffer), __LINE__, MEMTRACK_OBJECT_NOT_BOUND, "MEM",
-                             "%s: VkBuffer object 0x%" PRIxLEAST64 " used without first calling vkBindBufferMemory.", api_name,
-                             reinterpret_cast<const uint64_t &>(buffer_node->buffer));
+                             "%s: VkBuffer object 0x%" PRIxLEAST64 " used with no memory bound. Memory should be bound by calling "
+                             "vkBindImageMemory() and then the bound memory must not be freed prior to this operation.",
+                             api_name, reinterpret_cast<const uint64_t &>(buffer_node->buffer));
         }
     }
     return result;
@@ -5627,10 +5646,11 @@ static void RemoveMemoryRange(uint64_t handle, DEVICE_MEM_INFO *mem_info, bool i
     }
     erase_range->aliases.clear();
     mem_info->bound_ranges.erase(handle);
-    if (is_image)
+    if (is_image) {
         mem_info->bound_images.erase(handle);
-    else
+    } else {
         mem_info->bound_buffers.erase(handle);
+    }
 }
 
 static void RemoveBufferMemoryRange(uint64_t handle, DEVICE_MEM_INFO *mem_info) { RemoveMemoryRange(handle, mem_info, false); }

diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp
index e017f0e..d7ca025 100644 (file)
--- a/layers/descriptor_sets.cpp
+++ b/layers/descriptor_sets.cpp
@@ -690,8 +690,10 @@ bool cvdescriptorset::ValidateImageUpdate(VkImageView image_view, VkImageLayout
         format = image_node->createInfo.format;
         usage = image_node->createInfo.usage;
         // Validate that memory is bound to image
-        if (ValidateMemoryIsBoundToImage(dev_data, image_node, "vkUpdateDescriptorSets()"))
+        if (ValidateMemoryIsBoundToImage(dev_data, image_node, "vkUpdateDescriptorSets()")) {
+            *error = "No memory bound to image.";
             return false;
+        }
     } else {
         // Also need to check the swapchains.
         auto swapchain = getSwapchainFromImage(dev_data, image);
@@ -1184,8 +1186,10 @@ bool cvdescriptorset::DescriptorSet::ValidateBufferUpdate(VkDescriptorBufferInfo
         *error = error_str.str();
         return false;
     }
-    if (ValidateMemoryIsBoundToBuffer(device_data_, buffer_node, "vkUpdateDescriptorSets()"))
+    if (ValidateMemoryIsBoundToBuffer(device_data_, buffer_node, "vkUpdateDescriptorSets()")) {
+        *error = "No memory bound to buffer.";
         return false;
+    }
     // Verify usage bits
     if (!ValidateBufferUsage(buffer_node, type, error)) {
         // error will have been updated by ValidateBufferUsage()

diff --git a/tests/layer_validation_tests.cpp b/tests/layer_validation_tests.cpp
index 0ea2e69..0445573 100644 (file)
--- a/tests/layer_validation_tests.cpp
+++ b/tests/layer_validation_tests.cpp
@@ -6993,9 +6993,9 @@ TEST_F(VkLayerTest, ImageMemoryNotBound) {
     err = vkAllocateMemory(m_device->device(), &mem_alloc, NULL, &image_mem);
     ASSERT_VK_SUCCESS(err);
 
-    // Introduce error, do not call vkBindImageMemory(m_device->device(), image,
-    // image_mem, 0);
-    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, "used without first calling vkBindImageMemory");
+    // Introduce error, do not call vkBindImageMemory(m_device->device(), image, image_mem, 0);
+    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                         " used with no memory bound. Memory should be bound by calling vkBindImageMemory() and ");
 
     m_commandBuffer->BeginCommandBuffer();
     VkClearColorValue ccv;
@@ -7052,9 +7052,9 @@ TEST_F(VkLayerTest, BufferMemoryNotBound) {
     err = vkAllocateMemory(m_device->device(), &alloc_info, NULL, &mem);
     ASSERT_VK_SUCCESS(err);
 
-    // Introduce failure by not calling vkBindBufferMemory(m_device->device(),
-    // buffer, mem, 0);
-    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, "used without first calling vkBindBufferMemory");
+    // Introduce failure by not calling vkBindBufferMemory(m_device->device(), buffer, mem, 0);
+    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                         " used with no memory bound. Memory should be bound by calling vkBindImageMemory() and ");
     VkBufferImageCopy region = {};
     region.bufferRowLength = 128;
     region.bufferImageHeight = 128;
@@ -7824,7 +7824,8 @@ TEST_F(VkLayerTest, CreateBufferViewNoMemoryBoundToBuffer) {
                      " no memory bound to it.");
 
     VkResult err;
-    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, "used without first calling vkBindBufferMemory");
+    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                         " used with no memory bound. Memory should be bound by calling vkBindImageMemory() and ");
 
     ASSERT_NO_FATAL_FAILURE(InitState());
 
@@ -8029,7 +8030,10 @@ TEST_F(VkLayerTest, DescriptorBufferUpdateNoMemoryBound) {
     TEST_DESCRIPTION("Attempt to update a descriptor with a non-sparse buffer "
                      "that doesn't have memory bound");
     VkResult err;
-    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, " used without first calling vkBindBufferMemory.");
+    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                         " used with no memory bound. Memory should be bound by calling vkBindImageMemory() and ");
+    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                         "vkUpdateDescriptorsSets() failed write update validation for Descriptor Set 0x");
 
     ASSERT_NO_FATAL_FAILURE(InitState());
     ASSERT_NO_FATAL_FAILURE(InitViewport());
@@ -15826,8 +15830,8 @@ TEST_F(VkLayerTest, InvalidImageView) {
 
 TEST_F(VkLayerTest, CreateImageViewNoMemoryBoundToImage) {
     VkResult err;
-
-    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, "used without first calling vkBindImageMemory");
+    m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                         " used with no memory bound. Memory should be bound by calling vkBindImageMemory() and ");
 
     ASSERT_NO_FATAL_FAILURE(InitState());