terminator_DestroySurfaceKHR aborts in assert when ICD does not support VK_KHR_surface

[TomCoo]

For an ICD without support for VK_KHR_surface the surface list pointer will be null. If vkDestroySurface is called terminator_DestroySurfaceKHR will abort in this assert:
assert((VkSurfaceKHR)(uintptr_t)NULL == icd_term->surface_list.list[icd_surface->surface_index]);

[TomCoo]

main...TomCoo:tomcooper:destroy_surface_assert

[charles-lunarg]

The assert firing for you gives me cause for concern that something else in the code isn't right, and allowing a VkSurfaceKHR to be not-null when it should be NULL. wsi.c:84 has a check for "interface_version >= ICD_VER_SUPPORTS_ICD_SURFACE_KHR" but is missing the 'icd_term->enabled_instance_extensions.khr_surface' check. That would allow a surface to be created for an ICD that doesn't support khr_surface.

Let me create a PR to see if that fixes the issue for you.

[charles-lunarg]

Looking through wsi.c some more, it strikes me that something bad is happening somewhere. The ICD's surface handle array should only be valid if the khr_surface extension is present on the ICD and the ICD's interface version is high enough. allocate_icd_surface_struct()'s logic checks those conditions before allocating the ICD's surface handle array in the first place. My above comment about wsi_unwrap_icd_surface() (wsi.c:84) is redundant because it checks that the pointer to the array is non-NULL. (thus the PR I was going to create is not going to happen, as I believe something else is the root cause)

Do you have any more details about the environment that triggered the assert? Multiple drivers on a system, ICD interface version + VK_KHR_surface extension support (or lack of), etc etc. If I can recreated this in the test framework that will go a long way to preventing it in the future.

[TomCoo]

The assert is an issue after 2526eac

I believe it is only an issue if VK_KHR_surface is implemented in a layer rather than the driver. So it is ok for the app to call vkDestroySurfaceKHR but there is no support for it in the ICD and the surface list is not created in the loader.