KhronosGroup / Vulkan-ValidationLayers

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

Reachable "should never happen" assertion error in cc_descriptor.cpp #8497

Open NBickford-NV opened 1 week ago

NBickford-NV commented 1 week ago

Environment:

Describe the Issue

Hi Vulkan Validation Layers team!

I managed to find a way to reach this assertion error at the end of CoreChecks::VerifySetLayoutCompatibility() in cc_descriptor.cpp:

    // No detailed check should succeed if the trivial check failed -- or the dictionary has failed somehow.
    bool compatible = true;
    assert(!compatible);
    return compatible;

This can happen if an app:

  1. creates a sampler;
  2. creates two identical descriptor set layouts, pipeline layouts, and descriptor sets, each using a combined image sampler using the sampler from step 1;
  3. then calls vkCmdBindDescriptorSets() with a pipeline layout from the first group and a descriptor set from the second:
    // Create a sampler. The repro doesn't depend on the parameters.
    VkSamplerCreateInfo samplerInfo{ .sType = VK_STRUCTURE_TYPE_SAMPLER_CREATE_INFO };
    VkSampler sampler;
    NVVK_CHECK(vkCreateSampler(device, &samplerInfo, nullptr, &sampler));
...
    const uint32_t kCopies = 2;
    std::array<VkDescriptorSetLayout, kCopies> descriptorSetLayouts;
    std::array<VkPipelineLayout, kCopies> pipelineLayouts;
    std::array<VkDescriptorSet, kCopies> descriptorSets;
    for (uint32_t i = 0; i < kCopies; i++) {
        // Create identical descriptor set layouts, pipeline layouts, and descriptor sets.
        // pipelineLayouts[i] and descriptorSets[i] use descriptorSetLayouts[i], and
        // descriptorSetLayouts[i] uses `sampler`.
        // See attached repro for full details.
    }
...
    // Bind pipeline layout copy 0 and descriptor set copy 1:
    vkCmdBindDescriptorSets(cmd,        // commandBuffer
        VK_PIPELINE_BIND_POINT_COMPUTE, // pipelineBindPoint
        pipelineLayouts[0],             // layout
        0,                              // firstSet
        1,                              // descriptorSetCount
        &descriptorSets[1],             // pDescriptorSets
        0,                              // dynamicOffsetCount
        nullptr);                       // pDynamicOffsets

I've attached a .zip archive for a repro case and build instructions in the Additional context section. This can happen in real-world code; I ran into it while fixing a separate validation layer issue in the nvpro-samples vk_compute_mipmaps sample.

Expected behavior

The DescriptorSetLayout::IsCompatible() check should return true if the detailed check passes. I think this should return true, since it satisfies the identically defined as case of https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358 (although there's an additional edge case there; I'll create a follow-up issue for it).

Valid Usage ID None

Additional context

Here's source code for a sample that reproduces this assertion error: vk-validation-pipeline-repro.zip

To build it, run

cmake -B build -S .
cmake --build build

It uses the nvpro_core Vulkan helper library; to make things more convenient, I've copied the source code of main.cpp below.

Source code for the repro case, omitting nvpro_core's context_vk, error_vk, and resourceallocator_vk helpers ```cpp #include #include #include "nvvk/context_vk.hpp" #include "nvvk/error_vk.hpp" #include "nvvk/resourceallocator_vk.hpp" int main() { //------------------------------------------------------------------------- // Boilerplate // This will produce a breakpoint on any validation layer error messages. nvprintSetBreakpoints(true); nvvk::ContextCreateInfo deviceInfo; // Defaults are OK nvvk::Context context; // Instance, device, physical device, queues, etc. if (!context.init(deviceInfo)) { LOGE("Could not create context!\n"); return EXIT_FAILURE; } // Name aliases so the code below doesn't depend so explicitly on the NVVK types: VkDevice device = context.m_device; VkPhysicalDevice physicalDevice = context.m_physicalDevice; // Memory allocator nvvk::ResourceAllocatorDedicated allocator(device, physicalDevice); // Command pool VkCommandPoolCreateInfo commandPoolInfo{ .sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO, .flags = VK_COMMAND_POOL_CREATE_TRANSIENT_BIT, .queueFamilyIndex = context.m_queueC.familyIndex }; VkCommandPool commandPool; NVVK_CHECK(vkCreateCommandPool(device, &commandPoolInfo, nullptr, &commandPool)); VkCommandBuffer cmd; VkCommandBufferAllocateInfo commandAllocInfo{ .sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO, .commandPool = commandPool, .level = VK_COMMAND_BUFFER_LEVEL_PRIMARY, .commandBufferCount = 1 }; NVVK_CHECK(vkAllocateCommandBuffers(device, &commandAllocInfo, &cmd)); //------------------------------------------------------------------------- // Main repro code // Create a sampler. The repro doesn't depend on the parameters. VkSamplerCreateInfo samplerInfo{ .sType = VK_STRUCTURE_TYPE_SAMPLER_CREATE_INFO }; VkSampler sampler; NVVK_CHECK(vkCreateSampler(device, &samplerInfo, nullptr, &sampler)); // Create two identical images, descriptor set layouts, descriptor pools, // pipeline layouts, and descriptor sets. const uint32_t kCopies = 2; std::array images; std::array imageViews; std::array descriptorSetLayouts; std::array descriptorPools; std::array pipelineLayouts; std::array descriptorSets; for (uint32_t i = 0; i < kCopies; i++) { // Create the image. The dimensions and layout don't matter for this repro. VkImageCreateInfo imageInfo{ .sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO, .imageType = VK_IMAGE_TYPE_2D, .format = VK_FORMAT_R8G8B8A8_UNORM, .extent = {1, 1, 1}, .mipLevels = 1, .arrayLayers = 1, .samples = VK_SAMPLE_COUNT_1_BIT, .tiling = VK_IMAGE_TILING_OPTIMAL, .usage = VK_IMAGE_USAGE_SAMPLED_BIT, .sharingMode = VK_SHARING_MODE_EXCLUSIVE, .initialLayout = VK_IMAGE_LAYOUT_UNDEFINED }; images[i] = allocator.createImage(imageInfo); // Create the image view. VkImageViewCreateInfo viewInfo{ .sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO, .image = images[i].image, .viewType = VK_IMAGE_VIEW_TYPE_2D, .format = VK_FORMAT_R8G8B8A8_UNORM, .subresourceRange = { .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, .levelCount = 1, .layerCount = 1 } }; NVVK_CHECK(vkCreateImageView(device, &viewInfo, nullptr, &imageViews[i])); // Create the descriptor set layout. VkDescriptorSetLayoutBinding binding{ .binding = 0, .descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, .descriptorCount = 1, .stageFlags = VK_SHADER_STAGE_COMPUTE_BIT, .pImmutableSamplers = &sampler }; VkDescriptorSetLayoutCreateInfo dslci{ .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO, .bindingCount = 1, .pBindings = &binding }; NVVK_CHECK(vkCreateDescriptorSetLayout(device, &dslci, nullptr, &descriptorSetLayouts[i])); // Create the descriptor pool. VkDescriptorPoolSize poolSize{ .type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, .descriptorCount = 1 }; VkDescriptorPoolCreateInfo dpci{ .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO, .maxSets = 1, .poolSizeCount = 1, .pPoolSizes = &poolSize }; NVVK_CHECK(vkCreateDescriptorPool(device, &dpci, nullptr, &descriptorPools[i])); // Create the descriptor set. VkDescriptorSetAllocateInfo dsai{ .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO, .descriptorPool = descriptorPools[i], .descriptorSetCount = 1, .pSetLayouts = &descriptorSetLayouts[i] }; NVVK_CHECK(vkAllocateDescriptorSets(device, &dsai, &descriptorSets[i])); // Not necessary for this repro, but for cleanliness, let's make sure // to initialize the descriptor set. VkDescriptorImageInfo imageDS{ .sampler = sampler, .imageView = imageViews[i], .imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL }; VkWriteDescriptorSet writeDS{ .sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET, .dstSet = descriptorSets[i], .descriptorCount = 1, .descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, .pImageInfo = &imageDS }; vkUpdateDescriptorSets(device, 1, &writeDS, 0, nullptr); // Create the pipeline layout. VkPipelineLayoutCreateInfo plci{ .sType = VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO, .setLayoutCount = 1, .pSetLayouts = &descriptorSetLayouts[i] }; NVVK_CHECK(vkCreatePipelineLayout(device, &plci, nullptr, &pipelineLayouts[i])); } // Start a command buffer VkCommandBufferBeginInfo commandBeginInfo{ VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO }; NVVK_CHECK(vkBeginCommandBuffer(cmd, &commandBeginInfo)); // Bind pipeline layout copy 0 and descriptor set copy 1: vkCmdBindDescriptorSets(cmd, // commandBuffer VK_PIPELINE_BIND_POINT_COMPUTE, // pipelineBindPoint pipelineLayouts[0], // layout 0, // firstSet 1, // descriptorSetCount &descriptorSets[1], // pDescriptorSets 0, // dynamicOffsetCount nullptr); // pDynamicOffsets // No need to actually submit the command buffer for this repro NVVK_CHECK(vkEndCommandBuffer(cmd)); //------------------------------------------------------------------------- // Cleanup NVVK_CHECK(vkDeviceWaitIdle(device)); vkFreeCommandBuffers(device, commandPool, 1, &cmd); for (uint32_t i = 0; i < kCopies; i++) { vkDestroyPipelineLayout(device, pipelineLayouts[i], nullptr); vkDestroyDescriptorPool(device, descriptorPools[i], nullptr); vkDestroyDescriptorSetLayout(device, descriptorSetLayouts[i], nullptr); vkDestroyImageView(device, imageViews[i], nullptr); allocator.destroy(images[i]); } vkDestroySampler(device, sampler, nullptr); vkDestroyCommandPool(device, commandPool, nullptr); allocator.deinit(); context.deinit(); } ```

Thanks!

spencer-lunarg commented 1 week ago

I grabbed the repro case and can see the assert! I am currently slight backlogged, but will hopefully have time next week to look at this

NBickford-NV commented 1 week ago

Thank you Spencer! Much appreciated.

spencer-lunarg commented 1 week ago

the reproduce case was easy to spot this and have tests ready (just need to fix it now)

The issue is around the fact we use vvl::DescriptorSetLayoutDef::hash() to create a DescriptorSetLayout hash as we don't want thousands of duplicates

the pImmutableSamplers are the issue as it is a pointer to an array of VkSampler handles (which might be different, but identically defined which is considered the same)

need to rework things to account for this

spencer-lunarg commented 1 week ago

ok, so the hash_utils::Dictionary logic is too dense for me to figure out, going to get some help on it, until then, leaving these tests here for me to copy and paste later


TEST_F(PositiveDescriptors, DuplicateLayoutSameSampler) {
    TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8497");
    RETURN_IF_SKIP(Init());
    vkt::Sampler sampler(*m_device, SafeSaneSamplerCreateInfo());

    OneOffDescriptorSet ds_0(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_COMPUTE_BIT, &sampler.handle()}});
    const vkt::PipelineLayout pipeline_layout_0(*m_device, {&ds_0.layout_});

    OneOffDescriptorSet ds_1(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_COMPUTE_BIT, &sampler.handle()}});

    m_commandBuffer->begin();
    vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout_0.handle(), 0, 1,
                              &ds_1.set_, 0, nullptr);
    m_commandBuffer->end();
}

TEST_F(PositiveDescriptors, DuplicateLayoutDuplicateSampler) {
    TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8497");
    RETURN_IF_SKIP(Init());
    vkt::Sampler sampler_0(*m_device, SafeSaneSamplerCreateInfo());
    vkt::Sampler sampler_1(*m_device, SafeSaneSamplerCreateInfo());

    OneOffDescriptorSet ds_0(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_COMPUTE_BIT, &sampler_0.handle()}});
    const vkt::PipelineLayout pipeline_layout_0(*m_device, {&ds_0.layout_});

    OneOffDescriptorSet ds_1(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_COMPUTE_BIT, &sampler_1.handle()}});

    m_commandBuffer->begin();
    vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout_0.handle(), 0, 1,
                              &ds_1.set_, 0, nullptr);
    m_commandBuffer->end();
}

TEST_F(PositiveDescriptors, DuplicateLayoutSameSamplerArray) {
    TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8497");
    RETURN_IF_SKIP(Init());
    vkt::Sampler sampler(*m_device, SafeSaneSamplerCreateInfo());
    VkSampler sampler_array[3] = {sampler.handle(), sampler.handle(), sampler.handle()};

    OneOffDescriptorSet ds_0(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 3, VK_SHADER_STAGE_COMPUTE_BIT, sampler_array}});
    const vkt::PipelineLayout pipeline_layout_0(*m_device, {&ds_0.layout_});

    OneOffDescriptorSet ds_1(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 3, VK_SHADER_STAGE_COMPUTE_BIT, sampler_array}});

    m_commandBuffer->begin();
    vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout_0.handle(), 0, 1,
                              &ds_1.set_, 0, nullptr);
    m_commandBuffer->end();
}

TEST_F(PositiveDescriptors, DuplicateLayoutDuplicateSamplerArray) {
    TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8497");
    RETURN_IF_SKIP(Init());
    vkt::Sampler sampler_0(*m_device, SafeSaneSamplerCreateInfo());
    vkt::Sampler sampler_1(*m_device, SafeSaneSamplerCreateInfo());
    VkSampler sampler_array_0[3] = {sampler_0.handle(), sampler_0.handle(), sampler_0.handle()};
    VkSampler sampler_array_1[3] = {sampler_1.handle(), sampler_1.handle(), sampler_1.handle()};

    OneOffDescriptorSet ds_0(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 3, VK_SHADER_STAGE_COMPUTE_BIT, sampler_array_0}});
    const vkt::PipelineLayout pipeline_layout_0(*m_device, {&ds_0.layout_});

    OneOffDescriptorSet ds_1(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 3, VK_SHADER_STAGE_COMPUTE_BIT, sampler_array_1}});

    m_commandBuffer->begin();
    vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout_0.handle(), 0, 1,
                              &ds_1.set_, 0, nullptr);
    m_commandBuffer->end();
}
TEST_F(NegativeDescriptors, DuplicateLayoutDifferentSampler) {
    TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8497");
    RETURN_IF_SKIP(Init());
    auto sampler_ci = SafeSaneSamplerCreateInfo();
    vkt::Sampler sampler_0(*m_device, sampler_ci);
    sampler_ci.maxLod = 8.0;
    vkt::Sampler sampler_1(*m_device, sampler_ci);

    OneOffDescriptorSet ds_0(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_COMPUTE_BIT, &sampler_0.handle()}});
    const vkt::PipelineLayout pipeline_layout_0(*m_device, {&ds_0.layout_});

    OneOffDescriptorSet ds_1(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_COMPUTE_BIT, &sampler_1.handle()}});

    m_commandBuffer->begin();
    vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout_0.handle(), 0, 1,
                              &ds_1.set_, 0, nullptr);
    m_commandBuffer->end();
}

TEST_F(NegativeDescriptors, DuplicateLayoutDifferentSamplerArray) {
    TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8497");
    RETURN_IF_SKIP(Init());
    auto sampler_ci = SafeSaneSamplerCreateInfo();
    vkt::Sampler sampler_0(*m_device, sampler_ci);
    sampler_ci.maxLod = 8.0;
    vkt::Sampler sampler_1(*m_device, sampler_ci);
    VkSampler sampler_array_0[3] = {sampler_0.handle(), sampler_0.handle(), sampler_0.handle()};
    VkSampler sampler_array_1[3] = {sampler_0.handle(), sampler_1.handle(), sampler_0.handle()};

    OneOffDescriptorSet ds_0(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 3, VK_SHADER_STAGE_COMPUTE_BIT, sampler_array_0}});
    const vkt::PipelineLayout pipeline_layout_0(*m_device, {&ds_0.layout_});

    OneOffDescriptorSet ds_1(m_device,
                             {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 3, VK_SHADER_STAGE_COMPUTE_BIT, sampler_array_1}});

    m_commandBuffer->begin();
    vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout_0.handle(), 0, 1,
                              &ds_1.set_, 0, nullptr);
    m_commandBuffer->end();
}