From e13b0d1d0c63c2e30811d455d3b73d66445ba674 Mon Sep 17 00:00:00 2001 From: John Zulauf Date: Tue, 23 Jan 2018 11:27:35 -0700 Subject: [PATCH] layers: Add push descriptor set layout create VUID Add VUID checks to vkCreateDescriptorSetLayout for push descriptor sets. Additional checks include: VALIDATION_ERROR_05000230 VkDescriptorSetLayoutCreateInfo-flags-00280 VALIDATION_ERROR_05000232 VkDescriptorSetLayoutCreateInfo-flags-00281 Also added check for use of VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR when the required extension VK_KHR_push_descriptor is not enabled. Updated CreateDescriptorSetBindingWithIgnoredSamplers test, which violated the above check. Change-Id: Ie009019bbb7859553df92473796a1a929a9464f7 --- layers/core_validation.cpp | 18 +++++++++- layers/descriptor_sets.cpp | 43 +++++++++++++++++++++-- layers/descriptor_sets.h | 2 +- layers/vk_validation_error_database.txt | 4 +-- tests/layer_validation_tests.cpp | 62 ++++++++++++++++++++++----------- 5 files changed, 101 insertions(+), 28 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index b747add..c36d647 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -166,6 +166,11 @@ struct layer_data { PHYS_DEV_PROPERTIES_NODE phys_dev_properties = {}; VkPhysicalDeviceMemoryProperties phys_dev_mem_props = {}; VkPhysicalDeviceProperties phys_dev_props = {}; + // Device extension properties -- storing properties gathered from VkPhysicalDeviceProperties2KHR::pNext chain + struct DeviceExtensionProperties { + uint32_t max_push_descriptors; // from VkPhysicalDevicePushDescriptorPropertiesKHR::maxPushDescriptors + }; + DeviceExtensionProperties phys_dev_ext_props = {}; bool external_sync_warning = false; }; @@ -2173,6 +2178,15 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice gpu, const VkDevice // Store physical device properties and physical device mem limits into device layer_data structs instance_data->dispatch_table.GetPhysicalDeviceMemoryProperties(gpu, &device_data->phys_dev_mem_props); instance_data->dispatch_table.GetPhysicalDeviceProperties(gpu, &device_data->phys_dev_props); + + if (device_data->extensions.vk_khr_push_descriptor) { + // Get the needed push_descriptor limits + auto push_descriptor_prop = lvl_init_struct(); + auto prop2 = lvl_init_struct(&push_descriptor_prop); + instance_data->dispatch_table.GetPhysicalDeviceProperties2KHR(gpu, &prop2); + device_data->phys_dev_ext_props.max_push_descriptors = push_descriptor_prop.maxPushDescriptors; + } + lock.unlock(); ValidateLayerOrdering(*pCreateInfo); @@ -4704,7 +4718,9 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateSampler(VkDevice device, const VkSamplerCre static bool PreCallValidateCreateDescriptorSetLayout(layer_data *dev_data, const VkDescriptorSetLayoutCreateInfo *create_info) { if (dev_data->instance_data->disabled.create_descriptor_set_layout) return false; - return cvdescriptorset::DescriptorSetLayout::ValidateCreateInfo(dev_data->report_data, create_info); + return cvdescriptorset::DescriptorSetLayout::ValidateCreateInfo(dev_data->report_data, create_info, + dev_data->extensions.vk_khr_push_descriptor, + dev_data->phys_dev_ext_props.max_push_descriptors); } static void PostCallRecordCreateDescriptorSetLayout(layer_data *dev_data, const VkDescriptorSetLayoutCreateInfo *create_info, diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 47ca054..1adc81d 100644 --- a/layers/descriptor_sets.cpp +++ b/layers/descriptor_sets.cpp @@ -107,17 +107,54 @@ cvdescriptorset::DescriptorSetLayout::DescriptorSetLayout(const VkDescriptorSetL } // Validate descriptor set layout create info -bool cvdescriptorset::DescriptorSetLayout::ValidateCreateInfo(debug_report_data *report_data, - const VkDescriptorSetLayoutCreateInfo *create_info) { +bool cvdescriptorset::DescriptorSetLayout::ValidateCreateInfo(const debug_report_data *report_data, + const VkDescriptorSetLayoutCreateInfo *create_info, + const bool push_descriptor_ext, const uint32_t max_push_descriptors) { bool skip = false; std::unordered_set bindings; + uint64_t total_descriptors = 0; + + const bool push_descriptor_set = create_info->flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR; + if (push_descriptor_set && !push_descriptor_ext) { + skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, + DRAWSTATE_EXTENSION_NOT_ENABLED, "DS", + "Attemped to use %s in %s but its required extension %s has not been enabled.\n", + "VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR", "VkDescriptorSetLayoutCreateInfo::flags", + VK_KHR_PUSH_DESCRIPTOR_EXTENSION_NAME); + } + + auto valid_type = [push_descriptor_set](const VkDescriptorType type) { + return !push_descriptor_set || + ((type != VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC) && (type != VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC)); + }; + for (uint32_t i = 0; i < create_info->bindingCount; ++i) { - if (!bindings.insert(create_info->pBindings[i].binding).second) { + const auto &binding_info = create_info->pBindings[i]; + if (!bindings.insert(binding_info.binding).second) { skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, VALIDATION_ERROR_0500022e, "DS", "duplicated binding number in VkDescriptorSetLayoutBinding. %s", validation_error_map[VALIDATION_ERROR_0500022e]); } + if (!valid_type(binding_info.descriptorType)) { + skip |= + log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, + VALIDATION_ERROR_05000230, "DS", + "invalid type %s ,for push descriptors in VkDescriptorSetLayoutBinding entry %" PRIu32 ". %s", + string_VkDescriptorType(binding_info.descriptorType), i, validation_error_map[VALIDATION_ERROR_05000230]); + } + total_descriptors += binding_info.descriptorCount; + } + + if ((push_descriptor_set) && (total_descriptors > max_push_descriptors)) { + const char *undefined = push_descriptor_ext ? "" : " -- undefined"; + skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, + VALIDATION_ERROR_05000232, "DS", + "for push descriptor, total descriptor count in layout (%" PRIu64 + ") must not be greater than VkPhysicalDevicePushDescriptorPropertiesKHR::maxPushDescriptors (%" PRIu32 + "%s). %s", + total_descriptors, max_push_descriptors, undefined, validation_error_map[VALIDATION_ERROR_05000232]); } + return skip; } diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h index d933164..016230a 100644 --- a/layers/descriptor_sets.h +++ b/layers/descriptor_sets.h @@ -86,7 +86,7 @@ class DescriptorSetLayout { // Constructors and destructor DescriptorSetLayout(const VkDescriptorSetLayoutCreateInfo *p_create_info, const VkDescriptorSetLayout layout); // Validate create info - should be called prior to creation - static bool ValidateCreateInfo(debug_report_data *, const VkDescriptorSetLayoutCreateInfo *); + static bool ValidateCreateInfo(const debug_report_data *, const VkDescriptorSetLayoutCreateInfo *, const bool, const uint32_t); // Straightforward Get functions VkDescriptorSetLayout GetDescriptorSetLayout() const { return layout_; }; bool IsDestroyed() const { return layout_destroyed_; } diff --git a/layers/vk_validation_error_database.txt b/layers/vk_validation_error_database.txt index 4143dd9..b95f590 100644 --- a/layers/vk_validation_error_database.txt +++ b/layers/vk_validation_error_database.txt @@ -371,8 +371,8 @@ VALIDATION_ERROR_04e00236~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkD VALIDATION_ERROR_04e00bcc~^~N~^~None~^~VkDescriptorSetLayoutBinding~^~VUID-VkDescriptorSetLayoutBinding-descriptorType-01510~^~core~^~The spec valid usage text states 'If descriptorType is VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT and descriptorCount is not 0, then stageFlags must be 0 or VK_SHADER_STAGE_FRAGMENT_BIT' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-VkDescriptorSetLayoutBinding-descriptorType-01510)~^~ VALIDATION_ERROR_04e04e01~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutBinding-descriptorType-parameter~^~core~^~The spec valid usage text states 'descriptorType must be a valid VkDescriptorType value' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-VkDescriptorSetLayoutBinding-descriptorType-parameter)~^~implicit VALIDATION_ERROR_0500022e~^~Y~^~DuplicateDescriptorBinding~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-binding-00279~^~core~^~The spec valid usage text states 'The VkDescriptorSetLayoutBinding::binding members of the elements of the pBindings array must each have different values.' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-binding-00279)~^~ -VALIDATION_ERROR_05000230~^~N~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-flags-00280~^~(VK_KHR_push_descriptor)~^~The spec valid usage text states 'If flags contains VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR, then all elements of pBindings must not have a descriptorType of VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-flags-00280)~^~ -VALIDATION_ERROR_05000232~^~N~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-flags-00281~^~(VK_KHR_push_descriptor)~^~The spec valid usage text states 'If flags contains VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR, then the total number of elements of all bindings must be less than or equal to VkPhysicalDevicePushDescriptorPropertiesKHR::maxPushDescriptors' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-flags-00281)~^~ +VALIDATION_ERROR_05000230~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-flags-00280~^~(VK_KHR_push_descriptor)~^~The spec valid usage text states 'If flags contains VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR, then all elements of pBindings must not have a descriptorType of VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-flags-00280)~^~ +VALIDATION_ERROR_05000232~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-flags-00281~^~(VK_KHR_push_descriptor)~^~The spec valid usage text states 'If flags contains VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR, then the total number of elements of all bindings must be less than or equal to VkPhysicalDevicePushDescriptorPropertiesKHR::maxPushDescriptors' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-flags-00281)~^~ VALIDATION_ERROR_05009001~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-flags-parameter~^~core~^~The spec valid usage text states 'flags must be a valid combination of VkDescriptorSetLayoutCreateFlagBits values' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-flags-parameter)~^~implicit VALIDATION_ERROR_0500fc01~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-pBindings-parameter~^~core~^~The spec valid usage text states 'If bindingCount is not 0, pBindings must be a valid pointer to an array of bindingCount valid VkDescriptorSetLayoutBinding structures' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-pBindings-parameter)~^~implicit VALIDATION_ERROR_0501c40d~^~Y~^~Unknown~^~vkCreateDescriptorSetLayout~^~VUID-VkDescriptorSetLayoutCreateInfo-pNext-pNext~^~core~^~The spec valid usage text states 'pNext must be NULL' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-pNext-pNext)~^~implicit, TBD in parameter validation layer. diff --git a/tests/layer_validation_tests.cpp b/tests/layer_validation_tests.cpp index b924f31..5c6aafd 100644 --- a/tests/layer_validation_tests.cpp +++ b/tests/layer_validation_tests.cpp @@ -25355,8 +25355,26 @@ TEST_F(VkPositiveLayerTest, CreateComputePipelineCombinedImageSamplerConsumedAsB TEST_F(VkPositiveLayerTest, CreateDescriptorSetBindingWithIgnoredSamplers) { TEST_DESCRIPTION("Test that layers conditionally do ignore the pImmutableSamplers on vkCreateDescriptorSetLayout"); - ASSERT_NO_FATAL_FAILURE(Init()); + bool prop2_found = false; + if (InstanceExtensionSupported(VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME)) { + m_instance_extension_names.push_back(VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME); + prop2_found = true; + } else { + printf(" %s Extension not supported, skipping push descriptor sub-tests\n", + VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME); + } + ASSERT_NO_FATAL_FAILURE(InitFramework(myDbgFunc, m_errorMonitor)); + bool push_descriptor_found = false; + if (prop2_found && DeviceExtensionSupported(gpu(), nullptr, VK_KHR_PUSH_DESCRIPTOR_EXTENSION_NAME)) { + m_device_extension_names.push_back(VK_KHR_PUSH_DESCRIPTOR_EXTENSION_NAME); + push_descriptor_found = true; + } else { + printf(" %s Extension not supported, skipping push descriptor sub-tests\n", + VK_KHR_PUSH_DESCRIPTOR_EXTENSION_NAME); + } + + ASSERT_NO_FATAL_FAILURE(InitState()); const uint64_t fake_address_64 = 0xCDCDCDCDCDCDCDCD; const uint64_t fake_address_32 = 0xCDCDCDCD; const void *fake_pointer = @@ -25386,27 +25404,29 @@ TEST_F(VkPositiveLayerTest, CreateDescriptorSetBindingWithIgnoredSamplers) { } m_errorMonitor->VerifyNotFound(); - // push descriptors - m_errorMonitor->ExpectSuccess(); - { - const VkDescriptorSetLayoutBinding non_sampler_bindings[] = { - {0, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - {1, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - {2, VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - {3, VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - {4, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - {5, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - {6, VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, - }; - const VkDescriptorSetLayoutCreateInfo dslci = {VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO, nullptr, - VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR, - static_cast(size(non_sampler_bindings)), non_sampler_bindings}; - VkDescriptorSetLayout dsl; - const VkResult err = vkCreateDescriptorSetLayout(m_device->device(), &dslci, nullptr, &dsl); - ASSERT_VK_SUCCESS(err); - vkDestroyDescriptorSetLayout(m_device->device(), dsl, nullptr); + if (push_descriptor_found) { + // push descriptors + m_errorMonitor->ExpectSuccess(); + { + const VkDescriptorSetLayoutBinding non_sampler_bindings[] = { + {0, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + {1, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + {2, VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + {3, VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + {4, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + {5, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + {6, VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT, 1, VK_SHADER_STAGE_FRAGMENT_BIT, hopefully_undereferencable_pointer}, + }; + const VkDescriptorSetLayoutCreateInfo dslci = {VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO, nullptr, + VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR, + static_cast(size(non_sampler_bindings)), non_sampler_bindings}; + VkDescriptorSetLayout dsl; + const VkResult err = vkCreateDescriptorSetLayout(m_device->device(), &dslci, nullptr, &dsl); + ASSERT_VK_SUCCESS(err); + vkDestroyDescriptorSetLayout(m_device->device(), dsl, nullptr); + } + m_errorMonitor->VerifyNotFound(); } - m_errorMonitor->VerifyNotFound(); } TEST_F(VkPositiveLayerTest, Maintenance1Tests) { -- 2.7.4