From bd42cfd170990e0348bc78d6a62ac20a2f4ccba1 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Wed, 17 Feb 2016 10:35:18 -0700 Subject: [PATCH] layers: Add push constant validation to draw_state Validate that push constants don't exceed maxPushConstantSize at create or update time. Also validate that size at creation time is >0 and a multiple of 4. --- layers/draw_state.cpp | 70 ++++++++++++++++++++++++++++++++++- layers/draw_state.h | 11 +++--- layers/vk_validation_layer_details.md | 1 + 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp index 3b66960..eb5839b 100644 --- a/layers/draw_state.cpp +++ b/layers/draw_state.cpp @@ -4910,16 +4910,52 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateDescriptorSetLayout(VkDev return result; } +static bool validatePushConstantSize(const layer_data *dev_data, + const uint32_t offset, const uint32_t size, + const char *caller_name) { + bool skipCall = false; + if ((offset + size) > + dev_data->physDevProperties.properties.limits.maxPushConstantsSize) { + skipCall = log_msg( + dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, + (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_PUSH_CONSTANTS_ERROR, "DS", + "%s call has push constants with offset %u and size %u that " + "exceeds this device's maxPushConstantSize of %u.", + caller_name, offset, size, + dev_data->physDevProperties.properties.limits.maxPushConstantsSize); + } + return skipCall; +} + VKAPI_ATTR VkResult VKAPI_CALL vkCreatePipelineLayout(VkDevice device, const VkPipelineLayoutCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkPipelineLayout* pPipelineLayout) { + bool skipCall = false; layer_data* dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); + uint32_t i = 0; + for (i = 0; i < pCreateInfo->pushConstantRangeCount; ++i) { + skipCall |= validatePushConstantSize( + dev_data, pCreateInfo->pPushConstantRanges[i].offset, + pCreateInfo->pPushConstantRanges[i].size, + "vkCreatePipelineLayout()"); + if ((pCreateInfo->pPushConstantRanges[i].size == 0) || + ((pCreateInfo->pPushConstantRanges[i].size & 0x3) != 0)) { + skipCall |= log_msg( + dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, + (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_PUSH_CONSTANTS_ERROR, "DS", + "vkCreatePipelineLayout() call has push constant index %u with " + "size %u. Size must be greater than zero and a multiple of 4.", + i, pCreateInfo->pPushConstantRanges[i].size); + } + // TODO : Add warning if ranges overlap + } VkResult result = dev_data->device_dispatch_table->CreatePipelineLayout(device, pCreateInfo, pAllocator, pPipelineLayout); if (VK_SUCCESS == result) { loader_platform_thread_lock_mutex(&globalLock); // TODOSC : Merge capture of the setLayouts per pipeline PIPELINE_LAYOUT_NODE& plNode = dev_data->pipelineLayoutMap[*pPipelineLayout]; plNode.descriptorSetLayouts.resize(pCreateInfo->setLayoutCount); - uint32_t i = 0; for (i=0; isetLayoutCount; ++i) { plNode.descriptorSetLayouts[i] = pCreateInfo->pSetLayouts[i]; } @@ -6743,6 +6779,36 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdCopyQueryPoolResults(VkCommandBu firstQuery, queryCount, dstBuffer, dstOffset, stride, flags); } +VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL +vkCmdPushConstants(VkCommandBuffer commandBuffer, VkPipelineLayout layout, + VkShaderStageFlags stageFlags, uint32_t offset, + uint32_t size, const void *pValues) { + bool skipCall = false; + layer_data *dev_data = + get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map); + loader_platform_thread_lock_mutex(&globalLock); + GLOBAL_CB_NODE *pCB = getCBNode(dev_data, commandBuffer); + if (pCB) { + if (pCB->state == CB_RECORDING) { + skipCall |= addCmd(dev_data, pCB, CMD_PUSHCONSTANTS, + "vkCmdPushConstants()"); + } else { + skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, + "vkCmdPushConstants()"); + } + } + if ((offset + size) > + dev_data->physDevProperties.properties.limits.maxPushConstantsSize) { + skipCall |= validatePushConstantSize(dev_data, offset, size, + "vkCmdPushConstants()"); + } + // TODO : Add warning if push constant update doesn't align with range + loader_platform_thread_unlock_mutex(&globalLock); + if (!skipCall) + dev_data->device_dispatch_table->CmdPushConstants( + commandBuffer, layout, stageFlags, offset, size, pValues); +} + VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdWriteTimestamp(VkCommandBuffer commandBuffer, VkPipelineStageFlagBits pipelineStage, VkQueryPool queryPool, uint32_t slot) { VkBool32 skipCall = VK_FALSE; @@ -8100,6 +8166,8 @@ VK_LAYER_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL vkGetDeviceProcAddr(VkD return (PFN_vkVoidFunction) vkCmdEndQuery; if (!strcmp(funcName, "vkCmdResetQueryPool")) return (PFN_vkVoidFunction) vkCmdResetQueryPool; + if (!strcmp(funcName, "vkCmdPushConstants")) + return (PFN_vkVoidFunction)vkCmdPushConstants; if (!strcmp(funcName, "vkCmdWriteTimestamp")) return (PFN_vkVoidFunction) vkCmdWriteTimestamp; if (!strcmp(funcName, "vkCreateFramebuffer")) diff --git a/layers/draw_state.h b/layers/draw_state.h index 2585649..957def9 100644 --- a/layers/draw_state.h +++ b/layers/draw_state.h @@ -178,9 +178,9 @@ typedef enum _DRAW_STATE_ERROR { // 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 + // *_[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 @@ -200,6 +200,7 @@ typedef enum _DRAW_STATE_ERROR { // must be a valid VkLogicOp value DRAWSTATE_INVALID_QUEUE_INDEX, // Specified queue index exceeds number // of queried queue families + DRAWSTATE_PUSH_CONSTANTS_ERROR, // Push constants exceed maxPushConstantSize } DRAW_STATE_ERROR; typedef enum _SHADER_CHECKER_ERROR { @@ -482,8 +483,7 @@ typedef struct _DESCRIPTOR_POOL_NODE { } DESCRIPTOR_POOL_NODE; // Cmd Buffer Tracking -typedef enum _CMD_TYPE -{ +typedef enum _CMD_TYPE { CMD_BINDPIPELINE, CMD_BINDPIPELINEDELTA, CMD_SETVIEWPORTSTATE, @@ -525,6 +525,7 @@ typedef enum _CMD_TYPE CMD_RESETQUERYPOOL, CMD_COPYQUERYPOOLRESULTS, CMD_WRITETIMESTAMP, + CMD_PUSHCONSTANTS, CMD_INITATOMICCOUNTERS, CMD_LOADATOMICCOUNTERS, CMD_SAVEATOMICCOUNTERS, diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md index 57fa828..39531ce 100644 --- a/layers/vk_validation_layer_details.md +++ b/layers/vk_validation_layer_details.md @@ -87,6 +87,7 @@ The VK_LAYER_LUNARG_draw_state layer tracks state leading into Draw cmds. This i | Enabled Logic Operations | If logic operations is not enabled, logicOpEnable must be VK_FALSE | DISABLED_LOGIC_OP | vkCreateGraphicsPipelines | TODO | Create test | | Valid Logic Operations | If logicOpEnable is VK_TRUE, logicOp must be a valid VkLogicOp value | INVALID_LOGIC_OP | vkCreateGraphicsPipelines | TODO | Create test | | QueueFamilyIndex is Valid | Validates that QueueFamilyIndices are less an the number of QueueFamilies | INVALID_QUEUE_INDEX | vkCmdWaitEvents vkCmdPipelineBarrier vkCreateBuffer vkCreateImage | TODO | Create test | +| Push Constants | Validate that the size of push constant ranges and updates does not exceed maxPushConstantSize | PUSH_CONSTANTS_ERROR | vkCreatePipelineLayout vkCmdPushConstants | TODO | Create test | | NA | Enum used for informational messages | NONE | | NA | None | | NA | Enum used for errors in the layer itself. This does not indicate an app issue, but instead a bug in the layer. | INTERNAL_ERROR | | NA | None | | NA | Enum used when VK_LAYER_LUNARG_draw_state attempts to allocate memory for its own internal use and is unable to. | OUT_OF_MEMORY | | NA | None | -- 2.7.4