diff --git a/loader/loader.c b/loader/loader.c index f95c7594f..2fb422cc2 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -407,6 +407,19 @@ VkResult copy_str_to_start_of_string_list(const struct loader_instance *inst, st return prepend_str_to_string_list(inst, string_list, new_str); } +void remove_str_from_string_list(const struct loader_instance *inst, struct loader_string_list *string_list, const char *str) { + assert(string_list && str); + for (uint32_t i = 0; i < string_list->count; i++) { + if (strcmp(string_list->list[i], str) == 0) { + loader_instance_heap_free(inst, string_list->list[i]); + memmove((void *)(string_list->list + i), (const void *)(string_list->list + i + 1), + sizeof(char *) * (string_list->count - i - 1)); + string_list->count--; + return; + } + } +} + void free_string_list(const struct loader_instance *inst, struct loader_string_list *string_list) { assert(string_list); if (string_list->list) { @@ -3384,7 +3397,8 @@ VkResult add_if_manifest_file(const struct loader_instance *inst, const char *fi return VK_INCOMPLETE; } - return copy_str_to_string_list(inst, out_files, file_name, name_len); + // A directory search path and a file inside it can reach the same manifest. + return copy_str_to_string_list_if_unique(inst, out_files, file_name, name_len); } // If the file found is a manifest file name, add it to the start of the out_files manifest list. @@ -3399,6 +3413,8 @@ VkResult prepend_if_manifest_file(const struct loader_instance *inst, const char return VK_INCOMPLETE; } + // Move an existing entry to the front instead of loading it twice. + remove_str_from_string_list(inst, out_files, file_name); return copy_str_to_start_of_string_list(inst, out_files, file_name, name_len); } diff --git a/loader/loader.h b/loader/loader.h index ad498ed0e..6f6385152 100644 --- a/loader/loader.h +++ b/loader/loader.h @@ -137,6 +137,9 @@ VkResult copy_str_to_string_list_if_unique(const struct loader_instance *inst, s VkResult copy_str_to_start_of_string_list(const struct loader_instance *inst, struct loader_string_list *string_list, const char *str, size_t str_len); +// Remove the first string equal to str from string_list and free it, if there is one +void remove_str_from_string_list(const struct loader_instance *inst, struct loader_string_list *string_list, const char *str); + // Free any string inside of loader_string_list and then free the list itself void free_string_list(const struct loader_instance *inst, struct loader_string_list *string_list); diff --git a/tests/loader_envvar_tests.cpp b/tests/loader_envvar_tests.cpp index dc6fad395..6ae228f67 100644 --- a/tests/loader_envvar_tests.cpp +++ b/tests/loader_envvar_tests.cpp @@ -177,6 +177,31 @@ TEST(EnvVarICDOverrideSetup, TestOnlyDriverEnvVarInFolder) { ASSERT_EQ(inst2->vkEnumeratePhysicalDevices(inst2.inst, &phys_dev_count, phys_devs_array.data()), VK_SUCCESS); ASSERT_EQ(phys_dev_count, 5U); } +// REPRO for issue #1874: one ICD added by folder, two more added by file name in that same +// folder. The loader scans the folder (finding all three) and then also processes the two +// explicit file paths, so those two ICDs are loaded twice and their devices reported twice. +TEST(EnvVarICDOverrideSetup, DirAndFileOverlapDoesNotDuplicate) { + FrameworkEnvironment env{}; + // added by directory + env.add_icd(TEST_ICD_PATH_EXPORT_NONE, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::env_var).set_is_dir(true)) + .add_physical_device("pd0"); + // added by file name, into the very same folder + for (uint32_t add = 0; add < 2; ++add) { + env.add_icd(TEST_ICD_PATH_EXPORT_NONE, + ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::env_var).set_is_dir(false)) + .add_physical_device("pd" + std::to_string(add) + "0") + .add_physical_device("pd" + std::to_string(add) + "1"); + } + + InstWrapper inst{env.vulkan_functions}; + inst.create_info.set_api_version(VK_API_VERSION_1_1); + inst.CheckCreate(); + + uint32_t phys_dev_count = 0; + ASSERT_EQ(inst->vkEnumeratePhysicalDevices(inst.inst, &phys_dev_count, nullptr), VK_SUCCESS); + EXPECT_EQ(phys_dev_count, 5U); +} + // Test VK_DRIVER_FILES environment variable containing a path to a folder with elevated privileges TEST(EnvVarICDOverrideSetup, TestOnlyDriverEnvVarInFolderWithElevatedPrivileges) { FrameworkEnvironment env{FrameworkSettings{}.set_run_as_if_with_elevated_privileges(true)}; diff --git a/tests/loader_settings_tests.cpp b/tests/loader_settings_tests.cpp index 9454d6633..dc8b64d5e 100644 --- a/tests/loader_settings_tests.cpp +++ b/tests/loader_settings_tests.cpp @@ -3095,6 +3095,20 @@ TEST(SettingsFile, AdditionalDrivers) { ASSERT_TRUE(string_eq(props1.deviceName, regular_driver_name)); ASSERT_TRUE(string_eq(props2.deviceName, settings_driver_name)); } +// settings file provided driver that the regular search also finds +TEST(SettingsFile, AdditionalDriverAlreadyFound) { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::env_var)) + .add_physical_device({}); + + env.loader_settings.set_file_format_version({1, 0, 0}).add_app_specific_setting( + AppSpecificSettings{}.add_driver_configuration(LoaderSettingsDriverConfiguration{}.set_path(env.get_icd_manifest_path(0)))); + env.update_loader_settings(env.loader_settings); + + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + ASSERT_EQ(inst.GetPhysDevs().size(), 1U); +} // settings file provided drivers replacing system found drivers TEST(SettingsFile, ExclusiveAdditionalDrivers) { FrameworkEnvironment env{};