Skip to content

Commit

Permalink
Fix crash when descriptor update template uses unreferenced layout
Browse files Browse the repository at this point in the history 
It is possible for the VkDescriptorUpdateTemplateCreateInfo to have its
descriptorSetLayout or pipelineLayout be the only reference to the
corresponding object, as vkCmdPushDescriptorSetWithTemplateKHR and
vkUpdateDescriptorSetWithTemplate only require that the actual layout
matches/is compatible with the one used by the template. Before, there
was no dependency from the update template to the layout, so if it was
the sole use of a layout, the layout would not be saved in the capture,
resulting in a crash during playback.

This change also modifies the vk_parameter_zoo to test this case. With
those modifications, the change to vk_descriptor_funcs is required to
prevent the crash. Note that on my machine, I needed to comment out
other parts of vk_parameter_zoo, or else it would crash (even when
not run under renderdoc) due to driver issues.

For reference:

https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdPushDescriptorSetWithTemplateKHR.html#VUID-vkCmdPushDescriptorSetWithTemplateKHR-layout-07993
https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkUpdateDescriptorSetWithTemplate.html#VUID-vkUpdateDescriptorSetWithTemplate-pData-01685
https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDescriptorUpdateTemplateCreateInfoKHR.html#_members
  • Loading branch information
@w-pearson
w-pearson committed Nov 8, 2023
1 parent 0ff1c66 commit 4b340db
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
5 changes: 5 additions & 0 deletions renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,11 @@ VkResult WrappedVulkan::vkCreateDescriptorUpdateTemplate(
VkResourceRecord *record = GetResourceManager()->AddResourceRecord(*pDescriptorUpdateTemplate);
record->AddChunk(chunk);

if(unwrapped.templateType == VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_PUSH_DESCRIPTORS_KHR)
record->AddParent(GetRecord(pCreateInfo->pipelineLayout));
else if(unwrapped.templateType == VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_DESCRIPTOR_SET)
record->AddParent(GetRecord(pCreateInfo->descriptorSetLayout));

record->descTemplateInfo = new DescUpdateTemplate();
record->descTemplateInfo->Init(GetResourceManager(), m_CreationInfo, pCreateInfo);
}
Expand Down
36 changes: 20 additions & 16 deletions util/test/demos/vk/vk_parameter_zoo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,19 +471,21 @@ void main()
{10, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 0, VK_SHADER_STAGE_VERTEX_BIT},
}));

VkDescriptorSetLayout refsetlayout =
createDescriptorSetLayout(vkh::DescriptorSetLayoutCreateInfo({
{0, VK_DESCRIPTOR_TYPE_SAMPLER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{1, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{2, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT},
{3, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT},
{4, VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{5, VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{6, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{7, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{8, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, 1, VK_SHADER_STAGE_VERTEX_BIT},
{9, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC, 1, VK_SHADER_STAGE_VERTEX_BIT},
}));
VkDescriptorSetLayoutCreateInfo refsetlayoutcreateinfo = vkh::DescriptorSetLayoutCreateInfo({
{0, VK_DESCRIPTOR_TYPE_SAMPLER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{1, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{2, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT},
{3, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT},
{4, VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{5, VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{6, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{7, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_VERTEX_BIT},
{8, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, 1, VK_SHADER_STAGE_VERTEX_BIT},
{9, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC, 1, VK_SHADER_STAGE_VERTEX_BIT},
});

VkDescriptorSetLayout refsetlayout = createDescriptorSetLayout(&refsetlayoutcreateinfo);
VkDescriptorSetLayout refsetlayout_copy = createDescriptorSetLayout(&refsetlayoutcreateinfo);

VkSampler invalidSampler = (VkSampler)0x1234;
VkSampler validSampler = createSampler(vkh::SamplerCreateInfo(VK_FILTER_LINEAR));
Expand All @@ -497,7 +499,7 @@ void main()
{99, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_VERTEX_BIT, &invalidSampler},
}));

VkPipelineLayout layout, reflayout;
VkPipelineLayout layout, layout_copy, reflayout;

if(KHR_push_descriptor)
{
Expand All @@ -510,6 +512,7 @@ void main()
VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR));

layout = createPipelineLayout(vkh::PipelineLayoutCreateInfo({setlayout, pushlayout}));
layout_copy = createPipelineLayout(vkh::PipelineLayoutCreateInfo({setlayout, pushlayout}));

VkDescriptorSetLayout refpushlayout =
createDescriptorSetLayout(vkh::DescriptorSetLayoutCreateInfo(
Expand All @@ -535,6 +538,7 @@ void main()
else
{
layout = createPipelineLayout(vkh::PipelineLayoutCreateInfo({setlayout}));
layout_copy = createPipelineLayout(vkh::PipelineLayoutCreateInfo({setlayout}));

if(KHR_descriptor_update_template)
reflayout = createPipelineLayout(vkh::PipelineLayoutCreateInfo({refsetlayout, refsetlayout}));
Expand Down Expand Up @@ -1127,7 +1131,7 @@ void main()
sizeof(data)},
};

createInfo.descriptorSetLayout = refsetlayout;
createInfo.descriptorSetLayout = refsetlayout_copy;
createInfo.descriptorUpdateEntryCount = (uint32_t)entries.size();
createInfo.pDescriptorUpdateEntries = entries.data();

Expand Down Expand Up @@ -1156,7 +1160,7 @@ void main()
createInfo.templateType = VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_PUSH_DESCRIPTORS_KHR;
createInfo.descriptorSetLayout = (VkDescriptorSetLayout)0x1234;
createInfo.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS;
createInfo.pipelineLayout = layout;
createInfo.pipelineLayout = layout_copy;
createInfo.set = 1;
vkCreateDescriptorUpdateTemplateKHR(device, &createInfo, NULL, &pushtempl);

Expand Down

0 comments on commit 4b340db

Please sign in to comment.