layers: Fix "count" tests in swapchain layer.

The swapchain layer was attempting to test if the application gave a value for
a *Count variable (e.g. pSurfaceFormatCount for the
vkGetPhysicalDeviceSurfaceFormatsKHR() function) that was larger than what that
function returned previously (i.e. when the non-count variable was NULL).
However, the test was made after calling down the call-chain, which may modify
the value given by the application.  This test is now made before calling down
the chain.

In addition, a new test is made that tries to ensure that the application did
call the function with the non-Count variable equal to NULL **before** calling
the function with a non-NULL non-Count variable.

3 files changed, 89 insertions, 46 deletions

diff --git a/layers/swapchain.cpp b/layers/swapchain.cpp
index 8efa8883..998e3ebf 100644
--- a/layers/swapchain.cpp
+++ b/layers/swapchain.cpp
@@ -1138,6 +1138,23 @@ vkGetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice physicalDevice, VkSurfaceK
     }
     if (!pSurfaceFormatCount) {
         skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, physicalDevice, "pSurfaceFormatCount");
+    } else if (pPhysicalDevice && pSurfaceFormats) {
+        // Compare the preliminary value of *pSurfaceFormatCount with the
+        // value this time:
+        if (pPhysicalDevice->surfaceFormatCount == 0) {
+            // Since we haven't recorded a preliminary value of
+            // *pSurfaceFormatCount, that likely means that the application
+            // didn't previously call this function with a NULL value of
+            // pSurfaceFormats:
+            skipCall |= LOG_ERROR_ZERO_PRIOR_COUNT(
+                VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                physicalDevice, "pSurfaceFormatCount", "pSurfaceFormats");
+        } else if (*pSurfaceFormatCount > pPhysicalDevice->surfaceFormatCount) {
+            skipCall |= LOG_ERROR_INVALID_COUNT(
+                VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                physicalDevice, "pSurfaceFormatCount", "pSurfaceFormats",
+                *pSurfaceFormatCount, pPhysicalDevice->surfaceFormatCount);
+        }
     }
 
     if (!skipCall) {
@@ -1152,23 +1169,18 @@ vkGetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice physicalDevice, VkSurfaceK
         if ((result == VK_SUCCESS) && pPhysicalDevice && !pSurfaceFormats && pSurfaceFormatCount) {
             // Record the result of this preliminary query:
             pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount;
-        } else if ((result == VK_SUCCESS) && pPhysicalDevice && pSurfaceFormats && pSurfaceFormatCount) {
-            // Compare the preliminary value of *pSurfaceFormatCount with the
-            // value this time:
-            if (*pSurfaceFormatCount > pPhysicalDevice->surfaceFormatCount) {
-                LOG_ERROR_INVALID_COUNT(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, physicalDevice, "pSurfaceFormatCount",
-                                        "pSurfaceFormats", *pSurfaceFormatCount, pPhysicalDevice->surfaceFormatCount);
-            } else if (*pSurfaceFormatCount > 0) {
-                // Record the result of this query:
-                pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount;
-                pPhysicalDevice->pSurfaceFormats = (VkSurfaceFormatKHR *)malloc(*pSurfaceFormatCount * sizeof(VkSurfaceFormatKHR));
-                if (pPhysicalDevice->pSurfaceFormats) {
-                    for (uint32_t i = 0; i < *pSurfaceFormatCount; i++) {
-                        pPhysicalDevice->pSurfaceFormats[i] = pSurfaceFormats[i];
-                    }
-                } else {
-                    pPhysicalDevice->surfaceFormatCount = 0;
+        } else if ((result == VK_SUCCESS) && pPhysicalDevice &&
+                   pSurfaceFormats && pSurfaceFormatCount &&
+                   (*pSurfaceFormatCount > 0)) {
+            // Record the result of this query:
+            pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount;
+            pPhysicalDevice->pSurfaceFormats = (VkSurfaceFormatKHR *)malloc(*pSurfaceFormatCount * sizeof(VkSurfaceFormatKHR));
+            if (pPhysicalDevice->pSurfaceFormats) {
+                for (uint32_t i = 0; i < *pSurfaceFormatCount; i++) {
+                    pPhysicalDevice->pSurfaceFormats[i] = pSurfaceFormats[i];
                 }
+            } else {
+                pPhysicalDevice->surfaceFormatCount = 0;
             }
         }
         return result;
@@ -1194,6 +1206,23 @@ vkGetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice physicalDevice, VkSur
     }
     if (!pPresentModeCount) {
         skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, physicalDevice, "pPresentModeCount");
+    } else if (pPhysicalDevice && pPresentModes) {
+        // Compare the preliminary value of *pPresentModeCount with the
+        // value this time:
+        if (pPhysicalDevice->presentModeCount == 0) {
+            // Since we haven't recorded a preliminary value of
+            // *pPresentModeCount, that likely means that the application
+            // didn't previously call this function with a NULL value of
+            // pPresentModes:
+            skipCall |= LOG_ERROR_ZERO_PRIOR_COUNT(
+                VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                physicalDevice, "pPresentModeCount", "pPresentModes");
+        } else if (*pPresentModeCount > pPhysicalDevice->presentModeCount) {
+            skipCall |= LOG_ERROR_INVALID_COUNT(
+                VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                physicalDevice, "pPresentModeCount", "pPresentModes",
+                *pPresentModeCount, pPhysicalDevice->presentModeCount);
+        }
     }
 
     if (!skipCall) {
@@ -1208,23 +1237,18 @@ vkGetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice physicalDevice, VkSur
         if ((result == VK_SUCCESS) && pPhysicalDevice && !pPresentModes && pPresentModeCount) {
             // Record the result of this preliminary query:
             pPhysicalDevice->presentModeCount = *pPresentModeCount;
-        } else if ((result == VK_SUCCESS) && pPhysicalDevice && pPresentModes && pPresentModeCount) {
-            // Compare the preliminary value of *pPresentModeCount with the
-            // value this time:
-            if (*pPresentModeCount > pPhysicalDevice->presentModeCount) {
-                LOG_ERROR_INVALID_COUNT(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, physicalDevice, "pPresentModeCount",
-                                        "pPresentModes", *pPresentModeCount, pPhysicalDevice->presentModeCount);
-            } else if (*pPresentModeCount > 0) {
-                // Record the result of this query:
-                pPhysicalDevice->presentModeCount = *pPresentModeCount;
-                pPhysicalDevice->pPresentModes = (VkPresentModeKHR *)malloc(*pPresentModeCount * sizeof(VkPresentModeKHR));
-                if (pPhysicalDevice->pPresentModes) {
-                    for (uint32_t i = 0; i < *pPresentModeCount; i++) {
-                        pPhysicalDevice->pPresentModes[i] = pPresentModes[i];
-                    }
-                } else {
-                    pPhysicalDevice->presentModeCount = 0;
+        } else if ((result == VK_SUCCESS) && pPhysicalDevice &&
+                   pPresentModes && pPresentModeCount &&
+                   (*pPresentModeCount > 0)) {
+            // Record the result of this query:
+            pPhysicalDevice->presentModeCount = *pPresentModeCount;
+            pPhysicalDevice->pPresentModes = (VkPresentModeKHR *)malloc(*pPresentModeCount * sizeof(VkPresentModeKHR));
+            if (pPhysicalDevice->pPresentModes) {
+                for (uint32_t i = 0; i < *pPresentModeCount; i++) {
+                    pPhysicalDevice->pPresentModes[i] = pPresentModes[i];
                 }
+            } else {
+                pPhysicalDevice->presentModeCount = 0;
             }
         }
         return result;
@@ -1665,6 +1689,23 @@ vkGetSwapchainImagesKHR(VkDevice device, VkSwapchainKHR swapchain, uint32_t *pSw
     SwpSwapchain *pSwapchain = &my_data->swapchainMap[swapchain];
     if (!pSwapchainImageCount) {
         skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, device, "pSwapchainImageCount");
+    } else if (pSwapchain && pSwapchainImages) {
+        // Compare the preliminary value of *pSwapchainImageCount with the
+        // value this time:
+        if (pSwapchain->imageCount == 0) {
+            // Since we haven't recorded a preliminary value of
+            // *pSwapchainImageCount, that likely means that the application
+            // didn't previously call this function with a NULL value of
+            // pSwapchainImages:
+            skipCall |= LOG_ERROR_ZERO_PRIOR_COUNT(
+                VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+                device, "pSwapchainImageCount", "pSwapchainImages");
+        } else if (*pSwapchainImageCount > pSwapchain->imageCount) {
+            skipCall |= LOG_ERROR_INVALID_COUNT(
+                VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+                device, "pSwapchainImageCount", "pSwapchainImages",
+                *pSwapchainImageCount, pSwapchain->imageCount);
+        }
     }
 
     if (!skipCall) {
@@ -1678,20 +1719,14 @@ vkGetSwapchainImagesKHR(VkDevice device, VkSwapchainKHR swapchain, uint32_t *pSw
         if ((result == VK_SUCCESS) && pSwapchain && !pSwapchainImages && pSwapchainImageCount) {
             // Record the result of this preliminary query:
             pSwapchain->imageCount = *pSwapchainImageCount;
-        } else if ((result == VK_SUCCESS) && pSwapchain && pSwapchainImages && pSwapchainImageCount) {
-            // Compare the preliminary value of *pSwapchainImageCount with the
-            // value this time:
-            if (*pSwapchainImageCount > pSwapchain->imageCount) {
-                LOG_ERROR_INVALID_COUNT(VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, device, "pSwapchainImageCount", "pSwapchainImages",
-                                        *pSwapchainImageCount, pSwapchain->imageCount);
-            } else if (*pSwapchainImageCount > 0) {
-                // Record the images and their state:
-                pSwapchain->imageCount = *pSwapchainImageCount;
-                for (uint32_t i = 0; i < *pSwapchainImageCount; i++) {
-                    pSwapchain->images[i].image = pSwapchainImages[i];
-                    pSwapchain->images[i].pSwapchain = pSwapchain;
-                    pSwapchain->images[i].acquiredByApp = false;
-                }
+        } else if ((result == VK_SUCCESS) && pSwapchain && pSwapchainImages &&
+                   pSwapchainImageCount && (*pSwapchainImageCount > 0)) {
+            // Record the images and their state:
+            pSwapchain->imageCount = *pSwapchainImageCount;
+            for (uint32_t i = 0; i < *pSwapchainImageCount; i++) {
+                pSwapchain->images[i].image = pSwapchainImages[i];
+                pSwapchain->images[i].pSwapchain = pSwapchain;
+                pSwapchain->images[i].acquiredByApp = false;
             }
         }
         return result;
diff --git a/layers/swapchain.h b/layers/swapchain.h
index 21a0dc6f..acbdaf48 100644
--- a/layers/swapchain.h
+++ b/layers/swapchain.h
@@ -61,6 +61,7 @@ typedef enum _SWAPCHAIN_ERROR {
     SWAPCHAIN_INDEX_TOO_LARGE,          // Index is too large for swapchain
     SWAPCHAIN_INDEX_NOT_IN_USE,         // vkQueuePresentKHR() given index that is not acquired by app
     SWAPCHAIN_BAD_BOOL,                 // VkBool32 that doesn't have value of VK_TRUE or VK_FALSE (e.g. is a non-zero form of true)
+    SWAPCHAIN_PRIOR_COUNT,              // Query must be called first to get value of pCount, then called second time
     SWAPCHAIN_INVALID_COUNT,            // Second time a query called, the pCount value didn't match first time
     SWAPCHAIN_WRONG_STYPE,              // The sType for a struct has the wrong value
     SWAPCHAIN_WRONG_NEXT,               // The pNext for a struct is not NULL
@@ -92,6 +93,12 @@ typedef enum _SWAPCHAIN_ERROR {
                                                              "was returned when %s was NULL.",                                     \
                         __FUNCTION__, (obj2), (obj), (val), (val2), (obj2))                                                        \
               : VK_FALSE
+#define LOG_ERROR_ZERO_PRIOR_COUNT(objType, type, obj, obj2)                                                                       \
+    (my_data) ? log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (objType), (uint64_t)(obj), 0,                        \
+                        SWAPCHAIN_PRIOR_COUNT, LAYER_NAME, "%s() called with non-NULL %s; but no prior "                           \
+                        "positive value has been seen for %s.",                                                                    \
+                        __FUNCTION__, (obj), (obj2))                                                                               \
+              : VK_FALSE
 #define LOG_ERROR_WRONG_STYPE(objType, type, obj, val)                                                                             \
     (my_data) ? log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (objType), (uint64_t)(obj), 0, SWAPCHAIN_WRONG_STYPE, \
                         LAYER_NAME, "%s() called with the wrong value for %s->sType "                                              \
diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md
index 53882412..bf1c4ac8 100644
--- a/layers/vk_validation_layer_details.md
+++ b/layers/vk_validation_layer_details.md
@@ -379,6 +379,7 @@ This layer is a work in progress. VK_LAYER_LUNARG_swapchain layer is intended to
 | Index too large | Validates that an image index is within the number of images in a swapchain | INDEX_TOO_LARGE | vkQueuePresentKHR | NA | None |
 | Can't present a non-owned image | Validates that application only presents images that it owns | INDEX_NOT_IN_USE | vkQueuePresentKHR | NA | None |
 | A VkBool32 must have values of VK_TRUE or VK_FALSE | Validates that a VkBool32 must have values of VK_TRUE or VK_FALSE | BAD_BOOL | vkCreateSwapchainKHR | NA | None |
+| pCount must be set by the API before the other pointer is non-NULL | Validates that app queries for the value of pCount before trying to set it | PRIOR_COUNT | vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfacePresentModesKHR vkGetSwapchainImagesKHR | NA | None |
 | pCount must point to same value regardless of whether other pointer is NULL | Validates that app doesn't change value of pCount returned by a query | INVALID_COUNT | vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfacePresentModesKHR vkGetSwapchainImagesKHR | NA | None |
 | Valid sType | Validates that a struct has correct value for sType | WRONG_STYPE | vkCreateSwapchainKHR vkQueuePresentKHR | NA | None |
 | Valid pNext | Validates that a struct has NULL for the value of pNext | WRONG_NEXT | vkCreateSwapchainKHR vkQueuePresentKHR | NA | None |