diff --git a/loader/loader.c b/loader/loader.c index 7a547fd37..3a9df0a0c 100644 --- a/loader/loader.c +++ b/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; } diff --git a/loader/loader_common.h b/loader/loader_common.h index 65617481c..199584a44 100644 --- a/loader/loader_common.h +++ b/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 { diff --git a/loader/trampoline.c b/loader/trampoline.c index 5fe35d21f..2340c7e89 100644 --- a/loader/trampoline.c +++ b/loader/trampoline.c @@ -1111,7 +1111,15 @@ 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); @@ -1119,8 +1127,13 @@ LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceLayerProperties(Vk } 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; diff --git a/tests/framework/layer/test_layer.cpp b/tests/framework/layer/test_layer.cpp index 66d9ff03c..575ad68fd 100644 --- a/tests/framework/layer/test_layer.cpp +++ b/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; diff --git a/tests/framework/layer/test_layer.h b/tests/framework/layer/test_layer.h index 385b294db..badb21938 100644 --- a/tests/framework/layer/test_layer.h +++ b/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) diff --git a/tests/loader_layer_tests.cpp b/tests/loader_layer_tests.cpp index 2c67ce64e..a85d21f57 100644 --- a/tests/loader_layer_tests.cpp +++ b/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); +}