Only deduplicate driver manifests
authorCharles Giessen <charles@lunarg.com>
Mon, 17 Oct 2022 21:26:47 +0000 (15:26 -0600)
committerCharles Giessen <46324611+charles-lunarg@users.noreply.github.com>
Tue, 18 Oct 2022 23:23:54 +0000 (17:23 -0600)
Code which was originally meant to deduplicate drivers that moved from the old
registry HKEY_LOCAL_MACHINE/SOFTWARE/Khronos/Vulkan/Drivers to PnP locations
was incorrectly applied to layers as well. The issue is that multiple layers
can share the same json file name but not actually contain the same manifest.
Worse, the first layer found would be assumed to be correct, even the file had
been deleted from the file system.

loader/loader_windows.c
tests/loader_regression_tests.cpp

index 4015acbb8f71541335785ec1562d0a9f5026baa5..068e21147081b0a0ea37290287e65e8657426b1d 100644 (file)
@@ -525,8 +525,8 @@ VkResult windows_get_registry_files(const struct loader_instance *inst, char *lo
                                 foundDuplicate = true;
                             }
                         }
-
-                        if (foundDuplicate == false) {
+                        // Only skip if we are adding a driver and a duplicate was found
+                        if (!is_driver || (is_driver && foundDuplicate == false)) {
                             // Add the new entry to the list.
                             (void)snprintf(*reg_data + strlen(*reg_data), name_size + 2, "%c%s", PATH_SEPARATOR, name);
                             found = true;
index 4bf72eb1f057adb8140ececa8f03c7b3e5bb9404..7390596bdca9d77a8a4cca9bf5daccf2221d4f3b 100644 (file)
@@ -3527,4 +3527,47 @@ TEST(AppPackageDriverDiscovery, AppPackageTest) {
     InstWrapper inst{env.vulkan_functions};
     inst.CheckCreate();
 }
+
+// Make sure that stale layer manifests (path to nonexistant file) which have the same name as real manifests don't cause the real
+// manifests to be skipped. Stale registry entries happen when a registry is written on layer/driver installation but not cleaned up
+// when the corresponding manifest is removed from the file system.
+TEST(DuplicateRegistryEntries, Layers) {
+    FrameworkEnvironment env{};
+    env.add_icd(TestICDDetails(ManifestICD{}.set_lib_path(TEST_ICD_PATH_VERSION_2)));
+
+    auto null_path = env.get_folder(ManifestLocation::null).location() / "test_layer.json";
+
+    env.platform_shim->add_manifest(ManifestCategory::explicit_layer, null_path.str());
+
+    const char* layer_name = "TestLayer";
+    env.add_explicit_layer(
+        ManifestLayer{}.add_layer(
+            ManifestLayer::LayerDescription{}.set_name(layer_name).set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+        "test_layer.json");
+
+    InstWrapper inst{env.vulkan_functions};
+    inst.create_info.add_layer(layer_name);
+    inst.CheckCreate();
+}
+
+// Check that the de-duplication of drivers found in both PnP and generic Khronos/Vulkan/Drivers doesn't result in the same thing
+// being added twice
+TEST(DuplicateRegistryEntries, Drivers) {
+    FrameworkEnvironment env{};
+    auto null_path = env.get_folder(ManifestLocation::null).location() / "test_icd_0.json";
+    env.platform_shim->add_manifest(ManifestCategory::icd, null_path.str());
+
+    env.add_icd(TestICDDetails{TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA}.set_discovery_type(ManifestDiscoveryType::none));
+    auto& driver = env.get_test_icd();
+    driver.physical_devices.emplace_back("physical_device_0");
+    driver.adapterLUID = _LUID{10, 1000};
+    env.platform_shim->add_d3dkmt_adapter(D3DKMT_Adapter{0, _LUID{10, 1000}}.add_driver_manifest_path(env.get_icd_manifest_path()));
+
+    InstWrapper inst{env.vulkan_functions};
+    FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+    inst.CheckCreate();
+    ASSERT_TRUE(env.debug_log.find(
+        std::string("Skipping adding of json file \"") + null_path.str() +
+        "\" from registry \"HKEY_LOCAL_MACHINE\\SOFTWARE\\Khronos\\Vulkan\\Drivers\" to the list due to duplication"));
+}
 #endif