Swapchain: Added NULL_POINTER checks & pCount checks.

3 files changed, 115 insertions, 29 deletions

diff --git a/layers/swapchain.cpp b/layers/swapchain.cpp
index fca7d212..9f77fc67 100644
--- a/layers/swapchain.cpp
+++ b/layers/swapchain.cpp
@@ -936,6 +936,11 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceSupport
                               __FUNCTION__, VK_KHR_SURFACE_EXTENSION_NAME);
     }
     skipCall |= validateSurface(my_data, surface, (char *) __FUNCTION__);
+    if (!pSupported) {
+        skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                                           physicalDevice,
+                                           "pSupported");
+    }
 
     if (VK_FALSE == skipCall) {
         // Call down the call chain:
@@ -982,6 +987,11 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceCapabil
                               __FUNCTION__, VK_KHR_SURFACE_EXTENSION_NAME);
     }
     skipCall |= validateSurface(my_data, surface, (char *) __FUNCTION__);
+    if (!pSurfaceCapabilities) {
+        skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                                           physicalDevice,
+                                           "pSurfaceCapabilities");
+    }
 
     if (VK_FALSE == skipCall) {
         // Call down the call chain:
@@ -989,6 +999,7 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceCapabil
                 physicalDevice, surface, pSurfaceCapabilities);
 
         if ((result == VK_SUCCESS) && pPhysicalDevice) {
+            // Record the result of this query:
             pPhysicalDevice->gotSurfaceCapabilities = true;
 // FIXME: NEED TO COPY THIS DATA, BECAUSE pSurfaceCapabilities POINTS TO APP-ALLOCATED DATA
             pPhysicalDevice->surfaceCapabilities = *pSurfaceCapabilities;
@@ -1002,7 +1013,7 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceCapabil
 VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceFormatsKHR(
     VkPhysicalDevice physicalDevice,
     VkSurfaceKHR surface,
-    uint32_t* pCount,
+    uint32_t* pSurfaceFormatCount,
     VkSurfaceFormatKHR* pSurfaceFormats)
 {
     VkResult result = VK_SUCCESS;
@@ -1025,19 +1036,39 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceFormats
                               __FUNCTION__, VK_KHR_SURFACE_EXTENSION_NAME);
     }
     skipCall |= validateSurface(my_data, surface, (char *) __FUNCTION__);
+    if (!pSurfaceFormatCount) {
+        skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                                           physicalDevice,
+                                           "pSurfaceFormatCount");
+    }
 
     if (VK_FALSE == skipCall) {
         // Call down the call chain:
         result = my_data->instance_dispatch_table->GetPhysicalDeviceSurfaceFormatsKHR(
-                physicalDevice, surface, pCount, pSurfaceFormats);
+                physicalDevice, surface, pSurfaceFormatCount, pSurfaceFormats);
 
-        if ((result == VK_SUCCESS) && pPhysicalDevice && pSurfaceFormats && pCount &&
-            (*pCount > 0)) {
-            pPhysicalDevice->surfaceFormatCount = *pCount;
+        if ((result == VK_SUCCESS) && pPhysicalDevice && !pSurfaceFormats &&
+            pSurfaceFormatCount) {
+            // Record the result of this preliminary query:
+            pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount;
+        }
+        if ((result == VK_SUCCESS) && pPhysicalDevice && pSurfaceFormats &&
+            pSurfaceFormatCount && (*pSurfaceFormatCount > 0)) {
+            // 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",
+                                        pPhysicalDevice->surfaceFormatCount);
+            }
+            // Record the result of this query:
+            pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount;
             pPhysicalDevice->pSurfaceFormats = (VkSurfaceFormatKHR *)
-                malloc(*pCount * sizeof(VkSurfaceFormatKHR));
+                malloc(*pSurfaceFormatCount * sizeof(VkSurfaceFormatKHR));
             if (pPhysicalDevice->pSurfaceFormats) {
-                for (uint32_t i = 0 ; i < *pCount ; i++) {
+                for (uint32_t i = 0 ; i < *pSurfaceFormatCount ; i++) {
                     pPhysicalDevice->pSurfaceFormats[i] = pSurfaceFormats[i];
                 }
             } else {
@@ -1053,7 +1084,7 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfaceFormats
 VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfacePresentModesKHR(
     VkPhysicalDevice physicalDevice,
     VkSurfaceKHR surface,
-    uint32_t* pCount,
+    uint32_t* pPresentModeCount,
     VkPresentModeKHR* pPresentModes)
 {
     VkResult result = VK_SUCCESS;
@@ -1076,19 +1107,39 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetPhysicalDeviceSurfacePresent
                               __FUNCTION__, VK_KHR_SURFACE_EXTENSION_NAME);
     }
     skipCall |= validateSurface(my_data, surface, (char *) __FUNCTION__);
+    if (!pPresentModeCount) {
+        skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                                           physicalDevice,
+                                           "pPresentModeCount");
+    }
 
     if (VK_FALSE == skipCall) {
         // Call down the call chain:
         result = my_data->instance_dispatch_table->GetPhysicalDeviceSurfacePresentModesKHR(
-                physicalDevice, surface, pCount, pPresentModes);
+                physicalDevice, surface, pPresentModeCount, pPresentModes);
 
-        if ((result == VK_SUCCESS) && pPhysicalDevice && pPresentModes && pCount &&
-            (*pCount > 0)) {
-            pPhysicalDevice->presentModeCount = *pCount;
+        if ((result == VK_SUCCESS) && pPhysicalDevice && !pPresentModes &&
+            pPresentModeCount) {
+            // Record the result of this preliminary query:
+            pPhysicalDevice->presentModeCount = *pPresentModeCount;
+        }
+        if ((result == VK_SUCCESS) && pPhysicalDevice && pPresentModes &&
+            pPresentModeCount && (*pPresentModeCount > 0)) {
+            // 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",
+                                        pPhysicalDevice->presentModeCount);
+            }
+            // Record the result of this query:
+            pPhysicalDevice->presentModeCount = *pPresentModeCount;
             pPhysicalDevice->pPresentModes = (VkPresentModeKHR *)
-                malloc(*pCount * sizeof(VkPresentModeKHR));
+                malloc(*pPresentModeCount * sizeof(VkPresentModeKHR));
             if (pPhysicalDevice->pSurfaceFormats) {
-                for (uint32_t i = 0 ; i < *pCount ; i++) {
+                for (uint32_t i = 0 ; i < *pPresentModeCount ; i++) {
                     pPhysicalDevice->pPresentModes[i] = pPresentModes[i];
                 }
             } else {
@@ -1515,7 +1566,11 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkDestroySwapchainKHR(
     }
 }
 
-VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetSwapchainImagesKHR(VkDevice device, VkSwapchainKHR swapchain, uint32_t* pCount, VkImage* pSwapchainImages)
+VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetSwapchainImagesKHR(
+    VkDevice                                    device,
+    VkSwapchainKHR                              swapchain,
+    uint32_t*                                   pSwapchainImageCount,
+    VkImage*                                    pSwapchainImages)
 {
     VkResult result = VK_SUCCESS;
     VkBool32 skipCall = VK_FALSE;
@@ -1540,25 +1595,39 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetSwapchainImagesKHR(VkDevice
                                             swapchain.handle,
                                             "VkSwapchainKHR");
     }
+    if (!pSwapchainImageCount) {
+        skipCall |= LOG_ERROR_NULL_POINTER(VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT,
+                                           physicalDevice,
+                                           "pSwapchainImageCount");
+    }
 
     if (VK_FALSE == skipCall) {
         // Call down the call chain:
         result = my_data->device_dispatch_table->GetSwapchainImagesKHR(
-                device, swapchain, pCount, pSwapchainImages);
+                device, swapchain, pSwapchainImageCount, pSwapchainImages);
 
-// TBD: Should we validate that this function was called once with
-// pSwapchainImages set to NULL (and record pCount at that time), and then
-// called again with a non-NULL pSwapchainImages?
-        if ((result == VK_SUCCESS) && pSwapchain &&pSwapchainImages &&
-            pCount && (*pCount > 0)) {
+        if ((result == VK_SUCCESS) && pSwapchain && !pSwapchainImages &&
+            pSwapchainImageCount) {
+            // Record the result of this preliminary query:
+            pSwapchain->imageCount = *pSwapchainImageCount;
+        }
+        if ((result == VK_SUCCESS) && pSwapchain && pSwapchainImages &&
+            pSwapchainImageCount && (*pSwapchainImageCount > 0)) {
+            // 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",
+                                        pSwapchain->imageCount);
+            }
             // Record the images and their state:
-            if (pSwapchain) {
-                pSwapchain->imageCount = *pCount;
-                for (uint32_t i = 0 ; i < *pCount ; i++) {
-                    pSwapchain->images[i].image = pSwapchainImages[i];
-                    pSwapchain->images[i].pSwapchain = pSwapchain;
-                    pSwapchain->images[i].ownedByApp = false;
-                }
+            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].ownedByApp = false;
             }
         }
 
diff --git a/layers/swapchain.h b/layers/swapchain.h
index cb5682c5..4df160ec 100644
--- a/layers/swapchain.h
+++ b/layers/swapchain.h
@@ -61,6 +61,7 @@ using namespace std;
 typedef enum _SWAPCHAIN_ERROR
 {
     SWAPCHAIN_INVALID_HANDLE,                   // Handle used that isn't currently valid
+    SWAPCHAIN_NULL_POINTER,                     // Pointer set to NULL, instead of being a valid pointer
     SWAPCHAIN_EXT_NOT_ENABLED_BUT_USED,         // Did not enable WSI extension, but called WSI function 
     SWAPCHAIN_DEL_DEVICE_BEFORE_SWAPCHAINS,     // Called vkDestroyDevice() before vkDestroySwapchainKHR()
     SWAPCHAIN_CREATE_SWAP_WITHOUT_QUERY,        // Called vkCreateSwapchainKHR() without calling a query (e.g. vkGetPhysicalDeviceSurfaceCapabilitiesKHR())
@@ -82,6 +83,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 owned 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_INVALID_COUNT,                    // Second time a query called, the pCount value didn't match first time
 } SWAPCHAIN_ERROR;
 
 
@@ -93,7 +95,20 @@ typedef enum _SWAPCHAIN_ERROR
                 (uint64_t) (obj), __LINE__, SWAPCHAIN_INVALID_HANDLE, LAYER_NAME, \
                 "%s() called with a non-valid %s.", __FUNCTION__, (obj)) \
     : VK_FALSE
-
+#define LOG_ERROR_NULL_POINTER(objType, type, obj)                     \
+    (my_data) ?                                                         \
+        log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (objType), \
+                (uint64_t) (obj), 0, SWAPCHAIN_NULL_POINTER, LAYER_NAME, \
+                "%s() called with NULL pointer %s.", __FUNCTION__, (obj)) \
+    : VK_FALSE
+#define LOG_ERROR_INVALID_COUNT(objType, type, obj, obj2, val)          \
+    (my_data) ?                                                         \
+        log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (objType), \
+                (uint64_t) (obj), 0, SWAPCHAIN_INVALID_COUNT, LAYER_NAME, \
+                "%s() called with non-NULL %s, and with %s not matching " \
+                "the value (%d) that was returned when %s was NULL.",   \
+                __FUNCTION__, (obj2), (obj), (val), (obj2))             \
+    : VK_FALSE
 #define LOG_ERROR(objType, type, obj, enm, fmt, ...)                    \
     (my_data) ?                                                         \
         log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (objType), \
diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md
index 92904ad1..38b43d33 100644
--- a/layers/vk_validation_layer_details.md
+++ b/layers/vk_validation_layer_details.md
@@ -333,6 +333,7 @@ This layer is a work in progress. VK_LAYER_LUNARG_swapchain layer is intended to
 | Check | Overview | ENUM SWAPCHAIN_* | Relevant API | Testname | Notes/TODO |
 | ----- | -------- | ---------------- | ------------ | -------- | ---------- |
 | Valid handle | If an invalid handle is used, this error will be flagged | INVALID_HANDLE | vkDestroyInstance vkEnumeratePhysicalDevices vkCreateDevice vkDestroyDevice vkGetPhysicalDeviceSurfaceSupportKHR vkGetPhysicalDeviceSurfaceCapabilitiesKHR vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfacePresentModesKHR vkCreateSwapchainKHR vkDestroySwapchainKHR vkGetSwapchainImagesKHR vkAcquireNextImageKHR vkQueuePresentKHR | NA | None |
+| Valid pointer | If a NULL pointer is used, this error will be flagged | NULL_POINTER | vkGetPhysicalDeviceSurfaceSupportKHR vkGetPhysicalDeviceSurfaceCapabilitiesKHR vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfacePresentModesKHR vkGetSwapchainImagesKHR | NA | None |
 | Extension enabled before use | Validates that a WSI extension is enabled before its functions are used | EXT_NOT_ENABLED_BUT_USED |  vkGetPhysicalDeviceSurfaceSupportKHR vkGetPhysicalDeviceSurfaceCapabilitiesKHR vkGetPhysicalDeviceSurfaceFormatsKHR vkGetPhysicalDeviceSurfacePresentModesKHR vkCreateSwapchainKHR vkDestroySwapchainKHR vkGetSwapchainImagesKHR vkAcquireNextImageKHR vkQueuePresentKHR | NA | None |
 | Swapchains destroyed before devices | Validates that  vkDestroySwapchainKHR() is called for all swapchains associated with a device before vkDestroyDevice() is called | DEL_DEVICE_BEFORE_SWAPCHAINS | vkDestroyDevice | NA | None |
 | Queries occur before swapchain creation | Validates that vkGetPhysicalDeviceSurfaceCapabilitiesKHR(), vkGetPhysicalDeviceSurfaceFormatsKHR() and vkGetPhysicalDeviceSurfacePresentModesKHR() are called before vkCreateSwapchainKHR() | CREATE_SWAP_WITHOUT_QUERY | vkCreateSwapchainKHR | NA | None |
@@ -354,6 +355,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 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 |
 
 ### VK_LAYER_LUNARG_swapchain Pending Work
 Additional checks to be added to VK_LAYER_LUNARG_swapchain