Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
NBickford-NV opened this issue Sep 5, 2024 · 4 comments
Open
Assignees
Labels
Bug Something isn't working

Comments

@NBickford-NV
Copy link

NBickford-NV commented Sep 5, 2024

Environment:

  • OS: Windows 10 x86_64 (my repro should work on Windows 11 and Linux as well)
  • GPU and driver version: NVIDIA RTX 3090, 552.88
  • SDK or header version if building from repo: main (commit af7b0a3, 2024-09-03)
  • Options enabled (synchronization, best practices, etc.): None

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
#include <array>
#include <stdlib.h>

#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<nvvk::Image, kCopies> images;
    std::array<VkImageView, kCopies> imageViews;
    std::array<VkDescriptorSetLayout, kCopies> descriptorSetLayouts;
    std::array<VkDescriptorPool, kCopies> descriptorPools;
    std::array<VkPipelineLayout, kCopies> pipelineLayouts;
    std::array<VkDescriptorSet, kCopies> 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
Copy link
Contributor

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
Copy link
Author

Thank you Spencer! Much appreciated.

@spencer-lunarg
Copy link
Contributor

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
Copy link
Contributor

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();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants