Issue with unwrapping pNext in shader stages of graphics pipeline create info

[ziga-lunarg]

Issue is from #12107 (comment)

Uncommenting // stages[0].pNext = &mapping_info; in PositiveDescriptorHeap,YcbcrImage will lead to a crash when handle wrapping is enabled. I think the issue comes from VUL not deep copying VkDescriptorSetAndBindingMappingEXT::sourceData

[spencer-lunarg]

so the issue is the following

case VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_INFO: {
    auto* safe_struct = reinterpret_cast<vku::safe_VkSamplerYcbcrConversionInfo*>(cur_pnext);

    if (safe_struct->conversion) {
        safe_struct->conversion = Unwrap(safe_struct->conversion);
    }

is being called twice in the same dispatch call, such that the 2nd time we are doing a double unwrap and returning a null value

the issue is

UnwrapPnextChainHandles(local_pCreateInfos[idx0].pStages[idx1].pNext);

added is not making a local copy local_ of the struct such that they are sharing the handle... think this normally gets done correctly if this was a generated version... getting a fix

[spencer-lunarg]

Left open, we need to write a ShaderObject version as well now

[spencer-lunarg]

@ziga-lunarg no unions are not the issue, the problem is inside a single function (createPipeline/createShader) we see the VkSamplerYcbcrConversionInfo multiple times

The first issue was we were going

// loop 1
VkSamplerYcbcrConversionInfo::conversion = 0x0000d00d;
// unwrap, so now
VkSamplerYcbcrConversionInfo::conversion = 0x12341234; // real handle

// loop 2
VkSamplerYcbcrConversionInfo::conversion = 0x12341234; // real handle
// unwrap, so now
VkSamplerYcbcrConversionInfo::conversion = 0x00000000; // driver crashes with this

The issue with the unordered_set hack shader object showed is it would go

// loop 1
VkSamplerYcbcrConversionInfo::conversion = 0x0000d00d;
// [0x0000d00d] not in set... unwrap, so now
VkSamplerYcbcrConversionInfo::conversion = 0x12341234; // real handle

// loop 2
// (because they share the same pointer to the VkSamplerYcbcrConversionInfo struct)
VkSamplerYcbcrConversionInfo::conversion = 0x12341234; // real handle
// [0x12341234] not in set... unwrap, so now
VkSamplerYcbcrConversionInfo::conversion = 0x00000000; // driver crashes with this

Then I added a 3rd test where I have 2 different copies of VkSamplerYcbcrConversionInfo

... the "real" solution is making safe struct copies of VkSamplerYcbcrConversionInfo, but that would require having them allocated up front when we allocate the entire CreateInfo struct

[spencer-lunarg]

Going to get the "hack" fix in, I will need to run some script with VulkanObject to see if there are any other structs, that have handles, that can be provided multiple times in a given function call like this

[ziga-lunarg]

I didn't dig too dip into this one, but I assumed that using local_pCreateInfos would make sure we didn't have issues between multiple calls

[spencer-lunarg]

... ya, note sure if local_pCreateInfos is just having a hard time making "correct" copies... this is a very deep pNext struct

[spencer-lunarg]

... ok, so this is an Union problem 🤬, the issue is safe_VkDescriptorSetAndBindingMappingEXT doesn't do a deep copy when creating local_pCreateInfos...

sourceData = copy_src.sourceData;

This will be annoying to root out the logic we have to find the pEmeddedSamplers and make a copy of that