layers: GH465 Add validation for now bound VkPipeline

At the time of a Draw or Dispatch, a pipeline must be bound to the
command buffer. This adds a check to verify that.
Added tests for both draw and compute cases to verify the check.

2 files changed, 110 insertions, 100 deletions

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index 2cadc628..7e2cb7e8 100644
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -2757,112 +2757,124 @@ static bool validate_and_update_draw_state(layer_data *my_data, GLOBAL_CB_NODE *
     bool result = false;
     auto const &state = pCB->lastBound[bindPoint];
     PIPELINE_NODE *pPipe = getPipeline(my_data, state.pipeline);
+    if (nullptr == pPipe) {
+        result |= log_msg(
+            my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VkDebugReportObjectTypeEXT(0), 0, __LINE__,
+            DRAWSTATE_INVALID_PIPELINE, "DS",
+            "At Draw/Dispatch time no valid VkPipeline is bound! This is illegal. Please bind one with vkCmdBindPipeline().");
+        // Early return as any further checks below will be busted w/o a pipeline
+        if (result)
+            return true;
+    }
     // First check flag states
     if (VK_PIPELINE_BIND_POINT_GRAPHICS == bindPoint)
         result = validate_draw_state_flags(my_data, pCB, pPipe, indexedDraw);
+    else {
+        // First block of code below to validate active sets should eventually
+        //  work for the compute case but currently doesn't so return early for now
+        // TODO : When active sets in compute shaders are correctly parsed,
+        //  stop returning early here and handle them in top block below
+        return result;
+    }
 
     // Now complete other state checks
-    // TODO : Currently only performing next check if *something* was bound (non-zero last bound)
-    //  There is probably a better way to gate when this check happens, and to know if something *should* have been bound
-    //  We should have that check separately and then gate this check based on that check
-    if (pPipe) {
-        if (state.pipelineLayout) {
-            string errorString;
-            // Need a vector (vs. std::set) of active Sets for dynamicOffset validation in case same set bound w/ different offsets
-            vector<std::pair<SET_NODE *, unordered_set<uint32_t>>> activeSetBindingsPairs;
-            for (auto setBindingPair : pPipe->active_slots) {
-                uint32_t setIndex = setBindingPair.first;
-                // If valid set is not bound throw an error
-                if ((state.boundDescriptorSets.size() <= setIndex) || (!state.boundDescriptorSets[setIndex])) {
-                    result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
-                                      __LINE__, DRAWSTATE_DESCRIPTOR_SET_NOT_BOUND, "DS",
-                                      "VkPipeline %#" PRIxLEAST64 " uses set #%u but that set is not bound.",
-                                      (uint64_t)pPipe->pipeline, setIndex);
-                } else if (!verify_set_layout_compatibility(my_data, my_data->setMap[state.boundDescriptorSets[setIndex]],
-                                                            pPipe->graphicsPipelineCI.layout, setIndex, errorString)) {
-                    // Set is bound but not compatible w/ overlapping pipelineLayout from PSO
-                    VkDescriptorSet setHandle = my_data->setMap[state.boundDescriptorSets[setIndex]]->set;
-                    result |= log_msg(
-                        my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
-                        (uint64_t)setHandle, __LINE__, DRAWSTATE_PIPELINE_LAYOUTS_INCOMPATIBLE, "DS",
-                        "VkDescriptorSet (%#" PRIxLEAST64
-                        ") bound as set #%u is not compatible with overlapping VkPipelineLayout %#" PRIxLEAST64 " due to: %s",
-                        (uint64_t)setHandle, setIndex, (uint64_t)pPipe->graphicsPipelineCI.layout, errorString.c_str());
-                } else { // Valid set is bound and layout compatible, validate that it's updated
-                    // Pull the set node
-                    SET_NODE *pSet = my_data->setMap[state.boundDescriptorSets[setIndex]];
-                    // Save vector of all active sets to verify dynamicOffsets below
-                    activeSetBindingsPairs.push_back(std::make_pair(pSet, setBindingPair.second));
-                    // Make sure set has been updated if it has no immutable samplers
-                    //  If it has immutable samplers, we'll flag error later as needed depending on binding
-                    if (!pSet->pUpdateStructs) {
-                        for (auto binding : setBindingPair.second) {
-                            if (!pSet->p_layout->GetImmutableSamplerPtrFromBinding(binding)) {
-                                result |=
-                                    log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
-                                            VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, (uint64_t)pSet->set, __LINE__,
-                                            DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, "DS",
-                                            "DS %#" PRIxLEAST64 " bound but it was never updated. It is now being used to draw so "
-                                            "this will result in undefined behavior.",
-                                            (uint64_t)pSet->set);
-                            }
+    // TODO : When Compute shaders are properly parsed, fix this section to validate them as well
+    if (state.pipelineLayout) {
+        string errorString;
+        // Need a vector (vs. std::set) of active Sets for dynamicOffset validation in case same set bound w/ different offsets
+        vector<std::pair<SET_NODE *, unordered_set<uint32_t>>> activeSetBindingsPairs;
+        for (auto setBindingPair : pPipe->active_slots) {
+            uint32_t setIndex = setBindingPair.first;
+            // If valid set is not bound throw an error
+            if ((state.boundDescriptorSets.size() <= setIndex) || (!state.boundDescriptorSets[setIndex])) {
+                result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__,
+                                  DRAWSTATE_DESCRIPTOR_SET_NOT_BOUND, "DS",
+                                  "VkPipeline %#" PRIxLEAST64 " uses set #%u but that set is not bound.", (uint64_t)pPipe->pipeline,
+                                  setIndex);
+            } else if (!verify_set_layout_compatibility(my_data, my_data->setMap[state.boundDescriptorSets[setIndex]],
+                                                        pPipe->graphicsPipelineCI.layout, setIndex, errorString)) {
+                // Set is bound but not compatible w/ overlapping pipelineLayout from PSO
+                VkDescriptorSet setHandle = my_data->setMap[state.boundDescriptorSets[setIndex]]->set;
+                result |=
+                    log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                            (uint64_t)setHandle, __LINE__, DRAWSTATE_PIPELINE_LAYOUTS_INCOMPATIBLE, "DS",
+                            "VkDescriptorSet (%#" PRIxLEAST64
+                            ") bound as set #%u is not compatible with overlapping VkPipelineLayout %#" PRIxLEAST64 " due to: %s",
+                            (uint64_t)setHandle, setIndex, (uint64_t)pPipe->graphicsPipelineCI.layout, errorString.c_str());
+            } else { // Valid set is bound and layout compatible, validate that it's updated
+                // Pull the set node
+                SET_NODE *pSet = my_data->setMap[state.boundDescriptorSets[setIndex]];
+                // Save vector of all active sets to verify dynamicOffsets below
+                activeSetBindingsPairs.push_back(std::make_pair(pSet, setBindingPair.second));
+                // Make sure set has been updated if it has no immutable samplers
+                //  If it has immutable samplers, we'll flag error later as needed depending on binding
+                if (!pSet->pUpdateStructs) {
+                    for (auto binding : setBindingPair.second) {
+                        if (!pSet->p_layout->GetImmutableSamplerPtrFromBinding(binding)) {
+                            result |= log_msg(
+                                my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                                (uint64_t)pSet->set, __LINE__, DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, "DS",
+                                "DS %#" PRIxLEAST64 " bound but it was never updated. It is now being used to draw so "
+                                "this will result in undefined behavior.",
+                                (uint64_t)pSet->set);
                         }
                     }
                 }
             }
-            // For given active slots, verify any dynamic descriptors and record updated images & buffers
-            result |= validate_and_update_drawtime_descriptor_state(my_data, pCB, activeSetBindingsPairs);
-        }
-        if (VK_PIPELINE_BIND_POINT_GRAPHICS == bindPoint) {
-            // Verify Vtx binding
-            if (pPipe->vertexBindingDescriptions.size() > 0) {
-                for (size_t i = 0; i < pPipe->vertexBindingDescriptions.size(); i++) {
-                    if ((pCB->currentDrawData.buffers.size() < (i + 1)) || (pCB->currentDrawData.buffers[i] == VK_NULL_HANDLE)) {
-                        result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
-                                          __LINE__, DRAWSTATE_VTX_INDEX_OUT_OF_BOUNDS, "DS",
-                                          "The Pipeline State Object (%#" PRIxLEAST64
-                                          ") expects that this Command Buffer's vertex binding Index " PRINTF_SIZE_T_SPECIFIER
-                                          " should be set via vkCmdBindVertexBuffers.",
-                                          (uint64_t)state.pipeline, i);
-                    }
-                }
-            } else {
-                if (!pCB->currentDrawData.buffers.empty()) {
-                    result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT,
-                                      (VkDebugReportObjectTypeEXT)0, 0, __LINE__, DRAWSTATE_VTX_INDEX_OUT_OF_BOUNDS, "DS",
-                                      "Vertex buffers are bound to command buffer (%#" PRIxLEAST64
-                                      ") but no vertex buffers are attached to this Pipeline State Object (%#" PRIxLEAST64 ").",
-                                      (uint64_t)pCB->commandBuffer, (uint64_t)state.pipeline);
-                }
-            }
-            // If Viewport or scissors are dynamic, verify that dynamic count matches PSO count.
-            // Skip check if rasterization is disabled or there is no viewport.
-            if ((!pPipe->graphicsPipelineCI.pRasterizationState ||
-                 (pPipe->graphicsPipelineCI.pRasterizationState->rasterizerDiscardEnable == VK_FALSE)) &&
-                pPipe->graphicsPipelineCI.pViewportState) {
-                bool dynViewport = isDynamic(pPipe, VK_DYNAMIC_STATE_VIEWPORT);
-                bool dynScissor = isDynamic(pPipe, VK_DYNAMIC_STATE_SCISSOR);
-                if (dynViewport) {
-                    if (pCB->viewports.size() != pPipe->graphicsPipelineCI.pViewportState->viewportCount) {
-                        result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
-                                          __LINE__, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
-                                          "Dynamic viewportCount from vkCmdSetViewport() is " PRINTF_SIZE_T_SPECIFIER
-                                          ", but PSO viewportCount is %u. These counts must match.",
-                                          pCB->viewports.size(), pPipe->graphicsPipelineCI.pViewportState->viewportCount);
-                    }
-                }
-                if (dynScissor) {
-                    if (pCB->scissors.size() != pPipe->graphicsPipelineCI.pViewportState->scissorCount) {
-                        result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
-                                          __LINE__, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
-                                          "Dynamic scissorCount from vkCmdSetScissor() is " PRINTF_SIZE_T_SPECIFIER
-                                          ", but PSO scissorCount is %u. These counts must match.",
-                                          pCB->scissors.size(), pPipe->graphicsPipelineCI.pViewportState->scissorCount);
-                    }
-                }
-            }
         }
+        // For given active slots, verify any dynamic descriptors and record updated images & buffers
+        result |= validate_and_update_drawtime_descriptor_state(my_data, pCB, activeSetBindingsPairs);
     }
+    // TODO : If/when compute pipelines/shaders are handled above, code below is only for gfx bind poing
+    //if (VK_PIPELINE_BIND_POINT_GRAPHICS == bindPoint) {
+    // Verify Vtx binding
+    if (pPipe->vertexBindingDescriptions.size() > 0) {
+        for (size_t i = 0; i < pPipe->vertexBindingDescriptions.size(); i++) {
+            if ((pCB->currentDrawData.buffers.size() < (i + 1)) || (pCB->currentDrawData.buffers[i] == VK_NULL_HANDLE)) {
+                result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
+                                  __LINE__, DRAWSTATE_VTX_INDEX_OUT_OF_BOUNDS, "DS",
+                                  "The Pipeline State Object (%#" PRIxLEAST64
+                                  ") expects that this Command Buffer's vertex binding Index " PRINTF_SIZE_T_SPECIFIER
+                                  " should be set via vkCmdBindVertexBuffers.",
+                                  (uint64_t)state.pipeline, i);
+            }
+        }
+    } else {
+        if (!pCB->currentDrawData.buffers.empty()) {
+            result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT, (VkDebugReportObjectTypeEXT)0,
+                              0, __LINE__, DRAWSTATE_VTX_INDEX_OUT_OF_BOUNDS, "DS",
+                              "Vertex buffers are bound to command buffer (%#" PRIxLEAST64
+                              ") but no vertex buffers are attached to this Pipeline State Object (%#" PRIxLEAST64 ").",
+                              (uint64_t)pCB->commandBuffer, (uint64_t)state.pipeline);
+        }
+    }
+    // If Viewport or scissors are dynamic, verify that dynamic count matches PSO count.
+    // Skip check if rasterization is disabled or there is no viewport.
+    if ((!pPipe->graphicsPipelineCI.pRasterizationState ||
+         (pPipe->graphicsPipelineCI.pRasterizationState->rasterizerDiscardEnable == VK_FALSE)) &&
+        pPipe->graphicsPipelineCI.pViewportState) {
+        bool dynViewport = isDynamic(pPipe, VK_DYNAMIC_STATE_VIEWPORT);
+        bool dynScissor = isDynamic(pPipe, VK_DYNAMIC_STATE_SCISSOR);
+        if (dynViewport) {
+            if (pCB->viewports.size() != pPipe->graphicsPipelineCI.pViewportState->viewportCount) {
+                result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
+                                  __LINE__, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                                  "Dynamic viewportCount from vkCmdSetViewport() is " PRINTF_SIZE_T_SPECIFIER
+                                  ", but PSO viewportCount is %u. These counts must match.",
+                                  pCB->viewports.size(), pPipe->graphicsPipelineCI.pViewportState->viewportCount);
+            }
+        }
+        if (dynScissor) {
+            if (pCB->scissors.size() != pPipe->graphicsPipelineCI.pViewportState->scissorCount) {
+                result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0,
+                                  __LINE__, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                                  "Dynamic scissorCount from vkCmdSetScissor() is " PRINTF_SIZE_T_SPECIFIER
+                                  ", but PSO scissorCount is %u. These counts must match.",
+                                  pCB->scissors.size(), pPipe->graphicsPipelineCI.pViewportState->scissorCount);
+            }
+        }
+    }
+    //} // end of "if (VK_PIPELINE_BIND_POINT_GRAPHICS == bindPoint) {" block
     return result;
 }
 
@@ -7351,8 +7363,7 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdDispatch(VkCommandBuffer command
     std::unique_lock<std::mutex> lock(global_lock);
     GLOBAL_CB_NODE *pCB = getCBNode(dev_data, commandBuffer);
     if (pCB) {
-        // TODO : Re-enable validate_and_update_draw_state() when it supports compute shaders
-        // skipCall |= validate_and_update_draw_state(dev_data, pCB, false, VK_PIPELINE_BIND_POINT_COMPUTE);
+        skipCall |= validate_and_update_draw_state(dev_data, pCB, false, VK_PIPELINE_BIND_POINT_COMPUTE);
         // TODO : Call below is temporary until call above can be re-enabled
         update_shader_storage_images_and_buffers(dev_data, pCB);
         skipCall |= markStoreImagesAndBuffersAsWritten(dev_data, pCB);
@@ -7377,8 +7388,7 @@ vkCmdDispatchIndirect(VkCommandBuffer commandBuffer, VkBuffer buffer, VkDeviceSi
 #endif
     GLOBAL_CB_NODE *pCB = getCBNode(dev_data, commandBuffer);
     if (pCB) {
-        // TODO : Re-enable validate_and_update_draw_state() when it supports compute shaders
-        // skipCall |= validate_and_update_draw_state(dev_data, pCB, false, VK_PIPELINE_BIND_POINT_COMPUTE);
+        skipCall |= validate_and_update_draw_state(dev_data, pCB, false, VK_PIPELINE_BIND_POINT_COMPUTE);
         // TODO : Call below is temporary until call above can be re-enabled
         update_shader_storage_images_and_buffers(dev_data, pCB);
         skipCall |= markStoreImagesAndBuffersAsWritten(dev_data, pCB);
diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md
index 71c5ade9..803d3f05 100644
--- a/layers/vk_validation_layer_details.md
+++ b/layers/vk_validation_layer_details.md
@@ -39,7 +39,7 @@ The Draw State portion of the core validation layer tracks state leading into Dr
 | Valid DescriptorSet | Validate that descriptor set was properly created and is currently valid | INVALID_SET | vkCmdBindDescriptorSets | None | Is this needed other places (like Update/Clear descriptors) |
 | Valid DescriptorSetLayout | Flag DescriptorSetLayout object that was not properly created | INVALID_LAYOUT | vkAllocateDescriptorSets | None | Anywhere else to check this? |
 | Valid RenderArea | Flag renderArea field that is outside of the framebuffer | INVALID_RENDER_AREA | vkCmdBeginRenderPass | None | Anywhere else to check this? |
-| Valid Pipeline | Flag VkPipeline object that was not properly created | INVALID_PIPELINE | vkCmdBindPipeline | InvalidPipeline | NA |
+| Valid Pipeline | Flag VkPipeline object that was not properly created, or case when Draw/Dispatch is bound to cmd buffer without a pipeline being bound | INVALID_PIPELINE | vkCmdBindPipeline | InvalidPipeline | NA |
 | Valid PipelineLayout | Flag VkPipelineLayout object that was not properly created | INVALID_PIPELINE_LAYOUT | vkCmdBindPipeline | TODO | Write test for this case |
 | Valid Pipeline Create Info | Tests for the following: That compute shaders are not specified for the graphics pipeline, tess evaluation and tess control shaders are included or excluded as a pair, that VK_PRIMITIVE_TOPOLOGY_PATCH_LIST is set as IA topology for tessellation pipelines, that VK_PRIMITIVE_TOPOLOGY_PATCH_LIST primitive topology is only set for tessellation pipelines, and that Vtx Shader specified | INVALID_PIPELINE_CREATE_STATE | vkCreateGraphicsPipelines | InvalidPipelineCreateState | NA |
 | Valid CommandBuffer | Validates that the command buffer object was properly created and is currently valid | INVALID_COMMAND_BUFFER | vkQueueSubmit vkBeginCommandBuffer vkEndCommandBuffer vkCmdBindPipeline vkCmdBindDescriptorSets vkCmdBindIndexBuffer vkCmdBindVertexBuffers vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect vkCmdDispatch vkCmdDispatchIndirect vkCmdCopyBuffer vkCmdCopyImage vkCmdBlitImage vkCmdCopyBufferToImage vkCmdCopyImageToBuffer vkCmdUpdateBuffer vkCmdFillBuffer vkCmdClearAttachments vkCmdClearColorImage vkCmdClearDepthStencilImage vkCmdResolveImage vkCmdSetEvent vkCmdResetEvent vkCmdWaitEvents vkCmdPipelineBarrier vkCmdBeginQuery vkCmdEndQuery vkCmdResetQueryPool vkCmdWriteTimestamp vkCmdBeginRenderPass vkCmdNextSubpass vkCmdEndRenderPass vkCmdExecuteCommands vkAllocateCommandBuffers | None | NA |