Fixup layer negotiation failure

loader/loader.c

@@ -2187,6 +2187,7 @@ VkResult loader_scanned_icd_add(const struct loader_instance *inst, struct loade
                    "loader_scanned_icd_add: ICD %s doesn't support interface version compatible with loader, skip this ICD.",
                    filename);
         res = VK_ERROR_INCOMPATIBLE_DRIVER;
+        *lib_status = LOADER_LAYER_LIB_ERROR_NEGOTIATE_INTERFACE_VERSION_FAILED;
         goto out;
     }
 
@@ -2205,6 +2206,7 @@ VkResult loader_scanned_icd_add(const struct loader_instance *inst, struct loade
                        "vk_icdGetInstanceProcAddr, skip this ICD.",
                        filename, interface_vers);
             res = VK_ERROR_INCOMPATIBLE_DRIVER;
+            *lib_status = LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR;
             goto out;
         }
         // Use deprecated interface from version 0
@@ -2215,6 +2217,7 @@ VkResult loader_scanned_icd_add(const struct loader_instance *inst, struct loade
                        "\'vk_icdGetInstanceProcAddr\' from ICD %s failed.",
                        filename);
             res = VK_ERROR_INCOMPATIBLE_DRIVER;
+            *lib_status = LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR;
             goto out;
         } else {
             loader_log(inst, VULKAN_LOADER_WARN_BIT, 0,
@@ -2252,6 +2255,7 @@ VkResult loader_scanned_icd_add(const struct loader_instance *inst, struct loade
                        "loader_scanned_icd_add: Could not get \'vkCreateInstance\' via \'vk_icdGetInstanceProcAddr\' for ICD %s",
                        filename);
             res = VK_ERROR_INCOMPATIBLE_DRIVER;
+            *lib_status = LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR;
             goto out;
         }
         fp_get_inst_ext_props =
@@ -4201,6 +4205,8 @@ VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_t
             switch (lib_status) {
                 case LOADER_LAYER_LIB_NOT_LOADED:
                 case LOADER_LAYER_LIB_ERROR_FAILED_TO_LOAD:
+                case LOADER_LAYER_LIB_ERROR_NEGOTIATE_INTERFACE_VERSION_FAILED:
+                case LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR:
                     loader_log(inst, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_DRIVER_BIT, 0,
                                "loader_icd_scan: Failed loading library associated with ICD JSON %s. Ignoring this JSON",
                                icd_details[i].full_library_path);
@@ -4809,25 +4815,25 @@ bool loader_get_layer_interface_version(PFN_vkNegotiateLoaderLayerInterfaceVersi
     interface_struct->loaderLayerInterfaceVersion = 1;
     interface_struct->pNext = NULL;
 
-    if (fp_negotiate_layer_version != NULL) {
-        // Layer supports the negotiation API, so call it with the loader's
-        // latest version supported
-        interface_struct->loaderLayerInterfaceVersion = CURRENT_LOADER_LAYER_INTERFACE_VERSION;
-        VkResult result = fp_negotiate_layer_version(interface_struct);
+    // If the layer doesn't support the negotiation API, then negotiation failed.
+    if (fp_negotiate_layer_version == NULL) {
+        return false;
+    }
 
-        if (result != VK_SUCCESS) {
-            // Layer no longer supports the loader's latest interface version so
-            // fail loading the Layer
-            return false;
-        }
+    // Layer supports the negotiation API, so call it with the loader's
+    // latest version supported
+    interface_struct->loaderLayerInterfaceVersion = CURRENT_LOADER_LAYER_INTERFACE_VERSION;
+    VkResult result = fp_negotiate_layer_version(interface_struct);
+
+    if (result != VK_SUCCESS) {
+        // Layer no longer supports the loader's latest interface version so fail loading the Layer.
+        return false;
     }
 
     if (interface_struct->loaderLayerInterfaceVersion < MIN_SUPPORTED_LOADER_LAYER_INTERFACE_VERSION) {
-        // Loader no longer supports the layer's latest interface version so
-        // fail loading the layer
+        // Loader no longer supports the layer's latest interface version so fail loading the layer.
         return false;
     }
-
     return true;
 }
 
@@ -4853,9 +4859,11 @@ void setup_logical_device_enabled_layer_extensions(const struct loader_instance
             // also check if any layers support it.
             for (uint32_t j = 0; j < inst->app_activated_layer_list.count; j++) {
                 struct loader_layer_properties *layer = inst->app_activated_layer_list.list[j];
-                for (uint32_t k = 0; k < layer->device_extension_list.count; k++) {
-                    if (!strcmp(layer->device_extension_list.list[k].props.extensionName, VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) {
-                        dev->layer_extensions.ext_debug_marker_enabled = true;
+                if (layer->lib_status == LOADER_LAYER_LIB_SUCCESS_LOADED) {
+                    for (uint32_t k = 0; k < layer->device_extension_list.count; k++) {
+                        if (!strcmp(layer->device_extension_list.list[k].props.extensionName, VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) {
+                            dev->layer_extensions.ext_debug_marker_enabled = true;
+                        }
                     }
                 }
             }
@@ -5076,33 +5084,37 @@ VkResult loader_create_instance_chain(const VkInstanceCreateInfo *pCreateInfo, c
                         lib_handle, layer_prop->functions.str_negotiate_interface);
                 }
 
-                // If we can negotiate an interface version, then we can also
-                // get everything we need from the one function call, so try
-                // that first, and see if we can get all the function pointers
-                // necessary from that one call.
+                // If we can negotiate an interface version, then we can get everything we need from the one function call.
                 if (NULL != negotiate_interface) {
                     layer_prop->functions.negotiate_layer_interface = negotiate_interface;
 
-                    VkNegotiateLayerInterface interface_struct;
-
-                    if (loader_get_layer_interface_version(negotiate_interface, &interface_struct)) {
-                        // Go ahead and set the properties version to the
-                        // correct value.
-                        layer_prop->interface_version = interface_struct.loaderLayerInterfaceVersion;
-
-                        // If the interface is 2 or newer, we have access to the
-                        // new GetPhysicalDeviceProcAddr function, so grab it,
-                        // and the other necessary functions, from the
-                        // structure.
-                        if (interface_struct.loaderLayerInterfaceVersion > 1) {
-                            cur_gipa = interface_struct.pfnGetInstanceProcAddr;
-                            cur_gdpa = interface_struct.pfnGetDeviceProcAddr;
-                            cur_gpdpa = interface_struct.pfnGetPhysicalDeviceProcAddr;
-                            if (cur_gipa != NULL) {
-                                // We've set the functions, so make sure we
-                                // don't do the unnecessary calls later.
-                                functions_in_interface = true;
-                            }
+                    VkNegotiateLayerInterface interface_struct = {0};
+                    bool compatible = loader_get_layer_interface_version(negotiate_interface, &interface_struct);
+                    // When a layer supports vkNegotiateLoaderLayerInterfaceVersion but fails when the loader calls it, skip the
+                    // layer as it isn't compatible.
+                    if (!compatible) {
+                        loader_log(inst, VULKAN_LOADER_INFO_BIT | VULKAN_LOADER_LAYER_BIT, 0,
+                                   "loader_create_instance_chain: Failed to negotiate a compatible interface version with layer "
+                                   "\"%s\", skipping",
+                                   layer_prop->lib_name);
+                        layer_prop->lib_status = LOADER_LAYER_LIB_ERROR_NEGOTIATE_INTERFACE_VERSION_FAILED;
+                        continue;
+                    }
+
+                    // Go ahead and set the properties version to the correct value.
+                    layer_prop->interface_version = interface_struct.loaderLayerInterfaceVersion;
+
+                    // If the interface is 2 or newer, we have access to the
+                    // new GetPhysicalDeviceProcAddr function, so grab it,
+                    // and the other necessary functions, from the structure.
+                    if (interface_struct.loaderLayerInterfaceVersion > 1) {
+                        cur_gipa = interface_struct.pfnGetInstanceProcAddr;
+                        cur_gdpa = interface_struct.pfnGetDeviceProcAddr;
+                        cur_gpdpa = interface_struct.pfnGetPhysicalDeviceProcAddr;
+                        if (cur_gipa != NULL) {
+                            // We've set the functions, so make sure we
+                            // don't do the unnecessary calls later.
+                            functions_in_interface = true;
                         }
                     }
                 }
@@ -5119,6 +5131,7 @@ VkResult loader_create_instance_chain(const VkInstanceCreateInfo *pCreateInfo, c
                                 loader_log(inst, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_LAYER_BIT, 0,
                                            "loader_create_instance_chain: Failed to find \'vkGetInstanceProcAddr\' in layer \"%s\"",
                                            layer_prop->lib_name);
+                                layer_prop->lib_status = LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR;
                                 continue;
                             }
                         } else {
@@ -5129,6 +5142,7 @@ VkResult loader_create_instance_chain(const VkInstanceCreateInfo *pCreateInfo, c
                                 loader_log(inst, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_LAYER_BIT, 0,
                                            "loader_create_instance_chain: Failed to find \'%s\' in layer \"%s\"",
                                            layer_prop->functions.str_gipa, layer_prop->lib_name);
+                                layer_prop->lib_status = LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR;
                                 continue;
                             }
                         }
@@ -5218,6 +5232,15 @@ VkResult loader_create_instance_chain(const VkInstanceCreateInfo *pCreateInfo, c
                     loader_log(inst, log_flag, 0, "Requested layer \"%s\" failed to load%c", exp_layer_prop->info.layerName,
                                ending);
                     break;
+                case LOADER_LAYER_LIB_ERROR_NEGOTIATE_INTERFACE_VERSION_FAILED:
+                    loader_log(inst, log_flag, 0,
+                               "Requested layer \"%s\" failed to negotiate a compatible interface version with layer%c",
+                               exp_layer_prop->info.layerName, ending);
+                    break;
+                case LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR:
+                    loader_log(inst, log_flag, 0, "Requested layer \"%s\" is missing vkGetInstanceProcAddr %c",
+                               exp_layer_prop->info.layerName, ending);
+                    break;
                 case LOADER_LAYER_LIB_SUCCESS_LOADED:
                 case LOADER_LAYER_LIB_ERROR_OUT_OF_MEMORY:
                     // Shouldn't be able to reach this but if it is, best to report a debug
@@ -5457,9 +5480,9 @@ VkResult loader_create_device_chain(VkPhysicalDevice pd, const VkDeviceCreateInf
                 continue;
             }
 
-            // Skip the layer if the handle is NULL - this is likely because the library failed to load but wasn't removed from
-            // the list.
-            if (!lib_handle) {
+            // Skip the layer if the handle is NULL or if the lib_status isn't set to success - this is likely because the library
+            // failed to load but wasn't removed from the list.
+            if (!lib_handle || layer_prop->lib_status != LOADER_LAYER_LIB_SUCCESS_LOADED) {
                 continue;
             }
 

loader/loader_common.h

@@ -178,6 +178,8 @@ enum loader_layer_library_status {
     LOADER_LAYER_LIB_ERROR_WRONG_BIT_TYPE = 20,
     LOADER_LAYER_LIB_ERROR_FAILED_TO_LOAD = 21,
     LOADER_LAYER_LIB_ERROR_OUT_OF_MEMORY = 22,
+    LOADER_LAYER_LIB_ERROR_NEGOTIATE_INTERFACE_VERSION_FAILED = 23,
+    LOADER_LAYER_LIB_ERROR_UNABLE_TO_FIND_VK_GET_INSTANCE_PROC_ADDR = 24,
 };
 
 enum layer_type_flags {

loader/trampoline.c

@@ -1111,16 +1111,29 @@ LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceLayerProperties(Vk
 
     const struct loader_instance *inst = phys_dev->this_instance;
 
-    uint32_t count = inst->app_activated_layer_list.count;
+    // Only count layers that successfully loaded, or had nothing to load (like meta layers)
+    uint32_t count = 0;
+    for (uint32_t i = 0; i < inst->app_activated_layer_list.count; i++) {
+        if (inst->app_activated_layer_list.list[i]->lib_status == LOADER_LAYER_LIB_NOT_LOADED ||
+            inst->app_activated_layer_list.list[i]->lib_status == LOADER_LAYER_LIB_SUCCESS_LOADED) {
+            count++;
+        }
+    }
+
     if (count == 0 || pProperties == NULL) {
         *pPropertyCount = count;
         loader_platform_thread_unlock_mutex(&loader_lock);
         return VK_SUCCESS;
     }
 
     copy_size = (*pPropertyCount < count) ? *pPropertyCount : count;
-    for (uint32_t i = 0; i < copy_size; i++) {
-        memcpy(&pProperties[i], &(inst->app_activated_layer_list.list[i]->info), sizeof(VkLayerProperties));
+    uint32_t propertiesIter = 0;
+    for (uint32_t i = 0; i < inst->app_activated_layer_list.count; i++) {
+        if (propertiesIter < copy_size && (inst->app_activated_layer_list.list[i]->lib_status == LOADER_LAYER_LIB_NOT_LOADED ||
+                                           inst->app_activated_layer_list.list[i]->lib_status == LOADER_LAYER_LIB_SUCCESS_LOADED)) {
+            memcpy(&pProperties[propertiesIter], &(inst->app_activated_layer_list.list[i]->info), sizeof(VkLayerProperties));
+            propertiesIter++;
+        }
     }
     *pPropertyCount = copy_size;
 

tests/framework/layer/test_layer.cpp

@@ -1062,6 +1062,10 @@ FRAMEWORK_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL vk_layerGetPhysicalDev
 
 EXPORT_NEGOTIATE_FUNCTION VKAPI_ATTR VkResult VKAPI_CALL
 vkNegotiateLoaderLayerInterfaceVersion(VkNegotiateLayerInterface* pVersionStruct) {
+    if (layer.fail_negotiateLoaderLayerInterfaceVersion) {
+        return VK_ERROR_INITIALIZATION_FAILED;
+    }
+
     if (pVersionStruct) {
         if (pVersionStruct->loaderLayerInterfaceVersion < layer.min_implementation_version) {
             return VK_ERROR_INITIALIZATION_FAILED;

tests/framework/layer/test_layer.h

@@ -112,6 +112,8 @@ struct TestLayer {
     // Some layers may try to change the API version during instance creation - we should allow testing of such behavior
     BUILDER_VALUE_WITH_DEFAULT(uint32_t, alter_api_version, VK_API_VERSION_1_0)
 
+    BUILDER_VALUE(bool, fail_negotiateLoaderLayerInterfaceVersion)
+
     BUILDER_VECTOR(std::string, alternative_function_names, alternative_function_name)
 
     BUILDER_VECTOR(Extension, instance_extensions, instance_extension)

tests/loader_layer_tests.cpp

@@ -5830,3 +5830,21 @@ TEST(TestLayers, AllowFilterWithConditionallyImplicitLayerWithOverrideLayer) {
         ASSERT_NO_FATAL_FAILURE(inst.GetActiveLayers(inst.GetPhysDev(), 0));
     }
 }
+
+TEST(TestLayers, LayerFailsNegotiateLoaderLayerInterfaceVersion) {
+    FrameworkEnvironment env;
+    env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device({});
+
+    env.add_implicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                             .set_name("VK_LAYER_test_layer")
+                                                             .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)
+                                                             .set_disable_environment("DISABLE_ME")));
+    env.get_test_layer().set_fail_negotiateLoaderLayerInterfaceVersion(true);
+
+    env.GetLayerProperties(1);
+
+    InstWrapper inst{env.vulkan_functions};
+    inst.CheckCreate();
+
+    inst.GetActiveLayers(inst.GetPhysDev(), 0);
+}