From: Karl Schultz Date: Fri, 6 May 2016 19:56:42 +0000 (-0600) Subject: tests: GH431 Add layer checks and tests for push constant X-Git-Tag: upstream/1.1.92~3071 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6dfbf31cb2af1e3281300b50e69d12379a17b039;p=platform%2Fupstream%2FVulkan-Tools.git tests: GH431 Add layer checks and tests for push constant Change-Id: I46eb5f9adbe702a278cded0f73dc18d1617489f0 Fix windows compilation issues Change-Id: Ic9b9795e34d1e1e68bae9838b8538bb2e37d18b4 --- diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 236474e..ce404d2 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -5784,14 +5784,67 @@ CreateDescriptorSetLayout(VkDevice device, const VkDescriptorSetLayoutCreateInfo return result; } -static bool validatePushConstantSize(const layer_data *dev_data, const uint32_t offset, const uint32_t size, - const char *caller_name) { +// Used by CreatePipelineLayout and CmdPushConstants. +// Note that the index argument is optional and only used by CreatePipelineLayout. +static bool validatePushConstantRange(const layer_data *dev_data, const uint32_t offset, const uint32_t size, + const char *caller_name, uint32_t index = 0) { + uint32_t const maxPushConstantsSize = dev_data->phys_dev_properties.properties.limits.maxPushConstantsSize; bool skipCall = false; - if ((offset + size) > dev_data->phys_dev_properties.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->phys_dev_properties.properties.limits.maxPushConstantsSize); + // Check that offset + size don't exceed the max. + // Prevent arithetic overflow here by avoiding addition and testing in this order. + if ((offset >= maxPushConstantsSize) || (size > maxPushConstantsSize - offset)) { + // This is a pain just to adapt the log message to the caller, but better to sort it out only when there is a problem. + if (0 == strcmp(caller_name, "vkCreatePipelineLayout()")) { + 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 index %u with offset %u and size %u that " + "exceeds this device's maxPushConstantSize of %u.", + caller_name, index, offset, size, maxPushConstantsSize); + } else if (0 == strcmp(caller_name, "vkCmdPushConstants()")) { + 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, maxPushConstantsSize); + } else { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_INTERNAL_ERROR, "DS", "%s caller not supported.", caller_name); + } + } + // size needs to be non-zero and a multiple of 4. + if ((size == 0) || ((size & 0x3) != 0)) { + if (0 == strcmp(caller_name, "vkCreatePipelineLayout()")) { + 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 index %u with " + "size %u. Size must be greater than zero and a multiple of 4.", + caller_name, index, size); + } else if (0 == strcmp(caller_name, "vkCmdPushConstants()")) { + 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 " + "size %u. Size must be greater than zero and a multiple of 4.", + caller_name, size); + } else { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_INTERNAL_ERROR, "DS", "%s caller not supported.", caller_name); + } + } + // offset needs to be a multiple of 4. + if ((offset & 0x3) != 0) { + if (0 == strcmp(caller_name, "vkCreatePipelineLayout()")) { + 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 index %u with " + "offset %u. Offset must be a multiple of 4.", + caller_name, index, offset); + } else if (0 == strcmp(caller_name, "vkCmdPushConstants()")) { + 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. Offset must be a multiple of 4.", + caller_name, offset); + } else { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_INTERNAL_ERROR, "DS", "%s caller not supported.", caller_name); + } } return skipCall; } @@ -5800,18 +5853,34 @@ VKAPI_ATTR VkResult VKAPI_CALL CreatePipelineLayout(VkDevice device, const VkPip const VkAllocationCallbacks *pAllocator, VkPipelineLayout *pPipelineLayout) { bool skipCall = false; layer_data *dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); + // Push Constant Range checks 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); + skipCall |= validatePushConstantRange(dev_data, pCreateInfo->pPushConstantRanges[i].offset, + pCreateInfo->pPushConstantRanges[i].size, "vkCreatePipelineLayout()", i); + if (0 == pCreateInfo->pPushConstantRanges[i].stageFlags) { + 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 no stageFlags set."); + } + } + // Each range has been validated. Now check for overlap between ranges (if they are good). + if (!skipCall) { + uint32_t i, j; + for (i = 0; i < pCreateInfo->pushConstantRangeCount; ++i) { + for (j = i + 1; j < pCreateInfo->pushConstantRangeCount; ++j) { + const uint32_t minA = pCreateInfo->pPushConstantRanges[i].offset; + const uint32_t maxA = minA + pCreateInfo->pPushConstantRanges[i].size; + const uint32_t minB = pCreateInfo->pPushConstantRanges[j].offset; + const uint32_t maxB = minB + pCreateInfo->pPushConstantRanges[j].size; + if ((minA <= minB && maxA > minB) || (minB <= minA && maxB > minA)) { + 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 constants with " + "overlapping ranges: %u:[%u, %u), %u:[%u, %u)", + i, minA, maxA, j, minB, maxB); + } + } } - // TODO : Add warning if ranges overlap } if (skipCall) @@ -8006,10 +8075,36 @@ VKAPI_ATTR void VKAPI_CALL CmdPushConstants(VkCommandBuffer commandBuffer, VkPip skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdPushConstants()"); } } - if ((offset + size) > dev_data->phys_dev_properties.properties.limits.maxPushConstantsSize) { - skipCall |= validatePushConstantSize(dev_data, offset, size, "vkCmdPushConstants()"); + skipCall |= validatePushConstantRange(dev_data, offset, size, "vkCmdPushConstants()"); + if (0 == stageFlags) { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_PUSH_CONSTANTS_ERROR, "DS", "vkCmdPushConstants() call has no stageFlags set."); + } + + // Check if push constant update is within any of the ranges specified in pipeline layout. + auto pipeline_layout_it = dev_data->pipelineLayoutMap.find(layout); + if (pipeline_layout_it == dev_data->pipelineLayoutMap.end()) { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_PUSH_CONSTANTS_ERROR, "DS", "vkCmdPushConstants() Pipeline Layout %" PRIu64 " not found.", + (uint64_t)layout); + } else { + auto ranges = pipeline_layout_it->second.pushConstantRanges; + bool contained_in_a_range = false; + for (uint32_t i = 0; i < ranges.size(); ++i) { + if ((offset >= ranges[i].offset) && + ((uint64_t)offset + (uint64_t)size <= (uint64_t)ranges[i].offset + (uint64_t)ranges[i].size)) { + contained_in_a_range = true; + break; + } + } + if (!contained_in_a_range) { + skipCall |= + log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_PUSH_CONSTANTS_ERROR, "DS", "vkCmdPushConstants() Push constant range [%d, %d) " + "not within any of the ranges in pipeline layout %" PRIu64 ".", + offset, offset + size, (uint64_t)layout); + } } - // TODO : Add warning if push constant update doesn't align with range lock.unlock(); if (!skipCall) dev_data->device_dispatch_table->CmdPushConstants(commandBuffer, layout, stageFlags, offset, size, pValues);