From 438875314dc838d130098127c6e908798500fecc Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Wed, 14 Aug 2024 11:24:18 -0500 Subject: [PATCH] Fix vkCreateSharedSwapchainsKHR not unwrapping handles correctly The loader would unwrap the first surface and use that for all created swapchains rather than unwrap each surface handle individually for each VkSwapchainCreateInfoKHR struct. --- loader/wsi.c | 35 ++++++++++++++-------------- tests/framework/icd/test_icd.cpp | 7 +++++- tests/loader_get_proc_addr_tests.cpp | 11 ++++++++- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/loader/wsi.c b/loader/wsi.c index 471775023..c9c0ceca0 100644 --- a/loader/wsi.c +++ b/loader/wsi.c @@ -2078,31 +2078,32 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateSharedSwapchainsKHR(VkDevice dev "[VUID-vkCreateSharedSwapchainsKHR-device-parameter]"); abort(); /* Intentionally fail so user can correct issue. */ } + if (NULL != icd_term->surface_list.list) { + loader_log(NULL, VULKAN_LOADER_ERROR_BIT, 0, + "vkCreateSharedSwapchainsKHR Terminator: No VkSurfaceKHR objects were created, indicating an application " + "bug. Returning VK_SUCCESS. "); + return VK_SUCCESS; + } if (NULL == dev->loader_dispatch.extension_terminator_dispatch.CreateSharedSwapchainsKHR) { loader_log(NULL, VULKAN_LOADER_ERROR_BIT, 0, - "vkCreateSharedSwapchainsKHR: Driver's function pointer was NULL, returning VK_SUCCESS. Was the " + "vkCreateSharedSwapchainsKHR Terminator: Driver's function pointer was NULL, returning VK_SUCCESS. Was the " "VK_KHR_display_swapchain extension enabled?"); return VK_SUCCESS; } - VkIcdSurface *icd_surface = (VkIcdSurface *)(uintptr_t)pCreateInfos->surface; - if (NULL != icd_term->surface_list.list && - icd_term->surface_list.capacity > icd_surface->surface_index * sizeof(VkSurfaceKHR) && - icd_term->surface_list.list[icd_surface->surface_index]) { - // We found the ICD, and there is an ICD KHR surface - // associated with it, so copy the CreateInfo struct - // and point it at the ICD's surface. - VkSwapchainCreateInfoKHR *pCreateCopy = loader_stack_alloc(sizeof(VkSwapchainCreateInfoKHR) * swapchainCount); - if (NULL == pCreateCopy) { - return VK_ERROR_OUT_OF_HOST_MEMORY; - } - memcpy(pCreateCopy, pCreateInfos, sizeof(VkSwapchainCreateInfoKHR) * swapchainCount); - for (uint32_t sc = 0; sc < swapchainCount; sc++) { + + VkSwapchainCreateInfoKHR *pCreateCopy = loader_stack_alloc(sizeof(VkSwapchainCreateInfoKHR) * swapchainCount); + if (NULL == pCreateCopy) { + return VK_ERROR_OUT_OF_HOST_MEMORY; + } + memcpy(pCreateCopy, pCreateInfos, sizeof(VkSwapchainCreateInfoKHR) * swapchainCount); + for (uint32_t sc = 0; sc < swapchainCount; sc++) { + VkIcdSurface *icd_surface = (VkIcdSurface *)(uintptr_t)pCreateCopy[sc].surface; + if (icd_term->surface_list.capacity > icd_surface->surface_index * sizeof(VkSurfaceKHR) && + icd_term->surface_list.list[icd_surface->surface_index]) { pCreateCopy[sc].surface = icd_term->surface_list.list[icd_surface->surface_index]; } - return dev->loader_dispatch.extension_terminator_dispatch.CreateSharedSwapchainsKHR(device, swapchainCount, pCreateCopy, - pAllocator, pSwapchains); } - return dev->loader_dispatch.extension_terminator_dispatch.CreateSharedSwapchainsKHR(device, swapchainCount, pCreateInfos, + return dev->loader_dispatch.extension_terminator_dispatch.CreateSharedSwapchainsKHR(device, swapchainCount, pCreateCopy, pAllocator, pSwapchains); } diff --git a/tests/framework/icd/test_icd.cpp b/tests/framework/icd/test_icd.cpp index b4d92d2fa..bd17629a1 100644 --- a/tests/framework/icd/test_icd.cpp +++ b/tests/framework/icd/test_icd.cpp @@ -936,10 +936,15 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkGetPhysicalDeviceSurfaceFormats2KHR(VkPhys } // VK_KHR_display_swapchain VKAPI_ATTR VkResult VKAPI_CALL test_vkCreateSharedSwapchainsKHR([[maybe_unused]] VkDevice device, uint32_t swapchainCount, - [[maybe_unused]] const VkSwapchainCreateInfoKHR* pCreateInfos, + const VkSwapchainCreateInfoKHR* pCreateInfos, [[maybe_unused]] const VkAllocationCallbacks* pAllocator, VkSwapchainKHR* pSwapchains) { for (uint32_t i = 0; i < swapchainCount; i++) { + uint64_t surface_integer_value = from_nondispatch_handle(pCreateInfos[i].surface); + auto found_iter = std::find(icd.surface_handles.begin(), icd.surface_handles.end(), surface_integer_value); + if (found_iter == icd.surface_handles.end()) { + return VK_ERROR_INITIALIZATION_FAILED; + } common_nondispatch_handle_creation(icd.swapchain_handles, &pSwapchains[i]); } return VK_SUCCESS; diff --git a/tests/loader_get_proc_addr_tests.cpp b/tests/loader_get_proc_addr_tests.cpp index 991f1b79d..a2fe578d8 100644 --- a/tests/loader_get_proc_addr_tests.cpp +++ b/tests/loader_get_proc_addr_tests.cpp @@ -213,6 +213,9 @@ TEST(GetDeviceProcAddr, SwapchainFuncsWithTerminator) { VkSurfaceKHR surface{}; ASSERT_EQ(VK_SUCCESS, create_surface(inst, surface)); + VkSurfaceKHR surface2{}; + ASSERT_EQ(VK_SUCCESS, create_surface(inst, surface2)); + DebugUtilsWrapper log{inst}; ASSERT_EQ(VK_SUCCESS, CreateDebugUtilsMessenger(log)); auto phys_dev = inst.GetPhysDev(); @@ -286,9 +289,15 @@ TEST(GetDeviceProcAddr, SwapchainFuncsWithTerminator) { VkDeviceGroupPresentModeFlagsKHR modes{}; GetDeviceGroupSurfacePresentModesKHR(dev.dev, surface, &modes); - CreateSharedSwapchainsKHR(dev.dev, 1, &info, nullptr, &swapchain); + std::array infos{}; + infos[0] = info; + infos[1].sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR; + infos[1].surface = surface2; + + ASSERT_EQ(VK_SUCCESS, CreateSharedSwapchainsKHR(dev.dev, 2, infos.data(), nullptr, &swapchain)); } env.vulkan_functions.vkDestroySurfaceKHR(inst.inst, surface, nullptr); + env.vulkan_functions.vkDestroySurfaceKHR(inst.inst, surface2, nullptr); } // Verify that the various ways to get vkGetDeviceProcAddr return the same value