From 383cf8a6d3e54c42dac220692a8486e577612c61 Mon Sep 17 00:00:00 2001 From: Mark Young Date: Fri, 11 Feb 2022 12:10:17 -0700 Subject: [PATCH] Test layer pre-instance functions Add tests to verify that the 3 pre-instance functions are properly intercepted by the layer when everything is well-defined. Also include two fixes: 1) Disable environment variable value of 0 should be treated the same as not present 2) The vkEnumerateInstanceVersion override path was broken, missing a NULL check --- loader/loader.c | 23 ++-- .../layer/export_definitions/test_layer_1.def | 3 +- .../layer/export_definitions/test_layer_2.def | 6 +- tests/framework/layer/test_layer.cpp | 59 +++++++++ tests/framework/layer/test_layer.h | 3 + tests/framework/test_util.cpp | 17 ++- tests/framework/test_util.h | 4 +- tests/loader_layer_tests.cpp | 147 ++++++++++++++++++++- 8 files changed, 247 insertions(+), 15 deletions(-) diff --git a/loader/loader.c b/loader/loader.c index abde69c..3a8afa6 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -897,7 +897,7 @@ bool loader_implicit_layer_is_enabled(const struct loader_instance *inst, const // The disable_environment has priority over everything else. If it is defined, the layer is always // disabled. env_value = loader_getenv(prop->disable_env_var.name, inst); - if (env_value) { + if (NULL != env_value) { enable = false; } loader_free_getenv(env_value, inst); @@ -2471,7 +2471,7 @@ static VkResult loader_read_layer_json(const struct loader_instance *inst, struc cJSON *inst_version_json = cJSON_GetObjectItem(pre_instance, "vkEnumerateInstanceVersion"); if (inst_version_json) { char *inst_version_name = cJSON_Print(inst, inst_version_json); - if (inst_version_json) { + if (inst_version_json == NULL) { result = VK_ERROR_OUT_OF_HOST_MEMORY; goto out; } @@ -4854,16 +4854,23 @@ VkResult loader_create_instance_chain(const VkInstanceCreateInfo *pCreateInfo, c cur_gipa = (PFN_vkGetInstanceProcAddr)loader_platform_get_proc_address(lib_handle, "vkGetInstanceProcAddr"); layer_prop->functions.get_instance_proc_addr = cur_gipa; + + if (NULL == cur_gipa) { + 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); + continue; + } } else { cur_gipa = (PFN_vkGetInstanceProcAddr)loader_platform_get_proc_address(lib_handle, layer_prop->functions.str_gipa); - } - if (NULL == cur_gipa) { - 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); - continue; + if (NULL == cur_gipa) { + 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); + continue; + } } } } diff --git a/tests/framework/layer/export_definitions/test_layer_1.def b/tests/framework/layer/export_definitions/test_layer_1.def index 7a7262f..373507d 100644 --- a/tests/framework/layer/export_definitions/test_layer_1.def +++ b/tests/framework/layer/export_definitions/test_layer_1.def @@ -7,4 +7,5 @@ EXPORTS test_layer_GetInstanceProcAddr test_layer_GetDeviceProcAddr GetInstanceProcAddr - GetDeviceProcAddr \ No newline at end of file + GetDeviceProcAddr + test_override_vkGetInstanceProcAddr \ No newline at end of file diff --git a/tests/framework/layer/export_definitions/test_layer_2.def b/tests/framework/layer/export_definitions/test_layer_2.def index 13d229c..a29167b 100644 --- a/tests/framework/layer/export_definitions/test_layer_2.def +++ b/tests/framework/layer/export_definitions/test_layer_2.def @@ -9,4 +9,8 @@ EXPORTS GetInstanceProcAddr GetDeviceProcAddr vk_layerGetPhysicalDeviceProcAddr - vkNegotiateLoaderLayerInterfaceVersion \ No newline at end of file + vkNegotiateLoaderLayerInterfaceVersion + test_preinst_vkEnumerateInstanceLayerProperties + test_preinst_vkEnumerateInstanceExtensionProperties + test_preinst_vkEnumerateInstanceVersion + test_override_vkGetInstanceProcAddr \ No newline at end of file diff --git a/tests/framework/layer/test_layer.cpp b/tests/framework/layer/test_layer.cpp index 5cf1aa1..be0c4e0 100644 --- a/tests/framework/layer/test_layer.cpp +++ b/tests/framework/layer/test_layer.cpp @@ -165,6 +165,11 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkCreateInstance(const VkInstanceCreateInfo* return result; } + +VKAPI_ATTR VkResult VKAPI_CALL test_override_vkCreateInstance(const VkInstanceCreateInfo* pCreateInfo, + const VkAllocationCallbacks* pAllocator, VkInstance* pInstance) { + return VK_ERROR_INVALID_SHADER_NV; +} VKAPI_ATTR void VKAPI_CALL test_vkDestroyInstance(VkInstance instance, const VkAllocationCallbacks* pAllocator) { layer.instance_dispatch_table.DestroyInstance(instance, pAllocator); } @@ -245,6 +250,56 @@ VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL get_instance_func(VkInstance instance, // Exported functions extern "C" { #if TEST_LAYER_EXPORT_ENUMERATE_FUNCTIONS + +// Pre-instance handling functions + +FRAMEWORK_EXPORT VKAPI_ATTR VkResult VKAPI_CALL test_preinst_vkEnumerateInstanceLayerProperties( + const VkEnumerateInstanceLayerPropertiesChain* pChain, uint32_t* pPropertyCount, VkLayerProperties* pProperties) { + VkResult res = pChain->pfnNextLayer(pChain->pNextLink, pPropertyCount, pProperties); + if (nullptr == pProperties) { + *pPropertyCount = layer.reported_layer_props; + } else { + uint32_t count = layer.reported_layer_props; + if (*pPropertyCount < layer.reported_layer_props) { + count = *pPropertyCount; + res = VK_INCOMPLETE; + } + for (uint32_t i = 0; i < count; ++i) { + snprintf(pProperties[i].layerName, VK_MAX_EXTENSION_NAME_SIZE, "%02d_layer", count); + pProperties[i].specVersion = count; + pProperties[i].implementationVersion = 0xABCD0000 + count; + } + } + return res; +} + +FRAMEWORK_EXPORT VKAPI_ATTR VkResult VKAPI_CALL test_preinst_vkEnumerateInstanceExtensionProperties( + const VkEnumerateInstanceExtensionPropertiesChain* pChain, const char* pLayerName, uint32_t* pPropertyCount, + VkExtensionProperties* pProperties) { + VkResult res = pChain->pfnNextLayer(pChain->pNextLink, pLayerName, pPropertyCount, pProperties); + if (nullptr == pProperties) { + *pPropertyCount = layer.reported_extension_props; + } else { + uint32_t count = layer.reported_extension_props; + if (*pPropertyCount < layer.reported_extension_props) { + count = *pPropertyCount; + res = VK_INCOMPLETE; + } + for (uint32_t i = 0; i < count; ++i) { + snprintf(pProperties[i].extensionName, VK_MAX_EXTENSION_NAME_SIZE, "%02d_ext", count); + pProperties[i].specVersion = count; + } + } + return res; +} + +FRAMEWORK_EXPORT VKAPI_ATTR VkResult VKAPI_CALL +test_preinst_vkEnumerateInstanceVersion(const VkEnumerateInstanceVersionChain* pChain, uint32_t* pApiVersion) { + VkResult res = pChain->pfnNextLayer(pChain->pNextLink, pApiVersion); + *pApiVersion = layer.reported_instance_version; + return res; +} + FRAMEWORK_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, VkLayerProperties* pProperties) { return test_vkEnumerateInstanceLayerProperties(pPropertyCount, pProperties); @@ -272,6 +327,10 @@ FRAMEWORK_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceExtensionProper FRAMEWORK_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL test_layer_GetInstanceProcAddr(VkInstance instance, const char* pName) { return get_instance_func(instance, pName); } +FRAMEWORK_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL test_override_vkGetInstanceProcAddr(VkInstance instance, const char* pName) { + if (string_eq(pName, "vkCreateInstance")) return TO_VOID_PFN(test_override_vkCreateInstance); + return get_instance_func(instance, pName); +} #endif #if TEST_LAYER_EXPORT_LAYER_VK_GIPA diff --git a/tests/framework/layer/test_layer.h b/tests/framework/layer/test_layer.h index cb50300..7034dce 100644 --- a/tests/framework/layer/test_layer.h +++ b/tests/framework/layer/test_layer.h @@ -96,6 +96,9 @@ struct TestLayer { BUILDER_VALUE(TestLayer, std::string, unique_name, {}) BUILDER_VALUE(TestLayer, uint32_t, api_version, VK_API_VERSION_1_0) + BUILDER_VALUE(TestLayer, uint32_t, reported_layer_props, 1) + BUILDER_VALUE(TestLayer, uint32_t, reported_extension_props, 1) + BUILDER_VALUE(TestLayer, uint32_t, reported_instance_version, VK_API_VERSION_1_0) BUILDER_VALUE(TestLayer, uint32_t, implementation_version, 2) BUILDER_VALUE(TestLayer, uint32_t, min_implementation_version, 0) BUILDER_VALUE(TestLayer, std::string, description, {}) diff --git a/tests/framework/test_util.cpp b/tests/framework/test_util.cpp index dd6f466..56c9a01 100644 --- a/tests/framework/test_util.cpp +++ b/tests/framework/test_util.cpp @@ -97,6 +97,18 @@ std::string get_env_var(std::string const& name, bool report_failure) { #endif template +void print_list_of_t(std::string& out, const char* object_name, std::vector const& vec) { + if (vec.size() > 0) { + out += std::string(",\n\t\t\"") + object_name + "\": {"; + for (size_t i = 0; i < vec.size(); i++) { + if (i > 0) out += ",\t\t\t"; + out += "\n\t\t\t" + vec.at(i).get_manifest_str(); + } + out += "\n\t\t}"; + } +} + +template void print_vector_of_t(std::string& out, const char* object_name, std::vector const& vec) { if (vec.size() > 0) { out += std::string(",\n\t\t\"") + object_name + "\": ["; @@ -149,7 +161,7 @@ std::string ManifestLayer::LayerDescription::get_manifest_str() const { out += "\t\t\"api_version\": \"" + version_to_string(api_version) + "\",\n"; out += "\t\t\"implementation_version\":\"" + std::to_string(implementation_version) + "\",\n"; out += "\t\t\"description\": \"" + description + "\""; - print_vector_of_t(out, "functions", functions); + print_list_of_t(out, "functions", functions); print_vector_of_t(out, "instance_extensions", instance_extensions); print_vector_of_t(out, "device_extensions", device_extensions); if (!enable_environment.empty()) { @@ -161,9 +173,10 @@ std::string ManifestLayer::LayerDescription::get_manifest_str() const { print_vector_of_strings(out, "component_layers", component_layers); print_vector_of_strings(out, "blacklisted_layers", blacklisted_layers); print_vector_of_strings(out, "override_paths", override_paths); - print_vector_of_strings(out, "pre_instance_functions", pre_instance_functions); + print_list_of_t(out, "pre_instance_functions", pre_instance_functions); out += "\n\t}"; + return out; } diff --git a/tests/framework/test_util.h b/tests/framework/test_util.h index 4622ecf..6994883 100644 --- a/tests/framework/test_util.h +++ b/tests/framework/test_util.h @@ -541,7 +541,7 @@ struct ManifestLayer { BUILDER_VALUE(FunctionOverride, std::string, vk_func, {}) BUILDER_VALUE(FunctionOverride, std::string, override_name, {}) - std::string get_manifest_str() const { return std::string("{ \"") + vk_func + "\":\"" + override_name + "\" }"; } + std::string get_manifest_str() const { return std::string("\"") + vk_func + "\":\"" + override_name + "\""; } }; struct Extension { Extension() noexcept {} @@ -566,7 +566,7 @@ struct ManifestLayer { BUILDER_VECTOR(LayerDescription, std::string, component_layers, component_layer) BUILDER_VECTOR(LayerDescription, std::string, blacklisted_layers, blacklisted_layer) BUILDER_VECTOR(LayerDescription, std::string, override_paths, override_path) - BUILDER_VECTOR(LayerDescription, std::string, pre_instance_functions, pre_instance_function) + BUILDER_VECTOR(LayerDescription, FunctionOverride, pre_instance_functions, pre_instance_function) std::string get_manifest_str() const; VkLayerProperties get_layer_properties() const; diff --git a/tests/loader_layer_tests.cpp b/tests/loader_layer_tests.cpp index dfa7a58..4c0aa1b 100644 --- a/tests/loader_layer_tests.cpp +++ b/tests/loader_layer_tests.cpp @@ -125,7 +125,7 @@ TEST_F(ImplicitLayers, OnlyDisableEnvVar) { // don't set disable env-var, layer should load CheckLogForLayerString(*env, implicit_layer_name, true); - // set disable env-var to 0, layer should not load + // set disable env-var to 0, layer should load set_env_var(disable_env_var, "0"); CheckLogForLayerString(*env, implicit_layer_name, false); @@ -143,6 +143,151 @@ TEST_F(ImplicitLayers, OnlyDisableEnvVar) { remove_env_var(disable_env_var); } +TEST_F(ImplicitLayers, PreInstanceEnumInstLayerProps) { + const char* implicit_layer_name = "ImplicitTestLayer"; + const char* disable_env_var = "DISABLE_ME"; + + env->add_implicit_layer( + ManifestLayer{} + .set_file_format_version(ManifestVersion(1, 1, 2)) + .add_layer(ManifestLayer::LayerDescription{} + .set_name(implicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_disable_environment(disable_env_var) + .add_pre_instance_function(ManifestLayer::LayerDescription::FunctionOverride{} + .set_vk_func("vkEnumerateInstanceLayerProperties") + .set_override_name("test_preinst_vkEnumerateInstanceLayerProperties"))), + "implicit_test_layer.json"); + + uint32_t layer_props = 43; + auto& layer = env->get_test_layer(0); + layer.set_reported_layer_props(layer_props); + + uint32_t count = 0; + ASSERT_EQ(VK_SUCCESS, env->vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr)); + ASSERT_EQ(count, layer_props); + + // set disable env-var to 1, layer should not load + set_env_var(disable_env_var, "1"); + + count = 0; + ASSERT_EQ(VK_SUCCESS, env->vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr)); + ASSERT_NE(count, 0); + ASSERT_NE(count, layer_props); + + remove_env_var(disable_env_var); +} + +TEST_F(ImplicitLayers, PreInstanceEnumInstExtProps) { + const char* implicit_layer_name = "ImplicitTestLayer"; + const char* disable_env_var = "DISABLE_ME"; + + env->add_implicit_layer(ManifestLayer{} + .set_file_format_version(ManifestVersion(1, 1, 2)) + .add_layer(ManifestLayer::LayerDescription{} + .set_name(implicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_disable_environment(disable_env_var) + .add_pre_instance_function( + ManifestLayer::LayerDescription::FunctionOverride{} + .set_vk_func("vkEnumerateInstanceExtensionProperties") + .set_override_name("test_preinst_vkEnumerateInstanceExtensionProperties"))), + "implicit_test_layer.json"); + + uint32_t ext_props = 52; + auto& layer = env->get_test_layer(0); + layer.set_reported_extension_props(ext_props); + + uint32_t count = 0; + ASSERT_EQ(VK_SUCCESS, env->vulkan_functions.vkEnumerateInstanceExtensionProperties(nullptr, &count, nullptr)); + ASSERT_EQ(count, ext_props); + + // set disable env-var to 1, layer should not load + set_env_var(disable_env_var, "1"); + + count = 0; + ASSERT_EQ(VK_SUCCESS, env->vulkan_functions.vkEnumerateInstanceExtensionProperties(nullptr, &count, nullptr)); + ASSERT_NE(count, 0); + ASSERT_NE(count, ext_props); + + remove_env_var(disable_env_var); +} + +TEST_F(ImplicitLayers, PreInstanceVersion) { + env->get_test_icd().physical_devices.push_back({}); + env->get_test_icd().icd_api_version = VK_MAKE_API_VERSION(0, 1, 2, 3); + + const char* implicit_layer_name = "ImplicitTestLayer"; + const char* disable_env_var = "DISABLE_ME"; + + env->add_implicit_layer( + ManifestLayer{} + .set_file_format_version(ManifestVersion(1, 1, 2)) + .add_layer(ManifestLayer::LayerDescription{} + .set_name(implicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_api_version(VK_MAKE_API_VERSION(0, 1, 2, 3)) + .set_disable_environment(disable_env_var) + .add_pre_instance_function(ManifestLayer::LayerDescription::FunctionOverride{} + .set_vk_func("vkEnumerateInstanceVersion") + .set_override_name("test_preinst_vkEnumerateInstanceVersion"))), + "implicit_test_layer.json"); + + uint32_t layer_version = VK_MAKE_API_VERSION(1, 2, 3, 4); + auto& layer = env->get_test_layer(0); + layer.set_reported_instance_version(layer_version); + + uint32_t version = 0; + ASSERT_EQ(VK_SUCCESS, env->vulkan_functions.vkEnumerateInstanceVersion(&version)); + ASSERT_EQ(version, layer_version); + + // set disable env-var to 1, layer should not load + set_env_var(disable_env_var, "1"); + + version = 0; + ASSERT_EQ(VK_SUCCESS, env->vulkan_functions.vkEnumerateInstanceVersion(&version)); + ASSERT_NE(version, 0); + ASSERT_NE(version, layer_version); + + remove_env_var(disable_env_var); +} + +// Run with a pre-Negotiate function version of the layer so that it has to query vkCreateInstance using the +// renamed vkGetInstanceProcAddr function which returns one that intentionally fails. Then disable the +// layer and verify it works. The non-override version of vkCreateInstance in the layer also works (and is +// tested through behavior above). +TEST_F(ImplicitLayers, OverrideGetInstanceProcAddr) { + env->get_test_icd().physical_devices.push_back({}); + + const char* implicit_layer_name = "ImplicitTestLayer"; + const char* disable_env_var = "DISABLE_ME"; + + env->add_implicit_layer(ManifestLayer{} + .set_file_format_version(ManifestVersion(1, 0, 0)) + .add_layer(ManifestLayer::LayerDescription{} + .set_name(implicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_1) + .set_disable_environment(disable_env_var) + .add_function(ManifestLayer::LayerDescription::FunctionOverride{} + .set_vk_func("vkGetInstanceProcAddr") + .set_override_name("test_override_vkGetInstanceProcAddr"))), + "implicit_test_layer.json"); + + { + InstWrapper inst1{env->vulkan_functions}; + inst1.CheckCreate(VK_ERROR_INVALID_SHADER_NV); + } + + { + // set disable env-var to 1, layer should not load + set_env_var(disable_env_var, "1"); + InstWrapper inst2{env->vulkan_functions}; + inst2.CheckCreate(); + } + + remove_env_var(disable_env_var); +} + // Meta layer which contains component layers that do not exist. TEST_F(MetaLayers, InvalidComponentLayer) { const char* meta_layer_name = "VK_LAYER_MetaTestLayer"; -- 2.7.4