Check for NULL in unloading of preloaded-icd's

strcmp has undefined behavior if either parameter is NULL. Due to clearing
out unloaded entries, this would cause unload_drivers_without_physical_devices
to try to compare against already unloaded entries.

This commit also condenses the preloaded_icds list so that it doesn't contain
gaps.

diff --git a/loader/loader.c b/loader/loader.c
index 98ad3784c7247ad118ca96efa3fbc76f7b16ed4d..6bcefcd7d2ae24f39243304ee7227ee9cef22670 100644 (file)
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -6527,8 +6527,21 @@ void unload_drivers_without_physical_devices(struct loader_instance *inst) {
                 loader_platform_thread_lock_mutex(&loader_preload_icd_lock);
                 if (NULL != preloaded_icds.scanned_list) {
                     for (uint32_t i = 0; i < preloaded_icds.count; i++) {
-                        if (strcmp(preloaded_icds.scanned_list[i].lib_name, scanned_icd_to_remove->lib_name) == 0) {
+                        if (NULL != preloaded_icds.scanned_list[i].lib_name && NULL != scanned_icd_to_remove->lib_name &&
+                            strcmp(preloaded_icds.scanned_list[i].lib_name, scanned_icd_to_remove->lib_name) == 0) {
                             loader_unload_scanned_icd(inst, &preloaded_icds.scanned_list[i]);
+                            // condense the list so that it doesn't contain empty elements.
+                            if (i < preloaded_icds.count - 1) {
+                                memcpy((void *)&preloaded_icds.scanned_list[i],
+                                       (void *)&preloaded_icds.scanned_list[preloaded_icds.count - 1],
+                                       sizeof(struct loader_scanned_icd));
+                                memset((void *)&preloaded_icds.scanned_list[preloaded_icds.count - 1], 0,
+                                       sizeof(struct loader_scanned_icd));
+                            }
+                            if (i > 0) {
+                                preloaded_icds.count--;
+                            }
+
                             break;
                         }
                     }

diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp
index 523b5bf639bf6148d84dd5b23dc0fa5a876c3e61..366936fa5318cb987eaa34d44fcaa3935e2af193 100644 (file)
--- a/tests/loader_regression_tests.cpp
+++ b/tests/loader_regression_tests.cpp
@@ -4679,6 +4679,35 @@ TEST(DriverUnloadingFromZeroPhysDevs, AtFrontAndBack) {
     }
 }
 
+TEST(DriverUnloadingFromZeroPhysDevs, MultipleEnumerateCalls) {
+    FrameworkEnvironment env{};
+    add_empty_driver_for_unloading_testing(env);
+    add_empty_driver_for_unloading_testing(env);
+    add_driver_for_unloading_testing(env);
+    add_driver_for_unloading_testing(env);
+    add_empty_driver_for_unloading_testing(env);
+    add_empty_driver_for_unloading_testing(env);
+
+    uint32_t extension_count = 0;
+    ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceExtensionProperties(nullptr, &extension_count, 0));
+    ASSERT_EQ(extension_count, 6U);  // default extensions + surface extensions
+    std::array<VkExtensionProperties, 6> extensions;
+    ASSERT_EQ(VK_SUCCESS,
+              env.vulkan_functions.vkEnumerateInstanceExtensionProperties(nullptr, &extension_count, extensions.data()));
+
+    {
+        InstWrapper inst{env.vulkan_functions};
+        inst.CheckCreate();
+        auto phys_devs1 = inst.GetPhysDevs();
+        auto phys_devs2 = inst.GetPhysDevs();
+    }
+    {
+        InstWrapper inst{env.vulkan_functions};
+        inst.CheckCreate();
+        auto phys_devs1 = inst.GetPhysDevs();
+        auto phys_devs2 = inst.GetPhysDevs();
+    }
+}
 TEST(DriverUnloadingFromZeroPhysDevs, NoPhysicalDevices) {
     FrameworkEnvironment env{};
     add_empty_driver_for_unloading_testing(env);