KhronosGroup / Vulkan-ValidationLayers

Vulkan Validation Layers (VVL)
https://vulkan.lunarg.com/doc/sdk/latest/linux/khronos_validation_layer.html
Other
748 stars 400 forks source link

`vkUpdateDescriptorSetWithTemplate` does not validate buffer info, while `vkUpdateDescriptorSets` does #8251

Open jbit opened 2 months ago

jbit commented 2 months ago

Environment:

Describe the Issue

Passing a VkDescriptorBufferInfo with a badly aligned offset into vkUpdateDescriptorSets generates the following expected validation error:

VALIDATION: Validation Error: [ VUID-VkWriteDescriptorSet-descriptorType-00328 ] | MessageID = 0xea08144e | vkUpdateDescriptorSets(): pDescriptorWrites[0].pBufferInfo[0].offset (1) must be a multiple of device limit minStorageBufferOffsetAlignment 64 when descriptor type is VK_DESCRIPTOR_TYPE_STORAGE_BUFFER. The Vulkan spec states: If descriptorType is VK_DESCRIPTOR_TYPE_STORAGE_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC, the offset member of each element of pBufferInfo must be a multiple of VkPhysicalDeviceLimits::minStorageBufferOffsetAlignment

However passing the same update into vkUpdateDescriptorSetWithTemplate does not generate any validation error.

Expected behaviour

I would expect that vkUpdateDescriptorSetWithTemplate and vkUpdateDescriptorSets perform similar validations where possible.

Additional context

Some systems have minStorageBufferOffsetAlignment set to 1 and thus do not require alignment. The above validation error will never trigger on such systems, so this should be tested on a system with minStorageBufferOffsetAlignment > 1.

This may be related to: https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7472

I've attached a test case which can run inside the VVL gtest suite. It demonstrates the validation error being thrown from vkUpdateDescriptorSets and then nothing from vkUpdateDescriptorSetWithTemplate

gtest test code ```C++ class GithubIssue : public VkLayerTest {}; TEST_F(GithubIssue, UnalignedOffset) { SetTargetApiVersion(VK_API_VERSION_1_2); RETURN_IF_SKIP(Init()); // This test tries to build a badly aligned buffer so check our device actually requires alignment VkDeviceSize offset_alignment = m_device->phy().limits_.minStorageBufferOffsetAlignment; if (offset_alignment <= 1) { GTEST_SKIP() << "minStorageBufferOffsetAlignment is " << offset_alignment << " but must be greater than 1"; } // Build a buffer with a bad offset: VkBufferCreateInfo buffer_create_info = vku::InitStructHelper(); buffer_create_info.usage = VK_BUFFER_USAGE_STORAGE_BUFFER_BIT; buffer_create_info.size = 4096; vkt::Buffer buffer(*m_device, buffer_create_info, vkt::no_mem); ASSERT_NE(buffer.handle(), VK_NULL_HANDLE); VkMemoryRequirements memory_requirements = {}; vk::GetBufferMemoryRequirements(device(), buffer.handle(), &memory_requirements); VkMemoryAllocateInfo memory_allocate_info = vku::InitStructHelper(); memory_allocate_info.allocationSize = memory_requirements.size; ASSERT_TRUE(m_device->phy().set_memory_type(memory_requirements.memoryTypeBits, &memory_allocate_info, 0)); vkt::DeviceMemory memory(*m_device, memory_allocate_info); ASSERT_NE(memory.handle(), VK_NULL_HANDLE); ASSERT_EQ(vk::BindBufferMemory(device(), buffer.handle(), memory.handle(), 0), VK_SUCCESS); VkDescriptorBufferInfo buffer_info = {}; buffer_info.buffer = buffer.handle(); buffer_info.offset = 1; buffer_info.range = 64; OneOffDescriptorSet descriptor_set(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}}); // Test 1, doesn't use templates and generates a validation error as expected { VkWriteDescriptorSet write_descriptor_set = vku::InitStructHelper(); write_descriptor_set.dstBinding = 0; write_descriptor_set.descriptorCount = 1; write_descriptor_set.pTexelBufferView = nullptr; write_descriptor_set.pBufferInfo = &buffer_info; write_descriptor_set.pImageInfo = nullptr; write_descriptor_set.descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER; write_descriptor_set.dstSet = descriptor_set.set_; // Emits error: // offset (1) must be a multiple of device limit minStorageBufferOffsetAlignment 16 when descriptor type is // VK_DESCRIPTOR_TYPE_STORAGE_BUFFER. Monitor().SetDesiredError("VUID-VkWriteDescriptorSet-descriptorType-00328"); vk::UpdateDescriptorSets(device(), 1, &write_descriptor_set, 0, NULL); Monitor().VerifyFound(); } // Test 2, uses template and does not generate a validation error { VkDescriptorUpdateTemplateEntry template_entry = {}; template_entry.dstBinding = 0; template_entry.dstArrayElement = 0; template_entry.descriptorCount = 1; template_entry.descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER; template_entry.offset = 0; template_entry.stride = sizeof(VkDescriptorBufferInfo); VkDescriptorUpdateTemplateCreateInfo template_create_info = vku::InitStructHelper(); template_create_info.descriptorUpdateEntryCount = 1; template_create_info.pDescriptorUpdateEntries = &template_entry; template_create_info.templateType = VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_DESCRIPTOR_SET; template_create_info.descriptorSetLayout = descriptor_set.layout_.handle(); VkDescriptorUpdateTemplate template_ = VK_NULL_HANDLE; vk::CreateDescriptorUpdateTemplate(device(), &template_create_info, nullptr, &template_); ASSERT_NE(template_, VK_NULL_HANDLE); // Does not emit an error, despite the buffer descriptor being invalid Monitor().SetDesiredError("VUID-VkWriteDescriptorSet-descriptorType-00328"); vk::UpdateDescriptorSetWithTemplate(device(), descriptor_set.set_, template_, &buffer_info); Monitor().VerifyFound(); vk::DestroyDescriptorUpdateTemplate(device(), template_, nullptr); } } ```
spencer-lunarg commented 2 months ago

Thanks for the issue (and the tests!!)

Yes, it seems we are not properly checking descriptor with VK_KHR_descriptor_update_template

unfortunately, won't have time personally to look into this for awhile :disappointed: