Make correct layer be used when duplicates are present
authorCharles Giessen <charles@lunarg.com>
Wed, 29 Mar 2023 20:05:44 +0000 (14:05 -0600)
committerCharles Giessen <46324611+charles-lunarg@users.noreply.github.com>
Fri, 31 Mar 2023 22:22:06 +0000 (16:22 -0600)
When there are multiple versions of the same layer found through environment variables
or from implicit layer locations, the loader would use the *last* one that appeared rather
than the first, causing surprising behavior. This commit fixes that by checking to make
sure that layers do not already exist in the list of layers to be enabled before adding a
layer to the list, preventing duplicates from appearing in the list.

This commit adds a check in vkEnumerateInstanceLayerProperties to skip over layers that do
wish to intercept the function. The missing check would only cause a spurious log message
which said that the loader was unable to load a symbol names "" (as in an empty string)
from the layer.

This commit also amends the test framework with the following fixes:
* Create Layer JSON manifests that contain multiple layers
* Only add layers to the registry if they are generic (meant to simulate installed layers)
* Allow layers to print messages during vkCreateInstance - a mechanism to double check that
only the correct layer(s) are loaded
* Fixes env-var added layers not using full paths
* Only have 1 XDG env-var set, making all other XDG env-var have an empty string
* Set the XDG env-vars even on macOS (github actions seems to have them set)

12 files changed:
loader/loader.c
loader/loader.h
loader/loader_environment.c
loader/trampoline.c
tests/framework/layer/test_layer.cpp
tests/framework/layer/test_layer.h
tests/framework/test_environment.cpp
tests/framework/test_environment.h
tests/framework/test_util.cpp
tests/framework/test_util.h
tests/loader_layer_tests.cpp
tests/loader_testing_main.cpp

index 01507e6..7705346 100644 (file)
@@ -380,7 +380,7 @@ static struct loader_layer_properties *loader_find_layer_property(const char *na
 }
 
 // Search the given layer list for a layer matching the given layer name
-static bool loader_find_layer_name_in_list(const char *name, const struct loader_layer_list *layer_list) {
+bool loader_find_layer_name_in_list(const char *name, const struct loader_layer_list *layer_list) {
     if (NULL == layer_list) {
         return false;
     }
@@ -985,7 +985,7 @@ bool loader_implicit_layer_is_enabled(const struct loader_instance *inst, const
 }
 
 // Check the individual implicit layer for the enable/disable environment variable settings.  Only add it after
-// every check has passed indicating it should be used.
+// every check has passed indicating it should be used, including making sure a layer of the same name hasn't already been added.
 static VkResult loader_add_implicit_layer(const struct loader_instance *inst, const struct loader_layer_properties *prop,
                                           const struct loader_envvar_filter *enable_filter,
                                           const struct loader_envvar_disable_layers_filter *disable_filter,
@@ -994,6 +994,11 @@ static VkResult loader_add_implicit_layer(const struct loader_instance *inst, co
     VkResult result = VK_SUCCESS;
     if (loader_implicit_layer_is_enabled(inst, enable_filter, disable_filter, prop)) {
         if (0 == (prop->type_flags & VK_LAYER_TYPE_FLAG_META_LAYER)) {
+            // Make sure the layer isn't already in the output_list, skip adding it if it is.
+            if (loader_find_layer_name_in_list(&prop->info.layerName[0], target_list)) {
+                return result;
+            }
+
             result = loader_add_layer_properties_to_list(inst, target_list, 1, prop);
             if (result == VK_ERROR_OUT_OF_HOST_MEMORY) return result;
             if (NULL != expanded_target_list) {
index a73633c..f6ea880 100644 (file)
@@ -101,6 +101,7 @@ bool has_vk_extension_property_array(const VkExtensionProperties *vk_ext_prop, c
                                      const VkExtensionProperties *ext_array);
 bool has_vk_extension_property(const VkExtensionProperties *vk_ext_prop, const struct loader_extension_list *ext_list);
 
+bool loader_find_layer_name_in_list(const char *name, const struct loader_layer_list *layer_list);
 VkResult loader_add_layer_properties_to_list(const struct loader_instance *inst, struct loader_layer_list *list,
                                              uint32_t prop_list_count, const struct loader_layer_properties *props);
 void loader_free_layer_properties(const struct loader_instance *inst, struct loader_layer_properties *layer_properties);
index f4205f4..5d82d50 100644 (file)
@@ -497,7 +497,9 @@ VkResult loader_add_environment_layers(struct loader_instance *inst, const enum
 
         // If we are supposed to filter through all layers, we need to compare the layer name against the filter.
         // This can override the disable above, so we want to do it second.
-        if (check_name_matches_filter_environment_var(inst, source_prop->info.layerName, enable_filter)) {
+        // Also make sure the layer isn't already in the output_list, skip adding it if it is.
+        if (check_name_matches_filter_environment_var(inst, source_prop->info.layerName, enable_filter) &&
+            !loader_find_layer_name_in_list(source_prop->info.layerName, target_list)) {
             adding = true;
             // Only way is_substring is true is if there are enable variables.  If that's the case, and we're past the
             // above, we should indicate that it was forced on in this way.
@@ -506,9 +508,11 @@ VkResult loader_add_environment_layers(struct loader_instance *inst, const enum
         } 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)) {
+                    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;
                     }
index f264e53..944a48b 100644 (file)
@@ -264,6 +264,11 @@ LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateInstanceLayerProperties(
 
     // Prepend layers onto the chain if they implement this entry point
     for (uint32_t i = 0; i < layers.count; ++i) {
+        // Skip this layer if it doesn't expose the entry-point
+        if (layers.list[i].pre_instance_functions.enumerate_instance_layer_properties[0] == '\0') {
+            continue;
+        }
+
         loader_platform_dl_handle layer_lib = loader_platform_open_library(layers.list[i].lib_name);
         if (layer_lib == NULL) {
             loader_log(NULL, VULKAN_LOADER_WARN_BIT, 0, "%s: Unable to load implicit layer library \"%s\"", __FUNCTION__,
index d1cd26a..35fbba4 100644 (file)
@@ -210,6 +210,22 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkCreateInstance(const VkInstanceCreateInfo*
         }
     }
 
+    if (!layer.make_spurious_log_in_create_instance.empty()) {
+        auto* chain = reinterpret_cast<const VkBaseInStructure*>(pCreateInfo->pNext);
+        while (chain) {
+            if (chain->sType == VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT) {
+                auto* debug_messenger = reinterpret_cast<const VkDebugUtilsMessengerCreateInfoEXT*>(chain);
+                VkDebugUtilsMessengerCallbackDataEXT data{};
+                data.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CALLBACK_DATA_EXT;
+                data.pMessage = layer.make_spurious_log_in_create_instance.c_str();
+                debug_messenger->pfnUserCallback(VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT,
+                                                 VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT, &data, debug_messenger->pUserData);
+            }
+
+            chain = chain->pNext;
+        }
+    }
+
     return result;
 }
 
index c0f0012..c3c1b9a 100644 (file)
@@ -160,6 +160,8 @@ struct TestLayer {
         return nullptr;
     }
 
+    // Allows distinguishing different layers (that use the same binary)
+    BUILDER_VALUE(TestLayer, std::string, make_spurious_log_in_create_instance, "")
     BUILDER_VALUE(TestLayer, bool, do_spurious_allocations_in_create_instance, false)
     void* spurious_instance_memory_allocation = nullptr;
     BUILDER_VALUE(TestLayer, bool, do_spurious_allocations_in_create_device, false)
index 11e64f7..8081f6d 100644 (file)
@@ -198,7 +198,6 @@ void InstWrapper::CheckCreateWithInfo(InstanceCreateInfo& create_info, VkResult
     ASSERT_EQ(result_to_check, functions->vkCreateInstance(create_info.get(), callbacks, &inst));
 }
 
-
 std::vector<VkPhysicalDevice> InstWrapper::GetPhysDevs(uint32_t phys_dev_count, VkResult result_to_check) {
     uint32_t physical_count = phys_dev_count;
     std::vector<VkPhysicalDevice> physical_devices;
@@ -478,8 +477,7 @@ void FrameworkEnvironment::add_layer_impl(TestLayerDetails layer_details, Manife
             if (layer_details.is_dir) {
                 env_var_vk_layer_paths.add_to_list(fs_ptr->location().str());
             } else {
-                env_var_vk_layer_paths.add_to_list(fs_ptr->location().str());
-                env_var_vk_layer_paths.add_to_list(layer_details.json_name);
+                env_var_vk_layer_paths.add_to_list((fs_ptr->location() / layer_details.json_name).str());
             }
             break;
         case (ManifestDiscoveryType::add_env_var):
@@ -500,11 +498,10 @@ void FrameworkEnvironment::add_layer_impl(TestLayerDetails layer_details, Manife
     auto& folder = *fs_ptr;
     size_t new_layers_start = layers.size();
     for (auto& layer : layer_details.layer_manifest.layers) {
-        size_t cur_layer_index = layers.size();
         if (!layer.lib_path.str().empty()) {
-            std::string new_layer_name = layer.name + "_" + std::to_string(cur_layer_index) + "_" + layer.lib_path.filename().str();
+            std::string layer_binary_name = layer.lib_path.filename().str() + "_" + std::to_string(layers.size());
 
-            auto new_layer_location = folder.copy_file(layer.lib_path, new_layer_name);
+            auto new_layer_location = folder.copy_file(layer.lib_path, layer_binary_name);
 
             // Don't load the layer binary if using any of the wrap objects layers, since it doesn't export the same interface
             // functions
@@ -517,8 +514,12 @@ void FrameworkEnvironment::add_layer_impl(TestLayerDetails layer_details, Manife
         }
     }
     if (layer_details.discovery_type != ManifestDiscoveryType::none) {
+        // Write a manifest file to a folder as long as the discovery type isn't none
         auto layer_loc = folder.write_manifest(layer_details.json_name, layer_details.layer_manifest.get_manifest_str());
-        platform_shim->add_manifest(category, layer_loc);
+        // only add the manifest to the registry if its a system location (as if it was installed)
+        if (layer_details.discovery_type == ManifestDiscoveryType::generic) {
+            platform_shim->add_manifest(category, layer_loc);
+        }
         for (size_t i = new_layers_start; i < layers.size(); i++) {
             layers.at(i).manifest_path = layer_loc;
         }
index 6043250..c671700 100644 (file)
@@ -470,6 +470,7 @@ struct TestICDDetails {
     }
     BUILDER_VALUE(TestICDDetails, ManifestICD, icd_manifest, {});
     BUILDER_VALUE(TestICDDetails, std::string, json_name, "test_icd");
+    // Uses the json_name without modification - default is to append _1 in the json file to distinguish drivers
     BUILDER_VALUE(TestICDDetails, bool, disable_icd_inc, false);
     BUILDER_VALUE(TestICDDetails, ManifestDiscoveryType, discovery_type, ManifestDiscoveryType::generic);
     BUILDER_VALUE(TestICDDetails, bool, is_fake, false);
index 647463c..dd38089 100644 (file)
@@ -203,10 +203,10 @@ std::string ManifestLayer::get_manifest_str() const {
         out += "\t\"layer\": ";
         out += layers.at(0).get_manifest_str() + "\n";
     } else {
-        out += "\"\tlayers\": [";
+        out += "\t\"layers\": [";
         for (size_t i = 0; i < layers.size(); i++) {
             if (i > 0) out += ",";
-            out += "\n" + layers.at(0).get_manifest_str();
+            out += "\n" + layers.at(i).get_manifest_str();
         }
         out += "\n]";
     }
index 22f60f2..42a2da9 100644 (file)
@@ -131,15 +131,12 @@ struct EnvVarWrapper {
     std::string name;
     std::string cur_value;
 
-#if defined(WIN32)
     void set_env_var();
     void remove_env_var() const;
+#if defined(WIN32)
     // Environment variable list separator - not for filesystem paths
     const char OS_ENV_VAR_LIST_SEPARATOR = ';';
-
 #elif defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__)
-    void set_env_var();
-    void remove_env_var() const;
     // Environment variable list separator - not for filesystem paths
     const char OS_ENV_VAR_LIST_SEPARATOR = ':';
 #endif
@@ -148,11 +145,7 @@ struct EnvVarWrapper {
 // get_env_var() - returns a std::string of `name`. if report_failure is true, then it will log to stderr that it didn't find the
 //     env-var
 // NOTE: This is only intended for test framework code, all test code MUST use EnvVarWrapper
-#if defined(WIN32)
-std::string get_env_var(std::string const& name, bool report_failure = true);
-#elif defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__)
 std::string get_env_var(std::string const& name, bool report_failure = true);
-#endif
 
 // Windows specific error handling logic
 #if defined(WIN32)
index a0c9bff..5268f75 100644 (file)
@@ -962,6 +962,69 @@ TEST(ImplicitLayers, EnableAndDisableWithFilter) {
     ASSERT_FALSE(env.debug_log.find_prefix_then_postfix(implicit_layer_name_3, "disabled because name matches filter of env var"));
 }
 
+// Add 2 implicit layers with the same layer name and expect only one to be loaded.
+TEST(ImplicitLayers, DuplicateLayers) {
+    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* same_layer_name_1 = "VK_LAYER_RegularLayer1";
+    env.add_implicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                                          .set_name(same_layer_name_1)
+                                                                          .set_description("actually_layer_1")
+                                                                          .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)
+                                                                          .set_disable_environment("if_you_can")),
+                                            "regular_layer_1.json")
+                               // use override folder as just a folder and manually add it to the implicit layer search paths
+                               .set_discovery_type(ManifestDiscoveryType::override_folder));
+    auto& layer1 = env.get_test_layer(0);
+    layer1.set_description("actually_layer_1");
+    layer1.set_make_spurious_log_in_create_instance("actually_layer_1");
+#if defined(WIN32)
+    env.platform_shim->add_manifest(ManifestCategory::implicit_layer, env.get_folder(ManifestLocation::override_layer).location());
+#elif defined(__linux__) || defined(__APPLE__)
+    env.platform_shim->redirect_path(fs::path("/etc/vulkan/implicit_layer.d"),
+                                     env.get_folder(ManifestLocation::override_layer).location());
+#endif
+
+    env.add_implicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                                          .set_name(same_layer_name_1)
+                                                                          .set_description("actually_layer_2")
+                                                                          .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)
+                                                                          .set_disable_environment("if_you_can")),
+                                            "regular_layer_1.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<VkLayerProperties, 2> layer_props{};
+    ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data()));
+    ASSERT_EQ(count, 2U);
+    ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName));
+    ASSERT_TRUE(string_eq(same_layer_name_1, 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));
+
+    InstWrapper inst{env.vulkan_functions};
+    FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+    inst.CheckCreate();
+    auto phys_dev = inst.GetPhysDev();
+
+    env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+    ASSERT_EQ(1U, count);
+    VkLayerProperties enabled_layer_prop{};
+    env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+    ASSERT_EQ(1U, count);
+    ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+    ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+    ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+    ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+}
+
 // Meta layer which contains component layers that do not exist.
 TEST(MetaLayers, InvalidComponentLayer) {
     FrameworkEnvironment env;
@@ -2020,6 +2083,36 @@ TEST(LayerCreateInstance, GetPhysicalDeviceProperties2KHR) {
     inst.CheckCreate();
 }
 
+TEST(ExplicitLayers, MultipleLayersInSingleManifest) {
+    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* regular_layer_name_1 = "VK_LAYER_RegularLayer1";
+    const char* regular_layer_name_2 = "VK_LAYER_RegularLayer2";
+    const char* regular_layer_name_3 = "VK_LAYER_RegularLayer3";
+    env.add_explicit_layer(TestLayerDetails(
+        ManifestLayer{}
+            .set_file_format_version(ManifestVersion{1, 0, 1})
+            .add_layer(
+                ManifestLayer::LayerDescription{}.set_name(regular_layer_name_1).set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2))
+            .add_layer(
+                ManifestLayer::LayerDescription{}.set_name(regular_layer_name_2).set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2))
+            .add_layer(
+                ManifestLayer::LayerDescription{}.set_name(regular_layer_name_3).set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+        "multi_layer_manifest.json"));
+
+    uint32_t count = 0;
+    ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr));
+    ASSERT_EQ(count, 3U);
+    std::array<VkLayerProperties, 3> layer_props;
+    ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data()));
+    ASSERT_TRUE(string_eq(regular_layer_name_1, layer_props[0].layerName));
+    ASSERT_TRUE(string_eq(regular_layer_name_2, layer_props[1].layerName));
+    ASSERT_TRUE(string_eq(regular_layer_name_3, layer_props[2].layerName));
+}
+
 TEST(ExplicitLayers, WrapObjects) {
     FrameworkEnvironment env;
     env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA));
@@ -2177,6 +2270,204 @@ TEST(ExplicitLayers, VkLayerPathEnvVar) {
     }
 }
 
+TEST(ExplicitLayers, DuplicateLayersInVK_LAYER_PATH) {
+    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* same_layer_name_1 = "VK_LAYER_RegularLayer1";
+    env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                                          .set_name(same_layer_name_1)
+                                                                          .set_description("actually_layer_1")
+                                                                          .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+                                            "regular_layer_1.json")
+                               // use override folder as just a folder and manually set the VK_LAYER_PATH env-var to it
+                               .set_discovery_type(ManifestDiscoveryType::override_folder)
+                               .set_is_dir(true));
+    auto& layer1 = env.get_test_layer(0);
+    layer1.set_description("actually_layer_1");
+    layer1.set_make_spurious_log_in_create_instance("actually_layer_1");
+    env.env_var_vk_layer_paths.add_to_list(env.get_folder(ManifestLocation::override_layer).location().str());
+
+    env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                                          .set_name(same_layer_name_1)
+                                                                          .set_description("actually_layer_2")
+                                                                          .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+                                            "regular_layer_1.json")
+                               .set_discovery_type(ManifestDiscoveryType::env_var)
+                               .set_is_dir(true));
+    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<VkLayerProperties, 2> layer_props{};
+    ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data()));
+    ASSERT_EQ(count, 2U);
+    ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName));
+    ASSERT_TRUE(string_eq(same_layer_name_1, 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));
+    {
+        InstWrapper inst{env.vulkan_functions};
+        inst.create_info.add_layer(same_layer_name_1);
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate();
+        auto phys_dev = inst.GetPhysDev();
+
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+        ASSERT_EQ(1U, count);
+        VkLayerProperties enabled_layer_prop{};
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+        ASSERT_EQ(1U, count);
+        ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+        ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+        ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+        ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+    }
+    env.debug_log.clear();
+
+    {
+        EnvVarWrapper layers_enable_env_var{"VK_INSTANCE_LAYERS", same_layer_name_1};
+        InstWrapper inst{env.vulkan_functions};
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate();
+        auto phys_dev = inst.GetPhysDev();
+
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+        ASSERT_EQ(1U, count);
+        VkLayerProperties enabled_layer_prop{};
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+        ASSERT_EQ(1U, count);
+        ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+        ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+        ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+        ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+    }
+    env.debug_log.clear();
+
+    {
+        EnvVarWrapper layers_enable_env_var{"VK_LOADER_LAYERS_ENABLE", same_layer_name_1};
+        InstWrapper inst{env.vulkan_functions};
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate();
+        auto phys_dev = inst.GetPhysDev();
+
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+        ASSERT_EQ(1U, count);
+        VkLayerProperties enabled_layer_prop{};
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+        ASSERT_EQ(1U, count);
+        ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+        ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+        ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+        ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+    }
+    env.debug_log.clear();
+}
+
+TEST(ExplicitLayers, DuplicateLayersInVK_ADD_LAYER_PATH) {
+    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* same_layer_name_1 = "VK_LAYER_RegularLayer1";
+    env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                                          .set_name(same_layer_name_1)
+                                                                          .set_description("actually_layer_1")
+                                                                          .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+                                            "regular_layer_1.json")
+                               // use override folder as just a folder and manually set the VK_ADD_LAYER_PATH env-var to it
+                               .set_discovery_type(ManifestDiscoveryType::override_folder)
+                               .set_is_dir(true));
+    auto& layer1 = env.get_test_layer(0);
+    layer1.set_description("actually_layer_1");
+    layer1.set_make_spurious_log_in_create_instance("actually_layer_1");
+    env.add_env_var_vk_layer_paths.add_to_list(env.get_folder(ManifestLocation::override_layer).location().str());
+
+    env.add_explicit_layer(TestLayerDetails(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                                          .set_name(same_layer_name_1)
+                                                                          .set_description("actually_layer_2")
+                                                                          .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+                                            "regular_layer_1.json")
+                               .set_discovery_type(ManifestDiscoveryType::add_env_var)
+                               .set_is_dir(true));
+    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<VkLayerProperties, 2> layer_props{};
+    ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, layer_props.data()));
+    ASSERT_EQ(count, 2U);
+    ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName));
+    ASSERT_TRUE(string_eq(same_layer_name_1, 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));
+    {
+        InstWrapper inst{env.vulkan_functions};
+        inst.create_info.add_layer(same_layer_name_1);
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate();
+        auto phys_dev = inst.GetPhysDev();
+
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+        ASSERT_EQ(1U, count);
+        VkLayerProperties enabled_layer_prop{};
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+        ASSERT_EQ(1U, count);
+        ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+        ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+        ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+        ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+    }
+    env.debug_log.clear();
+
+    {
+        EnvVarWrapper layers_enable_env_var{"VK_INSTANCE_LAYERS", same_layer_name_1};
+        InstWrapper inst{env.vulkan_functions};
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate();
+        auto phys_dev = inst.GetPhysDev();
+
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+        ASSERT_EQ(1U, count);
+        VkLayerProperties enabled_layer_prop{};
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+        ASSERT_EQ(1U, count);
+        ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+        ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+        ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+        ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+    }
+    env.debug_log.clear();
+
+    {
+        EnvVarWrapper layers_enable_env_var{"VK_LOADER_LAYERS_ENABLE", same_layer_name_1};
+        InstWrapper inst{env.vulkan_functions};
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate();
+        auto phys_dev = inst.GetPhysDev();
+
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, nullptr);
+        ASSERT_EQ(1U, count);
+        VkLayerProperties enabled_layer_prop{};
+        env.vulkan_functions.vkEnumerateDeviceLayerProperties(phys_dev, &count, &enabled_layer_prop);
+        ASSERT_EQ(1U, count);
+        ASSERT_TRUE(string_eq(same_layer_name_1, enabled_layer_prop.layerName));
+        ASSERT_TRUE(string_eq(layer1.description.c_str(), enabled_layer_prop.description));
+        ASSERT_TRUE(env.debug_log.find("actually_layer_1"));
+        ASSERT_FALSE(env.debug_log.find("actually_layer_2"));
+    }
+    env.debug_log.clear();
+}
+
 TEST(LayerExtensions, ImplicitNoAdditionalInstanceExtension) {
     FrameworkEnvironment env;
     env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA));
index 7e37aa0..9ddb601 100644 (file)
@@ -88,13 +88,12 @@ int main(int argc, char** argv) {
     vk_loader_debug_env_var.remove_value();
     vk_loader_disable_inst_ext_filter_env_var.remove_value();
 
-#if defined(__linux__) || defined(__FreeBSD__) || defined(__OpenBSD__)
-    EnvVarWrapper xdg_config_home_env_var{"XDG_CONFIG_HOME", "/etc"};
-    EnvVarWrapper xdg_config_dirs_env_var{"XDG_CONFIG_DIRS", "/etc"};
-    EnvVarWrapper xdg_data_home_env_var{"XDG_DATA_HOME", "/etc"};
-    EnvVarWrapper xdg_data_dirs_env_var{"XDG_DATA_DIRS", "/etc"};
-#endif
 #if defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__)
+    // Set only one of the 4 XDG variables to /etc, let everything else be empty
+    EnvVarWrapper xdg_config_home_env_var{"XDG_CONFIG_HOME", "/etc"};
+    EnvVarWrapper xdg_config_dirs_env_var{"XDG_CONFIG_DIRS", ""};
+    EnvVarWrapper xdg_data_home_env_var{"XDG_DATA_HOME", ""};
+    EnvVarWrapper xdg_data_dirs_env_var{"XDG_DATA_DIRS", ""};
     EnvVarWrapper home_env_var{"HOME", "/home/fake_home"};
 #endif
     ::testing::InitGoogleTest(&argc, argv);