From d20128ab3f1e98295d32626124f094320835c004 Mon Sep 17 00:00:00 2001 From: Michael Lentine Date: Tue, 9 Feb 2016 20:59:50 -0600 Subject: [PATCH] layers: Validate semaphore is not in use when deleted. --- layers/draw_state.cpp | 61 +++++++++++++++++++++++++------- layers/draw_state.h | 65 +++++++++++++++++++++++++---------- layers/vk_validation_layer_details.md | 1 + 3 files changed, 96 insertions(+), 31 deletions(-) diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp index 2577dca..eafc763 100644 --- a/layers/draw_state.cpp +++ b/layers/draw_state.cpp @@ -119,7 +119,7 @@ struct layer_data { unordered_map eventMap; unordered_map queryToStateMap; unordered_map queryPoolMap; - unordered_map semaphoreSignaledMap; + unordered_map semaphoreMap; unordered_map commandBufferMap; unordered_map frameBufferMap; unordered_map imageLayoutMap; @@ -2767,6 +2767,7 @@ static void resetCB(layer_data* my_data, const VkCommandBuffer cb) pCB->updatedSets.clear(); pCB->boundDescriptorSets.clear(); pCB->waitedEvents.clear(); + pCB->semaphores.clear(); pCB->waitedEventsBeforeQueryReset.clear(); pCB->queryToStateMap.clear(); pCB->activeQueries.clear(); @@ -3307,6 +3308,20 @@ VkBool32 validateAndIncrementResources(layer_data* my_data, GLOBAL_CB_NODE* pCB) setNode->second->in_use.fetch_add(1); } } + for (auto semaphore : pCB->semaphores) { + auto semaphoreNode = my_data->semaphoreMap.find(semaphore); + if (semaphoreNode == my_data->semaphoreMap.end()) { + skip_call |= log_msg( + my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, + VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, + reinterpret_cast(semaphore), __LINE__, + DRAWSTATE_INVALID_SEMAPHORE, "DS", + "Cannot submit cmd buffer using deleted semaphore %" PRIu64 ".", + reinterpret_cast(semaphore)); + } else { + semaphoreNode->second.in_use.fetch_add(1); + } + } return skip_call; } @@ -3326,6 +3341,12 @@ void decrementResources(layer_data* my_data, VkCommandBuffer cmdBuffer) { setNode->second->in_use.fetch_sub(1); } } + for (auto semaphore : pCB->semaphores) { + auto semaphoreNode = my_data->semaphoreMap.find(semaphore); + if (semaphoreNode != my_data->semaphoreMap.end()) { + semaphoreNode->second.in_use.fetch_sub(1); + } + } for (auto queryStatePair : pCB->queryToStateMap) { my_data->queryToStateMap[queryStatePair.first] = queryStatePair.second; } @@ -3512,9 +3533,11 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueSubmit(VkQueue queue, uint loader_platform_thread_lock_mutex(&globalLock); for (uint32_t submit_idx = 0; submit_idx < submitCount; submit_idx++) { const VkSubmitInfo *submit = &pSubmits[submit_idx]; + vector semaphoreList; for (uint32_t i=0; i < submit->waitSemaphoreCount; ++i) { - if (dev_data->semaphoreSignaledMap[submit->pWaitSemaphores[i]]) { - dev_data->semaphoreSignaledMap[submit->pWaitSemaphores[i]] = 0; + semaphoreList.push_back(submit->pWaitSemaphores[i]); + if (dev_data->semaphoreMap[submit->pWaitSemaphores[i]].signaled) { + dev_data->semaphoreMap[submit->pWaitSemaphores[i]].signaled = 0; } else { skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, 0, __LINE__, DRAWSTATE_QUEUE_FORWARD_PROGRESS, "DS", "Queue %#" PRIx64 " is waiting on semaphore %#" PRIx64 " that has no way to be signaled.", @@ -3522,7 +3545,8 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueSubmit(VkQueue queue, uint } } for (uint32_t i=0; i < submit->signalSemaphoreCount; ++i) { - dev_data->semaphoreSignaledMap[submit->pSignalSemaphores[i]] = 1; + semaphoreList.push_back(submit->pSignalSemaphores[i]); + dev_data->semaphoreMap[submit->pSignalSemaphores[i]].signaled = 1; } for (uint32_t i=0; i < submit->commandBufferCount; i++) { #ifndef DISABLE_IMAGE_LAYOUT_VALIDATION @@ -3530,6 +3554,7 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueSubmit(VkQueue queue, uint #endif // DISABLE_IMAGE_LAYOUT_VALIDATION pCB = getCBNode(dev_data, submit->pCommandBuffers[i]); + pCB->semaphores = semaphoreList; pCB->submitCount++; // increment submit count skipCall |= validatePrimaryCommandBufferState(dev_data, pCB); } @@ -3716,7 +3741,15 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkDestroySemaphore(VkDevice device, V layer_data* dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); dev_data->device_dispatch_table->DestroySemaphore(device, semaphore, pAllocator); loader_platform_thread_lock_mutex(&globalLock); - dev_data->semaphoreSignaledMap.erase(semaphore); + if (dev_data->semaphoreMap[semaphore].in_use.load()) { + log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, + VK_DEBUG_REPORT_OBJECT_TYPE_SEMAPHORE_EXT, + reinterpret_cast(semaphore), __LINE__, + DRAWSTATE_INVALID_SEMAPHORE, "DS", + "Cannot delete semaphore %" PRIx64 " which is in use.", + reinterpret_cast(semaphore)); + } + dev_data->semaphoreMap.erase(semaphore); loader_platform_thread_unlock_mutex(&globalLock); // TODO : Clean up any internal data structures using this obj. } @@ -6845,8 +6878,9 @@ VKAPI_ATTR VkResult VKAPI_CALL vkQueueBindSparse( for (uint32_t bindIdx=0; bindIdx < bindInfoCount; ++bindIdx) { const VkBindSparseInfo& bindInfo = pBindInfo[bindIdx]; for (uint32_t i=0; i < bindInfo.waitSemaphoreCount; ++i) { - if (dev_data->semaphoreSignaledMap[bindInfo.pWaitSemaphores[i]]) { - dev_data->semaphoreSignaledMap[bindInfo.pWaitSemaphores[i]] = 0; + if (dev_data->semaphoreMap[bindInfo.pWaitSemaphores[i]].signaled) { + dev_data->semaphoreMap[bindInfo.pWaitSemaphores[i]].signaled = + 0; } else { skip_call |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, 0, __LINE__, DRAWSTATE_QUEUE_FORWARD_PROGRESS, "DS", "Queue %#" PRIx64 " is waiting on semaphore %#" PRIx64 " that has no way to be signaled.", @@ -6854,7 +6888,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkQueueBindSparse( } } for (uint32_t i=0; i < bindInfo.signalSemaphoreCount; ++i) { - dev_data->semaphoreSignaledMap[bindInfo.pSignalSemaphores[i]] = 1; + dev_data->semaphoreMap[bindInfo.pSignalSemaphores[i]].signaled = 1; } } loader_platform_thread_unlock_mutex(&globalLock); @@ -6875,7 +6909,8 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateSemaphore( VkResult result = dev_data->device_dispatch_table->CreateSemaphore(device, pCreateInfo, pAllocator, pSemaphore); if (result == VK_SUCCESS) { loader_platform_thread_lock_mutex(&globalLock); - dev_data->semaphoreSignaledMap[*pSemaphore] = 0; + dev_data->semaphoreMap[*pSemaphore].signaled = 0; + dev_data->semaphoreMap[*pSemaphore].in_use.store(0); loader_platform_thread_unlock_mutex(&globalLock); } return result; @@ -6959,8 +6994,10 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueuePresentKHR(VkQueue queue, if (pPresentInfo) { loader_platform_thread_lock_mutex(&globalLock); for (uint32_t i=0; i < pPresentInfo->waitSemaphoreCount; ++i) { - if (dev_data->semaphoreSignaledMap[pPresentInfo->pWaitSemaphores[i]]) { - dev_data->semaphoreSignaledMap[pPresentInfo->pWaitSemaphores[i]] = 0; + if (dev_data->semaphoreMap[pPresentInfo->pWaitSemaphores[i]] + .signaled) { + dev_data->semaphoreMap[pPresentInfo->pWaitSemaphores[i]] + .signaled = 0; } else { skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, 0, __LINE__, DRAWSTATE_QUEUE_FORWARD_PROGRESS, "DS", "Queue %#" PRIx64 " is waiting on semaphore %#" PRIx64 " that has no way to be signaled.", @@ -7001,7 +7038,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkAcquireNextImageKHR( VkResult result = dev_data->device_dispatch_table->AcquireNextImageKHR(device, swapchain, timeout, semaphore, fence, pImageIndex); loader_platform_thread_lock_mutex(&globalLock); // FIXME/TODO: Need to add some thing code the "fence" parameter - dev_data->semaphoreSignaledMap[semaphore] = 1; + dev_data->semaphoreMap[semaphore].signaled = 1; loader_platform_thread_unlock_mutex(&globalLock); return result; } diff --git a/layers/draw_state.h b/layers/draw_state.h index 71edd68..c585f96 100755 --- a/layers/draw_state.h +++ b/layers/draw_state.h @@ -54,6 +54,7 @@ typedef enum _DRAW_STATE_ERROR { DRAWSTATE_INVALID_BUFFER, // Invalid Buffer DRAWSTATE_INVALID_QUERY, // Invalid Query DRAWSTATE_INVALID_FENCE, // Invalid Fence + DRAWSTATE_INVALID_SEMAPHORE, // Invalid Semaphore DRAWSTATE_VTX_INDEX_OUT_OF_BOUNDS, // binding in vkCmdBindVertexData() too // large for PSO's // pVertexBindingDescriptions array @@ -90,9 +91,9 @@ typedef enum _DRAW_STATE_ERROR { // never had vkBeginCommandBuffer() // called on it DRAWSTATE_COMMAND_BUFFER_SINGLE_SUBMIT_VIOLATION, // Cmd Buffer created with - // VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT - // flag is submitted - // multiple times + // VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT + // flag is submitted + // multiple times DRAWSTATE_INVALID_SECONDARY_COMMAND_BUFFER, // vkCmdExecuteCommands() called // with a primary commandBuffer // in pCommandBuffers array @@ -146,14 +147,14 @@ typedef enum _DRAW_STATE_ERROR { // RECORDING state DRAWSTATE_INVALID_CB_SIMULTANEOUS_USE, // CmdBuffer is being used in // violation of - // VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT - // rules (i.e. simultaneous use w/o - // that bit set) + // VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT + // rules (i.e. simultaneous use w/o + // that bit set) DRAWSTATE_INVALID_COMMAND_BUFFER_RESET, // Attempting to call Reset (or // Begin on recorded cmdBuffer) that // was allocated from Pool w/o - // VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT - // bit set + // VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT + // bit set DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, // Count for viewports and scissors // mismatch and/or state doesn't match // count @@ -162,17 +163,36 @@ typedef enum _DRAW_STATE_ERROR { DRAWSTATE_MISSING_ATTACHMENT_REFERENCE, // Attachment reference must be // present in active subpass DRAWSTATE_INVALID_EXTENSION, - DRAWSTATE_SAMPLER_DESCRIPTOR_ERROR, // A Descriptor of *_SAMPLER type is being updated with an invalid or bad Sampler - DRAWSTATE_INCONSISTENT_IMMUTABLE_SAMPLER_UPDATE, // Descriptors of *COMBINED_IMAGE_SAMPLER type are being updated where some, but not all, of the updates use immutable samplers - DRAWSTATE_IMAGEVIEW_DESCRIPTOR_ERROR, // A Descriptor of *_IMAGE or *_ATTACHMENT type is being updated with an invalid or bad ImageView - DRAWSTATE_BUFFERVIEW_DESCRIPTOR_ERROR, // A Descriptor of *_TEXEL_BUFFER type is being updated with an invalid or bad BufferView - DRAWSTATE_BUFFERINFO_DESCRIPTOR_ERROR, // A Descriptor of *_[UNIFORM|STORAGE]_BUFFER_[DYNAMIC] type is being updated with an invalid or bad BufferView - DRAWSTATE_DYNAMIC_OFFSET_OVERFLOW, // At draw time the dynamic offset combined with buffer offset and range oversteps size of buffer - DRAWSTATE_DOUBLE_DESTROY, // Destroying an object twice - DRAWSTATE_OBJECT_INUSE, // Destroying or modifying an object in use by a command buffer - DRAWSTATE_QUEUE_FORWARD_PROGRESS, // Queue cannot guarantee forward progress - DRAWSTATE_INVALID_UNIFORM_BUFFER_OFFSET, // Dynamic Uniform Buffer Offsets violate device limit - DRAWSTATE_INVALID_STORAGE_BUFFER_OFFSET, // Dynamic Storage Buffer Offsets violate device limit + DRAWSTATE_SAMPLER_DESCRIPTOR_ERROR, // A Descriptor of *_SAMPLER type is + // being updated with an invalid or bad + // Sampler + DRAWSTATE_INCONSISTENT_IMMUTABLE_SAMPLER_UPDATE, // Descriptors of + // *COMBINED_IMAGE_SAMPLER + // type are being updated + // where some, but not all, + // of the updates use + // immutable samplers + DRAWSTATE_IMAGEVIEW_DESCRIPTOR_ERROR, // A Descriptor of *_IMAGE or + // *_ATTACHMENT type is being updated + // with an invalid or bad ImageView + DRAWSTATE_BUFFERVIEW_DESCRIPTOR_ERROR, // A Descriptor of *_TEXEL_BUFFER + // type is being updated with an + // invalid or bad BufferView + DRAWSTATE_BUFFERINFO_DESCRIPTOR_ERROR, // A Descriptor of + // *_[UNIFORM|STORAGE]_BUFFER_[DYNAMIC] + // type is being updated with an + // invalid or bad BufferView + DRAWSTATE_DYNAMIC_OFFSET_OVERFLOW, // At draw time the dynamic offset + // combined with buffer offset and range + // oversteps size of buffer + DRAWSTATE_DOUBLE_DESTROY, // Destroying an object twice + DRAWSTATE_OBJECT_INUSE, // Destroying or modifying an object in use by a + // command buffer + DRAWSTATE_QUEUE_FORWARD_PROGRESS, // Queue cannot guarantee forward progress + DRAWSTATE_INVALID_UNIFORM_BUFFER_OFFSET, // Dynamic Uniform Buffer Offsets + // violate device limit + DRAWSTATE_INVALID_STORAGE_BUFFER_OFFSET, // Dynamic Storage Buffer Offsets + // violate device limit } DRAW_STATE_ERROR; typedef enum _SHADER_CHECKER_ERROR { @@ -339,6 +359,12 @@ class FENCE_NODE : public BASE_NODE { VkFence priorFence; }; +class SEMAPHORE_NODE : public BASE_NODE { + public: + using BASE_NODE::in_use; + uint32_t signaled; +}; + class EVENT_NODE : public BASE_NODE { public: using BASE_NODE::in_use; @@ -609,6 +635,7 @@ typedef struct _GLOBAL_CB_NODE { std::set updatedSets; vector boundDescriptorSets; // Index is set# that given set is bound to vector waitedEvents; + vector semaphores; unordered_map > waitedEventsBeforeQueryReset; unordered_map queryToStateMap; // 0 is unavailable, 1 is available unordered_set activeQueries; diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md index cce7234..247c67c 100644 --- a/layers/vk_validation_layer_details.md +++ b/layers/vk_validation_layer_details.md @@ -73,6 +73,7 @@ The VK_LAYER_LUNARG_draw_state layer tracks state leading into Draw cmds. This i | Verify Object Not In Use | Validate that object being freed or modified is not in use | OBJECT_INUSE | vkDestroyBuffer vkFreeDescriptorSets vkUpdateDescriptorSets | TBD | None | | Verify Get Queries| Validate that that queries are properly setup, initialized and synchronized | INVALID_QUERY | vkGetFenceStatus vkQueueWaitIdle vkWaitForFences vkDeviceWaitIdle vkCmdBeginQuery vkCmdEndQuery | TBD | None | | Verify Fences Not In Use | Validate that that fences are not used in multiple submit calls at the same time | INVALID_FENCE | vkQueueSubmit | TBD | None | +| Verify Semaphores Not In Use | Validate that the semaphores are not used in multiple submit calls at the same time | INVALID_SEMAPHORE | vkQueueSubmit | TBD | None | | Live Semaphore | When waiting on a semaphore, need to make sure that the semaphore is live and therefore can be signalled, otherwise queue is stalled and cannot make forward progress. | QUEUE_FORWARD_PROGRESS | vkQueueSubmit vkQueueBindSparse vkQueuePresentKHR vkAcquireNextImageKHR | TODO | Create test | | Storage Buffer Alignment | Storage Buffer offsets in BindDescriptorSets must agree with offset alignment device limit | INVALID_STORAGE_BUFFER_OFFSET | vkCmdBindDescriptorSets | TODO | Create test | | Uniform Buffer Alignment | Uniform Buffer offsets in BindDescriptorSets must agree with offset alignment device limit | INVALID_UNIFORM_BUFFER_OFFSET | vkCmdBindDescriptorSets | TODO | Create test | -- 2.7.4