layers: Update DeviceQueueCreate checks

- remove some potential false-positives from QF check
- update error db with existing check
- use error db unique codes on existing checks
- move check that need state to core_validation
- deal with VK_QUEUE_FAMILY_IGNORED case
- improve error messages texts
- make messages return appropriate object
- move code that looks displaced to appropriate places
- add locks

1 files changed, 130 insertions, 124 deletions

diff --git a/layers/parameter_validation.cpp b/layers/parameter_validation.cpp
index d45d7d6b..c6653902 100644
--- a/layers/parameter_validation.cpp
+++ b/layers/parameter_validation.cpp
@@ -29,6 +29,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <inttypes.h>
 
 #include <iostream>
 #include <string>
@@ -36,6 +37,7 @@
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
+#include <mutex>
 
 #include "vk_loader_platform.h"
 #include "vulkan/vk_layer.h"
@@ -77,11 +79,15 @@ struct layer_data {
     VkPhysicalDeviceLimits device_limits = {};
     VkPhysicalDeviceFeatures physical_device_features = {};
     VkPhysicalDevice physical_device = VK_NULL_HANDLE;
+    VkDevice device = VK_NULL_HANDLE;
     DeviceExtensions enables;
 
     VkLayerDispatchTable dispatch_table = {};
 };
 
+// TODO : This can be much smarter, using separate locks for separate global data
+static std::mutex global_lock;
+
 static uint32_t loader_layer_if_version = CURRENT_LOADER_LAYER_INTERFACE_VERSION;
 static std::unordered_map<void *, layer_data *> layer_data_map;
 static std::unordered_map<void *, instance_layer_data *> instance_layer_data_map;
@@ -1224,25 +1230,24 @@ static bool validate_string(debug_report_data *report_data, const char *apiName,
     return skip;
 }
 
-static bool validate_queue_family_index(layer_data *device_data, const char *function_name, const char *parameter_name,
-                                        uint32_t index) {
-    assert(device_data != nullptr);
-    debug_report_data *report_data = device_data->report_data;
+static bool ValidateDeviceQueueFamily(layer_data *device_data, uint32_t queue_family, const char *cmd_name,
+                                      const char *parameter_name, int32_t error_code, bool optional = false,
+                                      const char *vu_note = nullptr) {
     bool skip = false;
 
-    if (index == VK_QUEUE_FAMILY_IGNORED) {
-        skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, 1,
-                        LayerName, "%s: %s cannot be VK_QUEUE_FAMILY_IGNORED.", function_name, parameter_name);
-    } else {
-        const auto &queue_data = device_data->queueFamilyIndexMap.find(index);
-        if (queue_data == device_data->queueFamilyIndexMap.end()) {
-            skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, 1,
-                            LayerName,
-                            "%s: %s (%d) must be one of the indices specified when the device was created, via "
-                            "the VkDeviceQueueCreateInfo structure.",
-                            function_name, parameter_name, index);
-            return false;
-        }
+    if (!vu_note) vu_note = validation_error_map[error_code];
+
+    if (!optional && queue_family == VK_QUEUE_FAMILY_IGNORED) {
+        skip |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+                        reinterpret_cast<uint64_t>(device_data->device), __LINE__, error_code, LayerName,
+                        "%s: %s is VK_QUEUE_FAMILY_IGNORED, but it is required to provide a valid queue family index value. %s",
+                        cmd_name, parameter_name, vu_note);
+    } else if (device_data->queueFamilyIndexMap.find(queue_family) == device_data->queueFamilyIndexMap.end()) {
+        skip |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+                        reinterpret_cast<uint64_t>(device_data->device), __LINE__, error_code, LayerName,
+                        "%s: %s (=%" PRIu32 ") is not one of the queue families given via VkDeviceQueueCreateInfo structures when "
+                        "the device was created. %s",
+                        cmd_name, parameter_name, queue_family, vu_note);
     }
 
     return skip;
@@ -1497,55 +1502,104 @@ VKAPI_ATTR void VKAPI_CALL GetPhysicalDeviceMemoryProperties(VkPhysicalDevice ph
     }
 }
 
-static void validateDeviceCreateInfo(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo,
-                                     const std::vector<VkQueueFamilyProperties> properties) {
-    std::unordered_set<uint32_t> set;
+static bool ValidateDeviceCreateInfo(instance_layer_data *instance_data, VkPhysicalDevice physicalDevice,
+                                     const VkDeviceCreateInfo *pCreateInfo) {
+    bool skip = false;
 
-    auto my_data = GetLayerDataPtr(get_dispatch_key(physicalDevice), instance_layer_data_map);
+    if ((pCreateInfo->enabledLayerCount > 0) && (pCreateInfo->ppEnabledLayerNames != NULL)) {
+        for (size_t i = 0; i < pCreateInfo->enabledLayerCount; i++) {
+            skip |= validate_string(instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledLayerNames",
+                                    pCreateInfo->ppEnabledLayerNames[i]);
+        }
+    }
+
+    if ((pCreateInfo->enabledExtensionCount > 0) && (pCreateInfo->ppEnabledExtensionNames != NULL)) {
+        for (size_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
+            skip |= validate_string(instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledExtensionNames",
+                                    pCreateInfo->ppEnabledExtensionNames[i]);
+        }
+    }
+
+    if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
+        // Check for get_physical_device_properties2 struct
+        struct std_header {
+            VkStructureType sType;
+            const void *pNext;
+        };
+        std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
+        while (cur_pnext) {
+            if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
+                // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
+                skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT,
+                                0, __LINE__, INVALID_USAGE, LayerName,
+                                "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
+                                "pCreateInfo->pEnabledFeatures is non-NULL.");
+                break;
+            }
+            cur_pnext = (std_header *)cur_pnext->pNext;
+        }
+    }
+    if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
+        // Check for get_physical_device_properties2 struct
+        struct std_header {
+            VkStructureType sType;
+            const void *pNext;
+        };
+        std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
+        while (cur_pnext) {
+            if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
+                // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
+                skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT,
+                                0, __LINE__, INVALID_USAGE, LayerName,
+                                "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
+                                "pCreateInfo->pEnabledFeatures is non-NULL.");
+                break;
+            }
+            cur_pnext = (std_header *)cur_pnext->pNext;
+        }
+    }
+
+    // Validate pCreateInfo->pQueueCreateInfos
+    if (pCreateInfo->pQueueCreateInfos) {
+        std::unordered_set<uint32_t> set;
 
-    if ((pCreateInfo != nullptr) && (pCreateInfo->pQueueCreateInfos != nullptr)) {
         for (uint32_t i = 0; i < pCreateInfo->queueCreateInfoCount; ++i) {
-            if (set.count(pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex)) {
-                log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
-                        VALIDATION_ERROR_00035, LayerName,
-                        "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->queueFamilyIndex, is not unique within this "
-                        "structure. %s",
-                        i, validation_error_map[VALIDATION_ERROR_00035]);
+            const uint32_t requested_queue_family = pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex;
+            if (requested_queue_family == VK_QUEUE_FAMILY_IGNORED) {
+                skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, reinterpret_cast<uint64_t>(physicalDevice),
+                                __LINE__, VALIDATION_ERROR_00054, LayerName,
+                                "vkCreateDevice: pCreateInfo->pQueueCreateInfos[%" PRIu32 "].queueFamilyIndex is "
+                                "VK_QUEUE_FAMILY_IGNORED, but it is required to provide a valid queue family index value. %s",
+                                i, validation_error_map[VALIDATION_ERROR_00054]);
+            } else if (set.count(requested_queue_family)) {
+                skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, reinterpret_cast<uint64_t>(physicalDevice),
+                                __LINE__, VALIDATION_ERROR_00035, LayerName,
+                                "vkCreateDevice: pCreateInfo->pQueueCreateInfos[%" PRIu32 "].queueFamilyIndex (=%" PRIu32 ") is "
+                                "not unique within pCreateInfo->pQueueCreateInfos array. %s",
+                                i, requested_queue_family, validation_error_map[VALIDATION_ERROR_00035]);
             } else {
-                set.insert(pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex);
+                set.insert(requested_queue_family);
             }
 
             if (pCreateInfo->pQueueCreateInfos[i].pQueuePriorities != nullptr) {
                 for (uint32_t j = 0; j < pCreateInfo->pQueueCreateInfos[i].queueCount; ++j) {
-                    if ((pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j] < 0.f) ||
-                        (pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j] > 1.f)) {
-                        log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0,
-                                __LINE__, INVALID_USAGE, LayerName,
-                                "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->pQueuePriorities[%d], must be "
-                                "between 0 and 1. Actual value is %f",
-                                i, j, pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j]);
+                    const float queue_priority = pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j];
+                    if (!(queue_priority >= 0.f) || !(queue_priority <= 1.f)) {
+                        skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
+                                        VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, reinterpret_cast<uint64_t>(physicalDevice),
+                                        __LINE__, VALIDATION_ERROR_02056, LayerName,
+                                        "vkCreateDevice: pCreateInfo->pQueueCreateInfos[%" PRIu32 "].pQueuePriorities[%" PRIu32
+                                        "] (=%f) is not between 0 and 1 (inclusive). %s",
+                                        i, j, queue_priority, validation_error_map[VALIDATION_ERROR_02056]);
                     }
                 }
             }
-
-            if (pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex >= properties.size()) {
-                log_msg(
-                    my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
-                    INVALID_USAGE, LayerName,
-                    "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->queueFamilyIndex cannot be more than the number "
-                    "of queue families.",
-                    i);
-            } else if (pCreateInfo->pQueueCreateInfos[i].queueCount >
-                       properties[pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex].queueCount) {
-                log_msg(
-                    my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
-                    INVALID_USAGE, LayerName,
-                    "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->queueCount cannot be more than the number of "
-                    "queues for the given family index.",
-                    i);
-            }
         }
     }
+
+    return skip;
 }
 
 void storeCreateDeviceData(VkDevice device, const VkDeviceCreateInfo *pCreateInfo) {
@@ -1570,62 +1624,11 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice physicalDevice, con
     bool skip = false;
     auto my_instance_data = GetLayerDataPtr(get_dispatch_key(physicalDevice), instance_layer_data_map);
     assert(my_instance_data != nullptr);
+    std::unique_lock<std::mutex> lock(global_lock);
 
     skip |= parameter_validation_vkCreateDevice(my_instance_data->report_data, pCreateInfo, pAllocator, pDevice);
 
-    if (pCreateInfo != NULL) {
-        if ((pCreateInfo->enabledLayerCount > 0) && (pCreateInfo->ppEnabledLayerNames != NULL)) {
-            for (size_t i = 0; i < pCreateInfo->enabledLayerCount; i++) {
-                skip |= validate_string(my_instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledLayerNames",
-                                        pCreateInfo->ppEnabledLayerNames[i]);
-            }
-        }
-
-        if ((pCreateInfo->enabledExtensionCount > 0) && (pCreateInfo->ppEnabledExtensionNames != NULL)) {
-            for (size_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
-                skip |= validate_string(my_instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledExtensionNames",
-                                        pCreateInfo->ppEnabledExtensionNames[i]);
-            }
-        }
-        if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
-            // Check for get_physical_device_properties2 struct
-            struct std_header {
-                VkStructureType sType;
-                const void *pNext;
-            };
-            std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
-            while (cur_pnext) {
-                if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
-                    // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
-                    skip |= log_msg(my_instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
-                                    VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, INVALID_USAGE, LayerName,
-                                    "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
-                                    "pCreateInfo->pEnabledFeatures is non-NULL.");
-                    break;
-                }
-                cur_pnext = (std_header *)cur_pnext->pNext;
-            }
-        }
-        if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
-            // Check for get_physical_device_properties2 struct
-            struct std_header {
-                VkStructureType sType;
-                const void *pNext;
-            };
-            std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
-            while (cur_pnext) {
-                if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
-                    // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
-                    skip |= log_msg(my_instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
-                                    VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, INVALID_USAGE, LayerName,
-                                    "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
-                                    "pCreateInfo->pEnabledFeatures is non-NULL.");
-                    break;
-                }
-                cur_pnext = (std_header *)cur_pnext->pNext;
-            }
-        }
-    }
+    if (pCreateInfo != NULL) skip |= ValidateDeviceCreateInfo(my_instance_data, physicalDevice, pCreateInfo);
 
     if (!skip) {
         VkLayerDeviceCreateInfo *chain_info = get_chain_info(pCreateInfo, VK_LAYER_LINK_INFO);
@@ -1642,10 +1645,14 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice physicalDevice, con
         // Advance the link info for the next element on the chain
         chain_info->u.pLayerInfo = chain_info->u.pLayerInfo->pNext;
 
+        lock.unlock();
+
         result = fpCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice);
 
+        lock.lock();
         validate_result(my_instance_data->report_data, "vkCreateDevice", {}, result);
 
+
         if (result == VK_SUCCESS) {
             layer_data *my_device_data = GetLayerDataPtr(get_dispatch_key(*pDevice), layer_data_map);
             assert(my_device_data != nullptr);
@@ -1655,12 +1662,6 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice physicalDevice, con
 
             my_device_data->enables.InitFromDeviceCreateInfo(pCreateInfo);
 
-            uint32_t count;
-            my_instance_data->dispatch_table.GetPhysicalDeviceQueueFamilyProperties(physicalDevice, &count, nullptr);
-            std::vector<VkQueueFamilyProperties> properties(count);
-            my_instance_data->dispatch_table.GetPhysicalDeviceQueueFamilyProperties(physicalDevice, &count, &properties[0]);
-
-            validateDeviceCreateInfo(physicalDevice, pCreateInfo, properties);
             storeCreateDeviceData(*pDevice, pCreateInfo);
 
             // Query and save physical device limits for this device
@@ -1668,6 +1669,7 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice physicalDevice, con
             my_instance_data->dispatch_table.GetPhysicalDeviceProperties(physicalDevice, &device_properties);
             memcpy(&my_device_data->device_limits, &device_properties.limits, sizeof(VkPhysicalDeviceLimits));
             my_device_data->physical_device = physicalDevice;
+            my_device_data->device = *pDevice;
 
             // Save app-enabled features in this device's layer_data structure
             if (pCreateInfo->pEnabledFeatures) {
@@ -1702,34 +1704,38 @@ VKAPI_ATTR void VKAPI_CALL DestroyDevice(VkDevice device, const VkAllocationCall
 }
 
 static bool PreGetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex) {
+    bool skip = false;
     layer_data *my_device_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
     assert(my_device_data != nullptr);
 
-    validate_queue_family_index(my_device_data, "vkGetDeviceQueue", "queueFamilyIndex", queueFamilyIndex);
+    skip |=
+        ValidateDeviceQueueFamily(my_device_data, queueFamilyIndex, "vkGetDeviceQueue", "queueFamilyIndex", VALIDATION_ERROR_00060);
 
     const auto &queue_data = my_device_data->queueFamilyIndexMap.find(queueFamilyIndex);
-    if (queue_data->second <= queueIndex) {
-        log_msg(
-            my_device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
-            VALIDATION_ERROR_00061, LayerName,
-            "vkGetDeviceQueue() parameter, uint32_t queueIndex %d, must be less than the number of queues given when the device "
-            "was created. %s",
-            queueIndex, validation_error_map[VALIDATION_ERROR_00061]);
-        return false;
+    if (queue_data != my_device_data->queueFamilyIndexMap.end() && queue_data->second <= queueIndex) {
+        skip |= log_msg(my_device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+                        reinterpret_cast<uint64_t>(device), __LINE__, VALIDATION_ERROR_00061, LayerName,
+                        "vkGetDeviceQueue: queueIndex (=%" PRIu32 ") is not less than the number of queues requested from "
+                        "queueFamilyIndex (=%" PRIu32 ") when the device was created (i.e. is not less than %" PRIu32 "). %s",
+                        queueIndex, queueFamilyIndex, queue_data->second, validation_error_map[VALIDATION_ERROR_00061]);
     }
-    return true;
+
+    return skip;
 }
 
 VKAPI_ATTR void VKAPI_CALL GetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue *pQueue) {
     bool skip = false;
     layer_data *my_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
     assert(my_data != NULL);
+    std::unique_lock<std::mutex> lock(global_lock);
 
     skip |= parameter_validation_vkGetDeviceQueue(my_data->report_data, queueFamilyIndex, queueIndex, pQueue);
 
     if (!skip) {
         PreGetDeviceQueue(device, queueFamilyIndex, queueIndex);
 
+        lock.unlock();
+
         my_data->dispatch_table.GetDeviceQueue(device, queueFamilyIndex, queueIndex, pQueue);
     }
 }
@@ -3963,8 +3969,8 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateCommandPool(VkDevice device, const VkComman
     layer_data *my_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
     assert(my_data != NULL);
 
-    skip |=
-        validate_queue_family_index(my_data, "vkCreateCommandPool", "pCreateInfo->queueFamilyIndex", pCreateInfo->queueFamilyIndex);
+    skip |= ValidateDeviceQueueFamily(my_data, pCreateInfo->queueFamilyIndex, "vkCreateCommandPool",
+                                      "pCreateInfo->queueFamilyIndex", VALIDATION_ERROR_00068);
 
     skip |= parameter_validation_vkCreateCommandPool(my_data->report_data, pCreateInfo, pAllocator, pCommandPool);