From 50df6bc578c851ddc7ec1f3b9f631918a4071b03 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Fri, 31 Mar 2023 17:26:07 -0600 Subject: [PATCH] Fix ordering regression for VK_INSTANCE_LAYERS The commit to add VK_LOADER_LAYERS_ENABLE/DISABLE inadvertently broke the ordering guarantees of VK_INSTANCE_LAYERS. This commit restores that order. If both VK_INSTANCE_LAYERS and VK_LOADER_LAYERS_ENABLE add layers, the order will be VK_INSTANCE_LAYERS enabled layers, in the order specified by the env-var, followed by any layers enabled by VK_LOADER_LAYERS_ENABLED enabled in the order they are found on the system. In addition to this, the test framework receieved two changes: * Make env-var folders report their contents in a deterministic order (aka the order the items were added to the framework). This mirrors existing behavior for redirected folders. * Make platform shim resilient to shutdown ordering issues. Namely, set a flag during shutdown so that any interception functions go straight to the real version of the functoin. This works around an issue during the destruction of the FolderManagers for the environment variables, where the readdir shim function was trying to use the FolderManager's data but Address Sanitizer declared that such a use was 'heap use after free'. * Rename set_path to set_fake_path to better indicate its purpose --- loader/loader.c | 9 +- loader/loader_environment.c | 75 ++++------- loader/loader_environment.h | 4 +- tests/framework/shim/shim.h | 20 ++- tests/framework/shim/shim_common.cpp | 13 +- tests/framework/shim/unix_shim.cpp | 21 ++- tests/framework/test_environment.cpp | 24 +++- tests/framework/test_environment.h | 1 + tests/loader_layer_tests.cpp | 239 +++++++++++++++++++++++++++++++++++ tests/loader_regression_tests.cpp | 4 +- 10 files changed, 335 insertions(+), 75 deletions(-) diff --git a/loader/loader.c b/loader/loader.c index 7705346..0d4de32 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -4450,9 +4450,8 @@ VkResult loader_enable_instance_layers(struct loader_instance *inst, const VkIns } // Add any layers specified via environment variable next - res = loader_add_environment_layers(inst, VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER, "VK_INSTANCE_LAYERS", &layers_enable_filter, - &layers_disable_filter, &inst->app_activated_layer_list, - &inst->expanded_activated_layer_list, instance_layers); + res = loader_add_environment_layers(inst, VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER, &layers_enable_filter, &layers_disable_filter, + &inst->app_activated_layer_list, &inst->expanded_activated_layer_list, instance_layers); if (res != VK_SUCCESS) { goto out; } @@ -5343,8 +5342,8 @@ VkResult loader_validate_instance_extensions(struct loader_instance *inst, const if (res != VK_SUCCESS) { goto out; } - res = loader_add_environment_layers(inst, VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER, ENABLED_LAYERS_ENV, &layers_enable_filter, - &layers_disable_filter, &active_layers, &expanded_layers, instance_layers); + res = loader_add_environment_layers(inst, VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER, &layers_enable_filter, &layers_disable_filter, + &active_layers, &expanded_layers, instance_layers); if (res != VK_SUCCESS) { goto out; } diff --git a/loader/loader_environment.c b/loader/loader_environment.c index 5d82d50..58e224e 100644 --- a/loader/loader_environment.c +++ b/loader/loader_environment.c @@ -417,62 +417,53 @@ bool check_name_matches_filter_environment_var(const struct loader_instance *ins // Get the layer name(s) from the env_name environment variable. If layer is found in // search_list then add it to layer_list. But only add it to layer_list if type_flags matches. -VkResult loader_add_environment_layers(struct loader_instance *inst, const enum layer_type_flags type_flags, const char *env_name, +VkResult loader_add_environment_layers(struct loader_instance *inst, const enum layer_type_flags type_flags, const struct loader_envvar_filter *enable_filter, const struct loader_envvar_disable_layers_filter *disable_filter, struct loader_layer_list *target_list, struct loader_layer_list *expanded_target_list, const struct loader_layer_list *source_list) { VkResult res = VK_SUCCESS; - char *next, *name; - char *layer_env = loader_getenv(env_name, inst); - char **vk_inst_layers = NULL; - uint32_t vk_inst_layer_count = 0; - uint32_t separator_count = 0; + char *layer_env = loader_getenv(ENABLED_LAYERS_ENV, inst); // If the layer environment variable is present (i.e. VK_INSTANCE_LAYERS), we will always add it to the layer list. if (layer_env != NULL) { - name = loader_stack_alloc(strlen(layer_env) + 1); + char *name = loader_stack_alloc(strlen(layer_env) + 1); if (name != NULL) { - separator_count = 1; - for (uint32_t c = 0; c < strlen(layer_env); ++c) { - if (layer_env[c] == PATH_SEPARATOR) { - separator_count++; - } - } - - vk_inst_layers = - loader_instance_heap_calloc(inst, (separator_count * sizeof(char *)), VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); - if (vk_inst_layers == NULL) { - res = VK_ERROR_OUT_OF_HOST_MEMORY; - goto out; - } - for (uint32_t cur_layer = 0; cur_layer < separator_count; ++cur_layer) { - vk_inst_layers[cur_layer] = - loader_instance_heap_calloc(inst, VK_MAX_EXTENSION_NAME_SIZE, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); - if (vk_inst_layers[cur_layer] == NULL) { - res = VK_ERROR_OUT_OF_HOST_MEMORY; - goto out; - } - } - strcpy(name, layer_env); loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, "env var \'%s\' defined and adding layers \"%s\"", - env_name, name); + ENABLED_LAYERS_ENV, name); // First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS while (name && *name) { - next = loader_get_next_path(name); + char *next = loader_get_next_path(name); if (strlen(name) > 0) { - strncpy(vk_inst_layers[vk_inst_layer_count++], name, VK_MAX_EXTENSION_NAME_SIZE); + for (uint32_t i = 0; i < source_list->count; i++) { + struct loader_layer_properties *source_prop = &source_list->list[i]; + + if (0 == strcmp(name, source_prop->info.layerName) && + !loader_find_layer_name_in_list(source_prop->info.layerName, target_list)) { + if (0 == (source_prop->type_flags & VK_LAYER_TYPE_FLAG_META_LAYER)) { + res = loader_add_layer_properties_to_list(inst, target_list, 1, source_prop); + if (res == VK_ERROR_OUT_OF_HOST_MEMORY) goto out; + res = loader_add_layer_properties_to_list(inst, expanded_target_list, 1, source_prop); + if (res == VK_ERROR_OUT_OF_HOST_MEMORY) goto out; + } else { + res = loader_add_meta_layer(inst, enable_filter, disable_filter, source_prop, target_list, + expanded_target_list, source_list, NULL); + if (res == VK_ERROR_OUT_OF_HOST_MEMORY) goto out; + } + break; + } + } } name = next; } } } - // Loop through all the layers and check the enable/disable filters as well as the VK_INSTANCE_LAYERS value. + // Loop through all the layers and check the enable/disable filters for (uint32_t i = 0; i < source_list->count; i++) { struct loader_layer_properties *source_prop = &source_list->list[i]; @@ -507,17 +498,6 @@ VkResult loader_add_environment_layers(struct loader_instance *inst, const enum "Layer \"%s\" forced enabled due to env var \'%s\'", source_prop->info.layerName, VK_LAYERS_ENABLE_ENV_VAR); } else { adding = false; - // If it's not in the enable filter, check the environment variable if it exists - // Also make sure the layer isn't already in the output_list, skip adding it if it is. - if (vk_inst_layer_count > 0) { - for (uint32_t cur_layer = 0; cur_layer < vk_inst_layer_count; ++cur_layer) { - if (!strcmp(vk_inst_layers[cur_layer], source_prop->info.layerName) && - !loader_find_layer_name_in_list(source_prop->info.layerName, target_list)) { - adding = true; - break; - } - } - } } if (!adding) { @@ -539,13 +519,6 @@ VkResult loader_add_environment_layers(struct loader_instance *inst, const enum out: - if (NULL != vk_inst_layers) { - for (uint32_t cur_layer = 0; cur_layer < separator_count; ++cur_layer) { - loader_instance_heap_free(inst, vk_inst_layers[cur_layer]); - } - loader_instance_heap_free(inst, vk_inst_layers); - } - if (layer_env != NULL) { loader_free_getenv(layer_env, inst); } diff --git a/loader/loader_environment.h b/loader/loader_environment.h index 2eb61d5..58d622a 100644 --- a/loader/loader_environment.h +++ b/loader/loader_environment.h @@ -50,8 +50,8 @@ VkResult parse_layers_disable_filter_environment_var(const struct loader_instanc struct loader_envvar_disable_layers_filter *disable_struct); bool check_name_matches_filter_environment_var(const struct loader_instance *inst, const char *name, const struct loader_envvar_filter *filter_struct); -VkResult loader_add_environment_layers(struct loader_instance *inst, const enum layer_type_flags type_flags, const char *env_name, +VkResult loader_add_environment_layers(struct loader_instance *inst, const enum layer_type_flags type_flags, const struct loader_envvar_filter *enable_filter, const struct loader_envvar_disable_layers_filter *disable_filter, struct loader_layer_list *target_list, struct loader_layer_list *expanded_target_list, - const struct loader_layer_list *source_list); \ No newline at end of file + const struct loader_layer_list *source_list); diff --git a/tests/framework/shim/shim.h b/tests/framework/shim/shim.h index 44d54dd..bb27bbe 100644 --- a/tests/framework/shim/shim.h +++ b/tests/framework/shim/shim.h @@ -29,6 +29,7 @@ #include "test_util.h" +#include #include #if defined(WIN32) @@ -116,7 +117,8 @@ struct DirEntry { DIR* directory; std::string folder_path; std::vector contents; - size_t current_index; + size_t current_index; // the current item being read by an app (incremented by readdir, reset to zero by opendir & closedir) + bool is_fake_path; // true when this entry is for folder redirection }; #endif @@ -138,7 +140,12 @@ struct PlatformShim { void redirect_all_paths(fs::path const& path); void redirect_category(fs::path const& new_path, ManifestCategory category); - void set_path(ManifestCategory category, fs::path const& path); + // fake paths are paths that the loader normally looks in but actually point to locations inside the test framework + void set_fake_path(ManifestCategory category, fs::path const& path); + + // known paths are real paths but since the test framework guarantee's the order files are found in, files in these paths need + // to be ordered correctly + void add_known_path(fs::path const& path); void add_manifest(ManifestCategory category, fs::path const& path); @@ -188,17 +195,22 @@ struct PlatformShim { void redirect_path(fs::path const& path, fs::path const& new_path); void remove_redirect(fs::path const& path); + bool is_known_path(fs::path const& path); + void remove_known_path(fs::path const& path); + std::unordered_map redirection_map; + std::unordered_set known_path_set; void set_elevated_privilege(bool elev) { use_fake_elevation = elev; } bool use_fake_elevation = false; std::vector dir_entries; - #if defined(__APPLE__) +#if defined(__APPLE__) std::string bundle_contents; - #endif #endif +#endif + bool is_during_destruction = false; }; std::vector parse_env_var_list(std::string const& var); diff --git a/tests/framework/shim/shim_common.cpp b/tests/framework/shim/shim_common.cpp index 02f71b8..b1a09c7 100644 --- a/tests/framework/shim/shim_common.cpp +++ b/tests/framework/shim/shim_common.cpp @@ -103,7 +103,8 @@ void PlatformShim::reset() { hkey_local_machine_drivers.clear(); } -void PlatformShim::set_path(ManifestCategory category, fs::path const& path) {} +void PlatformShim::set_fake_path(ManifestCategory category, fs::path const& path) {} +void PlatformShim::add_known_path(fs::path const& path) {} void PlatformShim::add_manifest(ManifestCategory category, fs::path const& path) { if (category == ManifestCategory::implicit_layer) @@ -161,10 +162,14 @@ std::string category_path_name(ManifestCategory category) { void PlatformShim::reset() { redirection_map.clear(); } -void PlatformShim::redirect_path(fs::path const& path, fs::path const& new_path) { redirection_map[path.str()] = new_path; } -void PlatformShim::remove_redirect(fs::path const& path) { redirection_map.erase(path.str()); } bool PlatformShim::is_fake_path(fs::path const& path) { return redirection_map.count(path.str()) > 0; } fs::path const& PlatformShim::get_fake_path(fs::path const& path) { return redirection_map.at(path.str()); } +void PlatformShim::redirect_path(fs::path const& path, fs::path const& new_path) { redirection_map[path.str()] = new_path; } +void PlatformShim::remove_redirect(fs::path const& path) { redirection_map.erase(path.str()); } + +bool PlatformShim::is_known_path(fs::path const& path) { return known_path_set.count(path.str()) > 0; } +void PlatformShim::add_known_path(fs::path const& path) { known_path_set.insert(path.str()); } +void PlatformShim::remove_known_path(fs::path const& path) { known_path_set.erase(path.str()); } void PlatformShim::add_manifest(ManifestCategory category, fs::path const& path) {} @@ -215,7 +220,7 @@ void PlatformShim::redirect_category(fs::path const& new_path, ManifestCategory } } -void PlatformShim::set_path(ManifestCategory category, fs::path const& path) { +void PlatformShim::set_fake_path(ManifestCategory category, fs::path const& path) { // use /etc as the 'redirection path' by default since its always searched redirect_path(fs::path(SYSCONFDIR) / "vulkan" / category_path_name(category), path); } diff --git a/tests/framework/shim/unix_shim.cpp b/tests/framework/shim/unix_shim.cpp index 1f03942..7e0e78d 100644 --- a/tests/framework/shim/unix_shim.cpp +++ b/tests/framework/shim/unix_shim.cpp @@ -123,11 +123,17 @@ FRAMEWORK_EXPORT DIR* OPENDIR_FUNC_NAME(const char* path_name) { #if !defined(__APPLE__) if (!real_opendir) real_opendir = (PFN_OPENDIR)dlsym(RTLD_NEXT, "opendir"); #endif + if (platform_shim.is_during_destruction) { + return real_opendir(path_name); + } DIR* dir; if (platform_shim.is_fake_path(path_name)) { auto fake_path_name = platform_shim.get_fake_path(fs::path(path_name)); dir = real_opendir(fake_path_name.c_str()); - platform_shim.dir_entries.push_back(DirEntry{dir, std::string(path_name), {}, false}); + platform_shim.dir_entries.push_back(DirEntry{dir, std::string(path_name), {}, 0, true}); + } else if (platform_shim.is_known_path(path_name)) { + dir = real_opendir(path_name); + platform_shim.dir_entries.push_back(DirEntry{dir, std::string(path_name), {}, 0, false}); } else { dir = real_opendir(path_name); } @@ -139,6 +145,9 @@ FRAMEWORK_EXPORT struct dirent* READDIR_FUNC_NAME(DIR* dir_stream) { #if !defined(__APPLE__) if (!real_readdir) real_readdir = (PFN_READDIR)dlsym(RTLD_NEXT, "readdir"); #endif + if (platform_shim.is_during_destruction) { + return real_readdir(dir_stream); + } auto it = std::find_if(platform_shim.dir_entries.begin(), platform_shim.dir_entries.end(), [dir_stream](DirEntry const& entry) { return entry.directory == dir_stream; }); @@ -157,8 +166,11 @@ FRAMEWORK_EXPORT struct dirent* READDIR_FUNC_NAME(DIR* dir_stream) { folder_contents.push_back(dir_entry); dirent_filenames.push_back(&dir_entry->d_name[0]); } - auto real_path = platform_shim.redirection_map.at(it->folder_path); - auto filenames = get_folder_contents(platform_shim.folders, real_path.str()); + auto real_path = it->folder_path; + if (it->is_fake_path) { + real_path = platform_shim.redirection_map.at(it->folder_path).str(); + } + auto filenames = get_folder_contents(platform_shim.folders, real_path); // Add the dirent structures in the order they appear in the FolderManager // Ignore anything which wasn't in the FolderManager @@ -179,6 +191,9 @@ FRAMEWORK_EXPORT int CLOSEDIR_FUNC_NAME(DIR* dir_stream) { #if !defined(__APPLE__) if (!real_closedir) real_closedir = (PFN_CLOSEDIR)dlsym(RTLD_NEXT, "closedir"); #endif + if (platform_shim.is_during_destruction) { + return real_closedir(dir_stream); + } auto it = std::find_if(platform_shim.dir_entries.begin(), platform_shim.dir_entries.end(), [dir_stream](DirEntry const& entry) { return entry.directory == dir_stream; }); diff --git a/tests/framework/test_environment.cpp b/tests/framework/test_environment.cpp index 8081f6d..8f944ad 100644 --- a/tests/framework/test_environment.cpp +++ b/tests/framework/test_environment.cpp @@ -374,9 +374,9 @@ FrameworkEnvironment::FrameworkEnvironment(bool enable_log, bool set_default_sea platform_shim->redirect_all_paths(get_folder(ManifestLocation::null).location()); if (set_default_search_paths) { - platform_shim->set_path(ManifestCategory::icd, get_folder(ManifestLocation::driver).location()); - platform_shim->set_path(ManifestCategory::explicit_layer, get_folder(ManifestLocation::explicit_layer).location()); - platform_shim->set_path(ManifestCategory::implicit_layer, get_folder(ManifestLocation::implicit_layer).location()); + platform_shim->set_fake_path(ManifestCategory::icd, get_folder(ManifestLocation::driver).location()); + platform_shim->set_fake_path(ManifestCategory::explicit_layer, get_folder(ManifestLocation::explicit_layer).location()); + platform_shim->set_fake_path(ManifestCategory::implicit_layer, get_folder(ManifestLocation::implicit_layer).location()); } #if defined(__APPLE__) // Necessary since bundles look in sub folders for manifests, not the test framework folder itself @@ -387,6 +387,14 @@ FrameworkEnvironment::FrameworkEnvironment(bool enable_log, bool set_default_sea #endif } +FrameworkEnvironment::~FrameworkEnvironment() { + // This is necessary to prevent the folder manager from using dead memory during destruction. + // What happens is that each folder manager tries to cleanup itself. Except, folders that were never called did not have their + // DirEntry array's filled out. So when that folder calls delete_folder, which calls readdir, the shim tries to order the files. + // Except, the list of files is in a object that is currently being destroyed. + platform_shim->is_during_destruction = true; +} + TestICDHandle& FrameworkEnvironment::add_icd(TestICDDetails icd_details) noexcept { size_t cur_icd_index = icds.size(); fs::FolderManager* folder = &get_folder(ManifestLocation::driver); @@ -428,9 +436,11 @@ TestICDHandle& FrameworkEnvironment::add_icd(TestICDDetails icd_details) noexcep break; case (ManifestDiscoveryType::env_var): env_var_vk_icd_filenames.add_to_list((folder->location() / full_json_name).str()); + platform_shim->add_known_path(folder->location()); break; case (ManifestDiscoveryType::add_env_var): add_env_var_vk_icd_filenames.add_to_list((folder->location() / full_json_name).str()); + platform_shim->add_known_path(folder->location()); break; case (ManifestDiscoveryType::macos_bundle): platform_shim->add_manifest(ManifestCategory::icd, icds.back().manifest_path); @@ -479,10 +489,16 @@ void FrameworkEnvironment::add_layer_impl(TestLayerDetails layer_details, Manife } else { env_var_vk_layer_paths.add_to_list((fs_ptr->location() / layer_details.json_name).str()); } + platform_shim->add_known_path(fs_ptr->location()); break; case (ManifestDiscoveryType::add_env_var): fs_ptr = &get_folder(ManifestLocation::explicit_layer_add_env_var); - add_env_var_vk_layer_paths.add_to_list(fs_ptr->location().str()); + if (layer_details.is_dir) { + add_env_var_vk_layer_paths.add_to_list(fs_ptr->location().str()); + } else { + add_env_var_vk_layer_paths.add_to_list((fs_ptr->location() / layer_details.json_name).str()); + } + platform_shim->add_known_path(fs_ptr->location()); break; case (ManifestDiscoveryType::override_folder): fs_ptr = &get_folder(ManifestLocation::override_layer); diff --git a/tests/framework/test_environment.h b/tests/framework/test_environment.h index c671700..ac29cf6 100644 --- a/tests/framework/test_environment.h +++ b/tests/framework/test_environment.h @@ -503,6 +503,7 @@ struct FrameworkEnvironment { FrameworkEnvironment() noexcept; // default is to enable VK_LOADER_DEBUG=all and enable the default search paths FrameworkEnvironment(bool enable_log) noexcept; FrameworkEnvironment(bool enable_log, bool enable_default_search_paths) noexcept; + ~FrameworkEnvironment(); TestICDHandle& add_icd(TestICDDetails icd_details) noexcept; void add_implicit_layer(ManifestLayer layer_manifest, const std::string& json_name) noexcept; diff --git a/tests/loader_layer_tests.cpp b/tests/loader_layer_tests.cpp index 5268f75..c8cc7b4 100644 --- a/tests/loader_layer_tests.cpp +++ b/tests/loader_layer_tests.cpp @@ -2468,6 +2468,245 @@ TEST(ExplicitLayers, DuplicateLayersInVK_ADD_LAYER_PATH) { env.debug_log.clear(); } +TEST(ExplicitLayers, CorrectOrderOfEnvVarEnabledLayers) { + FrameworkEnvironment env; + env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA)); + env.get_test_icd().add_physical_device({}); + + // verify layer loads successfully when setting VK_LAYER_PATH to a full filepath + const char* layer_name_1 = "VK_LAYER_RegularLayer1"; + env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(layer_name_1) + .set_description("actually_layer_1") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)), + "regular_layer_1.json") + .set_discovery_type(ManifestDiscoveryType::env_var) + .set_is_dir(true)); + auto& layer1 = env.get_test_layer(0); + layer1.set_description("actually_layer_1"); + + const char* layer_name_2 = "VK_LAYER_RegularLayer2"; + env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(layer_name_2) + .set_description("actually_layer_2") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)), + "regular_layer_2.json") + .set_discovery_type(ManifestDiscoveryType::env_var) + .set_is_dir(true)); + auto& layer2 = env.get_test_layer(1); + layer2.set_description("actually_layer_2"); + + uint32_t count = 0; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr)); + ASSERT_EQ(count, 2U); + std::array layer_props{}; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data())); + ASSERT_EQ(count, 2U); + ASSERT_TRUE(string_eq(layer_name_1, layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_2, layer_props[1].layerName)); + ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); + ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); + // 1 then 2 + { + EnvVarWrapper inst_layers_env_var{"VK_INSTANCE_LAYERS"}; + inst_layers_env_var.add_to_list(layer_name_1); + inst_layers_env_var.add_to_list(layer_name_2); + + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + auto phys_dev = inst.GetPhysDev(); + + // Expect the env-var layer to be first + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr); + ASSERT_EQ(2U, count); + std::array enabled_layer_props{}; + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, enabled_layer_props.data()); + ASSERT_EQ(2U, count); + ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_props[0].description)); + ASSERT_TRUE(string_eq(layer2.description.c_str(), enabled_layer_props[1].description)); + ASSERT_TRUE(string_eq(layer_name_1, enabled_layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_2, enabled_layer_props[1].layerName)); + } + // 2 then 1 + { + EnvVarWrapper inst_layers_env_var{"VK_INSTANCE_LAYERS"}; + inst_layers_env_var.add_to_list(layer_name_2); + inst_layers_env_var.add_to_list(layer_name_1); + + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + auto phys_dev = inst.GetPhysDev(); + + // Expect the env-var layer to be first + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr); + ASSERT_EQ(2U, count); + std::array enabled_layer_props{}; + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, enabled_layer_props.data()); + ASSERT_EQ(2U, count); + ASSERT_TRUE(string_eq(layer2.description.c_str(), enabled_layer_props[0].description)); + ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_props[1].description)); + ASSERT_TRUE(string_eq(layer_name_2, enabled_layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_1, enabled_layer_props[1].layerName)); + } +} + +TEST(ExplicitLayers, CorrectOrderOfEnvVarEnabledLayersFromSystemLocations) { + FrameworkEnvironment env; + env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA)); + env.get_test_icd().add_physical_device({}); + + // verify layer loads successfully when setting VK_LAYER_PATH to a full filepath + const char* layer_name_1 = "VK_LAYER_RegularLayer1"; + env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(layer_name_1) + .set_description("actually_layer_1") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)), + "regular_layer_1.json")); + auto& layer1 = env.get_test_layer(0); + layer1.set_description("actually_layer_1"); + + const char* layer_name_2 = "VK_LAYER_RegularLayer2"; + env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(layer_name_2) + .set_description("actually_layer_2") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)), + "regular_layer_2.json")); + auto& layer2 = env.get_test_layer(1); + layer2.set_description("actually_layer_2"); + + uint32_t count = 0; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr)); + ASSERT_EQ(count, 2U); + std::array layer_props{}; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data())); + ASSERT_EQ(count, 2U); + ASSERT_TRUE(string_eq(layer_name_1, layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_2, layer_props[1].layerName)); + ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); + ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); + // 1 then 2 + { + EnvVarWrapper inst_layers_env_var{"VK_INSTANCE_LAYERS"}; + inst_layers_env_var.add_to_list(layer_name_1); + inst_layers_env_var.add_to_list(layer_name_2); + + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + auto phys_dev = inst.GetPhysDev(); + + // Expect the env-var layer to be first + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr); + ASSERT_EQ(2U, count); + std::array enabled_layer_props{}; + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, enabled_layer_props.data()); + ASSERT_EQ(2U, count); + ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_props[0].description)); + ASSERT_TRUE(string_eq(layer2.description.c_str(), enabled_layer_props[1].description)); + ASSERT_TRUE(string_eq(layer_name_1, enabled_layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_2, enabled_layer_props[1].layerName)); + } + // 2 then 1 + { + EnvVarWrapper inst_layers_env_var{"VK_INSTANCE_LAYERS"}; + inst_layers_env_var.add_to_list(layer_name_2); + inst_layers_env_var.add_to_list(layer_name_1); + + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + auto phys_dev = inst.GetPhysDev(); + + // Expect the env-var layer to be first + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr); + ASSERT_EQ(2U, count); + std::array enabled_layer_props{}; + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, enabled_layer_props.data()); + ASSERT_EQ(2U, count); + ASSERT_TRUE(string_eq(layer2.description.c_str(), enabled_layer_props[0].description)); + ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_props[1].description)); + ASSERT_TRUE(string_eq(layer_name_2, enabled_layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_1, enabled_layer_props[1].layerName)); + } +} + +TEST(ExplicitLayers, CorrectOrderOfApplicationEnabledLayers) { + FrameworkEnvironment env; + env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA)); + env.get_test_icd().add_physical_device({}); + + // verify layer loads successfully when setting VK_LAYER_PATH to a full filepath + const char* layer_name_1 = "VK_LAYER_RegularLayer1"; + env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(layer_name_1) + .set_description("actually_layer_1") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)), + "regular_layer_1.json")); + auto& layer1 = env.get_test_layer(0); + layer1.set_description("actually_layer_1"); + layer1.set_make_spurious_log_in_create_instance("actually_layer_1"); + + const char* layer_name_2 = "VK_LAYER_RegularLayer2"; + env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(layer_name_2) + .set_description("actually_layer_2") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)), + "regular_layer_2.json")); + auto& layer2 = env.get_test_layer(1); + layer2.set_description("actually_layer_2"); + layer2.set_make_spurious_log_in_create_instance("actually_layer_2"); + + uint32_t count = 0; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr)); + ASSERT_EQ(count, 2U); + std::array layer_props{}; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data())); + ASSERT_EQ(count, 2U); + ASSERT_TRUE(string_eq(layer_name_1, layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_2, layer_props[1].layerName)); + ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); + ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); + + // 1 then 2 + { + InstWrapper inst{env.vulkan_functions}; + inst.create_info.add_layer(layer_name_1); + inst.create_info.add_layer(layer_name_2); + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + inst.CheckCreate(); + auto phys_dev = inst.GetPhysDev(); + + // Expect the env-var layer to be first + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr); + ASSERT_EQ(2U, count); + std::array enabled_layer_props{}; + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, enabled_layer_props.data()); + ASSERT_EQ(2U, count); + ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_props[0].description)); + ASSERT_TRUE(string_eq(layer2.description.c_str(), enabled_layer_props[1].description)); + ASSERT_TRUE(string_eq(layer_name_1, enabled_layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_2, enabled_layer_props[1].layerName)); + } + // 2 then 1 + { + InstWrapper inst{env.vulkan_functions}; + inst.create_info.add_layer(layer_name_2); + inst.create_info.add_layer(layer_name_1); + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + inst.CheckCreate(); + auto phys_dev = inst.GetPhysDev(); + + // Expect the env-var layer to be first + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr); + ASSERT_EQ(2U, count); + std::array enabled_layer_props{}; + env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, enabled_layer_props.data()); + ASSERT_EQ(2U, count); + ASSERT_TRUE(string_eq(layer2.description.c_str(), enabled_layer_props[0].description)); + ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_props[1].description)); + ASSERT_TRUE(string_eq(layer_name_2, enabled_layer_props[0].layerName)); + ASSERT_TRUE(string_eq(layer_name_1, enabled_layer_props[1].layerName)); + } +} + TEST(LayerExtensions, ImplicitNoAdditionalInstanceExtension) { FrameworkEnvironment env; env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA)); diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp index 2f99a45..be663b8 100644 --- a/tests/loader_regression_tests.cpp +++ b/tests/loader_regression_tests.cpp @@ -3991,7 +3991,7 @@ TEST(ManifestDiscovery, ValidSymlink) { int res = symlink(driver_path.c_str(), symlink_path.c_str()); ASSERT_EQ(res, 0); - env.platform_shim->set_path(ManifestCategory::icd, env.get_folder(ManifestLocation::driver_env_var).location()); + env.platform_shim->set_fake_path(ManifestCategory::icd, env.get_folder(ManifestLocation::driver_env_var).location()); InstWrapper inst{env.vulkan_functions}; FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); @@ -4025,7 +4025,7 @@ TEST(ManifestDiscovery, InvalidSymlink) { ASSERT_EQ(res, 0); env.get_folder(ManifestLocation::driver).add_existing_file(symlink_name); - env.platform_shim->set_path(ManifestCategory::icd, env.get_folder(ManifestLocation::driver_env_var).location()); + env.platform_shim->set_fake_path(ManifestCategory::icd, env.get_folder(ManifestLocation::driver_env_var).location()); InstWrapper inst{env.vulkan_functions}; FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); -- 2.7.4