From 17ecbde130b930d3e8515c0797785e07886ecf4b Mon Sep 17 00:00:00 2001 From: Karl Schultz Date: Wed, 2 May 2018 19:40:34 -0600 Subject: layers: Add checks for indexed draw count/offset Check that the first index and index count are within the bounds of an index buffer for DrawIndexed, taking into account any offset specified in the binding of the index buffer. Fixes #2448 Change-Id: I47a1acbf0bb0e586692c75380d8d43a1c72450fd --- layers/core_validation.cpp | 35 +++++++++++++++++++++++++++++---- layers/core_validation_types.h | 9 +++++++++ layers/vk_validation_error_database.txt | 2 +- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index ca854df8..755699bd 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -1944,6 +1944,7 @@ static void ResetCommandBufferState(layer_data *dev_data, const VkCommandBuffer } pCB->framebuffers.clear(); pCB->activeFramebuffer = VK_NULL_HANDLE; + memset(&pCB->index_buffer_binding, 0, sizeof(pCB->index_buffer_binding)); } } @@ -6708,6 +6709,10 @@ VKAPI_ATTR void VKAPI_CALL CmdBindIndexBuffer(VkCommandBuffer commandBuffer, VkB if (skip) return; cb_node->status |= CBSTATUS_INDEX_BUFFER_BOUND; + cb_node->index_buffer_binding.buffer = buffer; + cb_node->index_buffer_binding.size = buffer_state->createInfo.size; + cb_node->index_buffer_binding.offset = offset; + cb_node->index_buffer_binding.index_type = indexType; lock.unlock(); dev_data->dispatch_table.CmdBindIndexBuffer(commandBuffer, buffer, offset, indexType); @@ -6812,9 +6817,31 @@ VKAPI_ATTR void VKAPI_CALL CmdDraw(VkCommandBuffer commandBuffer, uint32_t verte } static bool PreCallValidateCmdDrawIndexed(layer_data *dev_data, VkCommandBuffer cmd_buffer, bool indexed, - VkPipelineBindPoint bind_point, GLOBAL_CB_NODE **cb_state, const char *caller) { - return ValidateCmdDrawType(dev_data, cmd_buffer, indexed, bind_point, CMD_DRAWINDEXED, cb_state, caller, VK_QUEUE_GRAPHICS_BIT, - VALIDATION_ERROR_1a402415, VALIDATION_ERROR_1a400017, VALIDATION_ERROR_1a40039c); + VkPipelineBindPoint bind_point, GLOBAL_CB_NODE **cb_state, const char *caller, + uint32_t indexCount, uint32_t firstIndex) { + bool skip = + ValidateCmdDrawType(dev_data, cmd_buffer, indexed, bind_point, CMD_DRAWINDEXED, cb_state, caller, VK_QUEUE_GRAPHICS_BIT, + VALIDATION_ERROR_1a402415, VALIDATION_ERROR_1a400017, VALIDATION_ERROR_1a40039c); + if (!skip && ((*cb_state)->status & CBSTATUS_INDEX_BUFFER_BOUND)) { + unsigned int index_size = 0; + const auto &index_buffer_binding = (*cb_state)->index_buffer_binding; + if (index_buffer_binding.index_type == VK_INDEX_TYPE_UINT16) { + index_size = 2; + } else if (index_buffer_binding.index_type == VK_INDEX_TYPE_UINT32) { + index_size = 4; + } + VkDeviceSize end_offset = (index_size * ((VkDeviceSize)firstIndex + indexCount)) + index_buffer_binding.offset; + if (end_offset > index_buffer_binding.size) { + skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT, + HandleToUint64(index_buffer_binding.buffer), VALIDATION_ERROR_1a40039e, + "vkCmdDrawIndexed() index size (%d) * (firstIndex (%d) + indexCount (%d)) " + "+ binding offset (%" PRIuLEAST64 ") = an ending offset of %" PRIuLEAST64 + " bytes, " + "which is greater than the index buffer size (%" PRIuLEAST64 ").", + index_size, firstIndex, indexCount, index_buffer_binding.offset, end_offset, index_buffer_binding.size); + } + } + return skip; } static void PostCallRecordCmdDrawIndexed(layer_data *dev_data, GLOBAL_CB_NODE *cb_state, VkPipelineBindPoint bind_point) { @@ -6827,7 +6854,7 @@ VKAPI_ATTR void VKAPI_CALL CmdDrawIndexed(VkCommandBuffer commandBuffer, uint32_ GLOBAL_CB_NODE *cb_state = nullptr; unique_lock_t lock(global_lock); bool skip = PreCallValidateCmdDrawIndexed(dev_data, commandBuffer, true, VK_PIPELINE_BIND_POINT_GRAPHICS, &cb_state, - "vkCmdDrawIndexed()"); + "vkCmdDrawIndexed()", indexCount, firstIndex); lock.unlock(); if (!skip) { dev_data->dispatch_table.CmdDrawIndexed(commandBuffer, indexCount, instanceCount, firstIndex, vertexOffset, firstInstance); diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h index f9f33387..58231daf 100644 --- a/layers/core_validation_types.h +++ b/layers/core_validation_types.h @@ -165,6 +165,13 @@ struct MEM_BINDING { VkDeviceSize size; }; +struct INDEX_BUFFER_BINDING { + VkBuffer buffer; + VkDeviceSize size; + VkDeviceSize offset; + VkIndexType index_type; +}; + inline bool operator==(MEM_BINDING a, MEM_BINDING b) NOEXCEPT { return a.mem == b.mem && a.offset == b.offset && a.size == b.size; } namespace std { @@ -781,6 +788,8 @@ struct GLOBAL_CB_NODE : public BASE_NODE { std::vector> eventUpdates; std::vector> queryUpdates; std::unordered_set validated_descriptor_sets; + // Contents valid only after an index buffer is bound (CBSTATUS_INDEX_BUFFER_BOUND set) + INDEX_BUFFER_BINDING index_buffer_binding; }; struct SEMAPHORE_WAIT { diff --git a/layers/vk_validation_error_database.txt b/layers/vk_validation_error_database.txt index 289d4030..1e7e446b 100644 --- a/layers/vk_validation_error_database.txt +++ b/layers/vk_validation_error_database.txt @@ -2099,7 +2099,7 @@ VALIDATION_ERROR_1a400396~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndex VALIDATION_ERROR_1a400398~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-None-00460~^~core~^~The spec valid usage text states 'For a given vertex buffer binding, any attribute data fetched must be entirely contained within the corresponding vertex buffer binding, as described in fxvertex-input' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-None-00460)~^~ VALIDATION_ERROR_1a40039a~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-None-00461~^~core~^~The spec valid usage text states 'A valid graphics pipeline must be bound to the current command buffer with VK_PIPELINE_BIND_POINT_GRAPHICS' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-None-00461)~^~ VALIDATION_ERROR_1a40039c~^~Y~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-None-00462~^~core~^~The spec valid usage text states 'If the VkPipeline object bound to VK_PIPELINE_BIND_POINT_GRAPHICS requires any dynamic state, that state must have been set on the current command buffer' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-None-00462)~^~ -VALIDATION_ERROR_1a40039e~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-indexSize-00463~^~core~^~The spec valid usage text states '(indexSize * (firstIndex + indexCount) + offset) must be less than or equal to the size of the bound index buffer, with indexSize being based on the type specified by indexType, where the index buffer, indexType, and offset are specified via vkCmdBindIndexBuffer' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-indexSize-00463)~^~ +VALIDATION_ERROR_1a40039e~^~Y~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-indexSize-00463~^~core~^~The spec valid usage text states '(indexSize * (firstIndex + indexCount) + offset) must be less than or equal to the size of the bound index buffer, with indexSize being based on the type specified by indexType, where the index buffer, indexType, and offset are specified via vkCmdBindIndexBuffer' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-indexSize-00463)~^~ VALIDATION_ERROR_1a4003a0~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-None-00464~^~core~^~The spec valid usage text states 'Every input attachment used by the current subpass must be bound to the pipeline via a descriptor set' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-None-00464)~^~ VALIDATION_ERROR_1a4003a2~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-None-00465~^~core~^~The spec valid usage text states 'If any VkSampler object that is accessed from a shader by the VkPipeline bound to VK_PIPELINE_BIND_POINT_GRAPHICS uses unnormalized coordinates, it must not be used to sample from any VkImage with a VkImageView of the type VK_IMAGE_VIEW_TYPE_3D, VK_IMAGE_VIEW_TYPE_CUBE, VK_IMAGE_VIEW_TYPE_1D_ARRAY, VK_IMAGE_VIEW_TYPE_2D_ARRAY or VK_IMAGE_VIEW_TYPE_CUBE_ARRAY, in any shader stage' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-None-00465)~^~ VALIDATION_ERROR_1a4003a4~^~N~^~Unknown~^~vkCmdDrawIndexed~^~VUID-vkCmdDrawIndexed-None-00466~^~core~^~The spec valid usage text states 'If any VkSampler object that is accessed from a shader by the VkPipeline bound to VK_PIPELINE_BIND_POINT_GRAPHICS uses unnormalized coordinates, it must not be used with any of the SPIR-V OpImageSample* or OpImageSparseSample* instructions with ImplicitLod, Dref or Proj in their name, in any shader stage' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdDrawIndexed-None-00466)~^~ -- cgit v1.2.3