loader: Make device extensions return same count
authorCharles Giessen <charles@lunarg.com>
Wed, 5 Aug 2020 19:42:44 +0000 (13:42 -0600)
committerCharles Giessen <46324611+charles-lunarg@users.noreply.github.com>
Wed, 2 Sep 2020 23:40:36 +0000 (17:40 -0600)
Previously the loader would return a different count in the call to
vkEnumeratePhysicalDeviceExtensionProperties if any implicit layers contained extensions
that a driver also happened to support, thus causing a duplicate entry to the count, but
was later resolved in the second call to that function. This commit de-duplicates the
extensions when getting the count, preventing such an issue from occuring.

Change-Id: I3b3c0b8871e412d26dda916ed1e1e3fe3299491e

loader/loader.c

index b7660b92b550613f25b4f47973e79c245574d7f0..13d93753ce344eae6ef3bd3dfd95a901ac28f57e 100644 (file)
@@ -7562,10 +7562,26 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceExtensionProperties(VkP
     // This case is during the call down the instance chain with pLayerName == NULL
     struct loader_icd_term *icd_term = phys_dev_term->this_icd_term;
     uint32_t icd_ext_count = *pPropertyCount;
+    VkExtensionProperties *icd_props_list = pProperties;
     VkResult res;
 
-    // Get the available device extensions
-    res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &icd_ext_count, pProperties);
+    if (NULL == icd_props_list) {
+        // We need to find the count without duplicates. This requires querying the driver for the names of the extensions.
+        // A small amount of storage is then needed to facilitate the de-duplication.
+        res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &icd_ext_count, NULL);
+        if (res != VK_SUCCESS) {
+            goto out;
+        }
+        icd_props_list = loader_instance_heap_alloc(icd_term->this_instance, sizeof(VkExtensionProperties) * icd_ext_count,
+                                                    VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
+        if (NULL == icd_props_list) {
+            res = VK_ERROR_OUT_OF_HOST_MEMORY;
+            goto out;
+        }
+    }
+
+    // Get the available device extension count, and if pProperties is not NULL, the extensions as well
+    res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &icd_ext_count, icd_props_list);
     if (res != VK_SUCCESS) {
         goto out;
     }
@@ -7576,36 +7592,36 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceExtensionProperties(VkP
     }
 
     loaderAddImplicitLayers(icd_term->this_instance, &implicit_layer_list, NULL, &icd_term->this_instance->instance_layer_list);
+
+    // Initialize dev_extension list within the physicalDevice object
+    res = loader_init_device_extensions(icd_term->this_instance, phys_dev_term, icd_ext_count, icd_props_list, &icd_exts);
+    if (res != VK_SUCCESS) {
+        goto out;
+    }
+
     // We need to determine which implicit layers are active, and then add their extensions. This can't be cached as
     // it depends on results of environment variables (which can change).
-    if (pProperties != NULL) {
-        // Initialize dev_extension list within the physicalDevice object
-        res = loader_init_device_extensions(icd_term->this_instance, phys_dev_term, icd_ext_count, pProperties, &icd_exts);
-        if (res != VK_SUCCESS) {
-            goto out;
-        }
-
-        // We need to determine which implicit layers are active, and then add their extensions. This can't be cached as
-        // it depends on results of environment variables (which can change).
-        res = loader_add_to_ext_list(icd_term->this_instance, &all_exts, icd_exts.count, icd_exts.list);
-        if (res != VK_SUCCESS) {
-            goto out;
-        }
+    res = loader_add_to_ext_list(icd_term->this_instance, &all_exts, icd_exts.count, icd_exts.list);
+    if (res != VK_SUCCESS) {
+        goto out;
+    }
 
-        loaderAddImplicitLayers(icd_term->this_instance, &implicit_layer_list, NULL, &icd_term->this_instance->instance_layer_list);
+    loaderAddImplicitLayers(icd_term->this_instance, &implicit_layer_list, NULL, &icd_term->this_instance->instance_layer_list);
 
-        for (uint32_t i = 0; i < implicit_layer_list.count; i++) {
-            for (uint32_t j = 0; j < implicit_layer_list.list[i].device_extension_list.count; j++) {
-                res = loader_add_to_ext_list(icd_term->this_instance, &all_exts, 1,
-                                             &implicit_layer_list.list[i].device_extension_list.list[j].props);
-                if (res != VK_SUCCESS) {
-                    goto out;
-                }
+    for (uint32_t i = 0; i < implicit_layer_list.count; i++) {
+        for (uint32_t j = 0; j < implicit_layer_list.list[i].device_extension_list.count; j++) {
+            res = loader_add_to_ext_list(icd_term->this_instance, &all_exts, 1,
+                                         &implicit_layer_list.list[i].device_extension_list.list[j].props);
+            if (res != VK_SUCCESS) {
+                goto out;
             }
         }
-        uint32_t capacity = *pPropertyCount;
-        VkExtensionProperties *props = pProperties;
+    }
+    uint32_t capacity = *pPropertyCount;
+    VkExtensionProperties *props = pProperties;
 
+    res = VK_SUCCESS;
+    if (NULL != pProperties) {
         for (uint32_t i = 0; i < all_exts.count && i < capacity; i++) {
             props[i] = all_exts.list[i];
         }
@@ -7617,47 +7633,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceExtensionProperties(VkP
             *pPropertyCount = all_exts.count;
         }
     } else {
-        // Have to find the number of unique extensions, ie no duplicates, as the properties list returned contains no duplicates.
-
-        // Find the current number of extensions (with duplicates). This is the upper bound for the ext_name_list
-        uint32_t max_exts_num = icd_ext_count;
-        for (uint32_t i = 0; i < implicit_layer_list.count; i++) {
-            max_exts_num += implicit_layer_list.list[i].device_extension_list.count;
-        }
-
-        const struct loader_instance *inst = icd_term->this_instance;
-
-        uint32_t ext_name_count = 0;
-        char **ext_name_list = loader_instance_heap_alloc(inst, sizeof(char *) * max_exts_num, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
-        if (ext_name_list == NULL) {
-            // Failed to allocate string list, bail
-            res = VK_ERROR_OUT_OF_HOST_MEMORY;
-            goto out;
-        }
-
-        // Look through the implicit_layer_list of device extensions and determine if its not in the ext_name_list, add it if it
-        // isn't. If it is, ignore it, as it is a duplicate
-        for (uint32_t i = 0; i < implicit_layer_list.count; i++) {
-            for (uint32_t j = 0; j < implicit_layer_list.list[i].device_extension_list.count; j++) {
-                char *extension_name = implicit_layer_list.list[i].device_extension_list.list[j].props.extensionName;
-                bool in_list = false;
-                for (uint32_t k = 0; k < ext_name_count; k++) {
-                    if (strncmp(extension_name, ext_name_list[k], 256) == 0) {
-                        in_list = true;
-                        break;
-                    }
-                }
-                if (!in_list) {
-                    ext_name_list[ext_name_count] = implicit_layer_list.list[i].device_extension_list.list[j].props.extensionName;
-                    ext_name_count++;
-                }
-            }
-        }
-        loader_instance_heap_free(inst, ext_name_list);
-        // Add the device extensions already found. Can't check for duplicates from the devices because the names aren't available
-        *pPropertyCount = ext_name_count + icd_ext_count;
-
-        res = VK_SUCCESS;
+        *pPropertyCount = all_exts.count;
     }
 
 out:
@@ -7671,7 +7647,9 @@ out:
     if (NULL != icd_exts.list) {
         loader_destroy_generic_list(icd_term->this_instance, (struct loader_generic_list *)&icd_exts);
     }
-
+    if (NULL == pProperties && NULL != icd_props_list) {
+        loader_instance_heap_free(icd_term->this_instance, icd_props_list);
+    }
     return res;
 }