layers: LX257, Validate Uniform/Storage buffer offset alignments

7 files changed, 142 insertions, 22 deletions

diff --git a/layers/device_limits.cpp b/layers/device_limits.cpp
index e46149fb..fbb9af3b 100644
--- a/layers/device_limits.cpp
+++ b/layers/device_limits.cpp
@@ -58,17 +58,18 @@ struct devExts {
 
 // This struct will be stored in a map hashed by the dispatchable object
 struct layer_data {
-    debug_report_data *report_data;
-    std::vector<VkDebugReportCallbackEXT> logging_callback;
-    VkLayerDispatchTable* device_dispatch_table;
-    VkLayerInstanceDispatchTable* instance_dispatch_table;
-    devExts device_extensions;
+    debug_report_data                                  *report_data;
+    std::vector<VkDebugReportCallbackEXT>               logging_callback;
+    VkLayerDispatchTable                               *device_dispatch_table;
+    VkLayerInstanceDispatchTable                       *instance_dispatch_table;
+    devExts                                             device_extensions;
     // Track state of each instance
-    unique_ptr<INSTANCE_STATE> instanceState;
-    unique_ptr<PHYSICAL_DEVICE_STATE> physicalDeviceState;
-    VkPhysicalDeviceFeatures actualPhysicalDeviceFeatures;
-    VkPhysicalDeviceFeatures requestedPhysicalDeviceFeatures;
-    VkPhysicalDeviceProperties physicalDeviceProperties;
+    unique_ptr<INSTANCE_STATE>                          instanceState;
+    unique_ptr<PHYSICAL_DEVICE_STATE>                   physicalDeviceState;
+    VkPhysicalDeviceFeatures                            actualPhysicalDeviceFeatures;
+    VkPhysicalDeviceFeatures                            requestedPhysicalDeviceFeatures;
+    unordered_map<VkDevice, VkPhysicalDeviceProperties> physDevPropertyMap;
+
     // Track physical device per logical device
     VkPhysicalDevice physicalDevice;
     // Vector indices correspond to queueFamilyIndex
@@ -81,7 +82,6 @@ struct layer_data {
         device_extensions(),
         instanceState(nullptr),
         physicalDeviceState(nullptr),
-        physicalDeviceProperties(),
         actualPhysicalDeviceFeatures(),
         requestedPhysicalDeviceFeatures(),
         physicalDevice()
@@ -257,7 +257,6 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumeratePhysicalDevices(VkInst
                 phy_dev_data->physicalDeviceState = unique_ptr<PHYSICAL_DEVICE_STATE>(new PHYSICAL_DEVICE_STATE());
                 // Init actual features for each physical device
                 my_data->instance_dispatch_table->GetPhysicalDeviceFeatures(pPhysicalDevices[i], &(phy_dev_data->actualPhysicalDeviceFeatures));
-                my_data->instance_dispatch_table->GetPhysicalDeviceProperties(pPhysicalDevices[i], &(phy_dev_data->physicalDeviceProperties));
             }
         }
         return result;
@@ -449,13 +448,17 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateDevice(VkPhysicalDevice g
     if (skipCall)
         return VK_ERROR_VALIDATION_FAILED_EXT;
 
-    layer_data *my_device_data = get_my_data_ptr(get_dispatch_key(*pDevice), layer_data_map);
-    VkResult result = my_device_data->device_dispatch_table->CreateDevice(gpu, pCreateInfo, pAllocator, pDevice);
+    layer_data *device_data = get_my_data_ptr(get_dispatch_key(*pDevice), layer_data_map);
+    VkResult result = device_data->device_dispatch_table->CreateDevice(gpu, pCreateInfo, pAllocator, pDevice);
     if (result == VK_SUCCESS) {
-        my_device_data->report_data = layer_debug_report_create_device(phy_dev_data->report_data, *pDevice);
+        device_data->report_data = layer_debug_report_create_device(phy_dev_data->report_data, *pDevice);
         createDeviceRegisterExtensions(pCreateInfo, *pDevice);
-        my_device_data->physicalDevice = gpu;
+        device_data->physicalDevice = gpu;
+
+        // Get physical device properties for this device
+        phy_dev_data->instance_dispatch_table->GetPhysicalDeviceProperties(gpu, &(phy_dev_data->physDevPropertyMap[*pDevice]));
     }
+
     return result;
 }
 
@@ -517,6 +520,71 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkGetDeviceQueue(VkDevice device, uin
     dev_data->device_dispatch_table->GetDeviceQueue(device, queueFamilyIndex, queueIndex, pQueue);
 }
 
+VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkBindBufferMemory(
+    VkDevice       device,
+    VkBuffer       buffer,
+    VkDeviceMemory mem,
+    VkDeviceSize   memoryOffset)
+{
+    layer_data *dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map);
+    VkResult    result   = VK_ERROR_VALIDATION_FAILED_EXT;
+    VkBool32    skipCall = VK_FALSE;
+
+    VkDeviceSize uniformAlignment = dev_data->physDevPropertyMap[device].limits.minUniformBufferOffsetAlignment;
+    if (vk_safe_modulo(memoryOffset, uniformAlignment) != 0) {
+        skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, 0,
+        __LINE__, DEVLIMITS_INVALID_UNIFORM_BUFFER_OFFSET, "DL",
+        "vkBindBufferMemory(): memoryOffset %#" PRIxLEAST64 " must be a multiple of device limit minUniformBufferOffsetAlignment %#" PRIxLEAST64,
+        memoryOffset, uniformAlignment);
+    }
+
+    if (VK_FALSE == skipCall) {
+        result = dev_data->device_dispatch_table->BindBufferMemory(device, buffer, mem, memoryOffset);
+    }
+    return result;
+}
+
+VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkUpdateDescriptorSets(
+    VkDevice                    device,
+    uint32_t                    descriptorWriteCount,
+    const VkWriteDescriptorSet *pDescriptorWrites,
+    uint32_t                    descriptorCopyCount,
+    const VkCopyDescriptorSet  *pDescriptorCopies)
+{
+    layer_data* dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map);
+    VkResult    result   = VK_ERROR_VALIDATION_FAILED_EXT;
+    VkBool32    skipCall = VK_FALSE;
+
+    for (auto i = 0; i < descriptorWriteCount; i++) {
+        if ((pDescriptorWrites[i].descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER)         ||
+            (pDescriptorWrites[i].descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC))  {
+            VkDeviceSize uniformAlignment = dev_data->physDevPropertyMap[device].limits.minUniformBufferOffsetAlignment;
+            for (auto j = 0; j < pDescriptorWrites[i].descriptorCount; j++) {
+                if (vk_safe_modulo(pDescriptorWrites[i].pBufferInfo[j].offset, uniformAlignment) != 0) {
+                    skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, 0,
+                    __LINE__, DEVLIMITS_INVALID_UNIFORM_BUFFER_OFFSET, "DL",
+                    "vkUpdateDescriptorSets(): pDescriptorWrites[%d].pBufferInfo[%d].offset (%#" PRIxLEAST64 ") must be a multiple of device limit minUniformBufferOffsetAlignment %#" PRIxLEAST64,
+                    i, j, pDescriptorWrites[i].pBufferInfo[j].offset, uniformAlignment);
+                }
+            }
+        } else if ((pDescriptorWrites[i].descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER)         ||
+                   (pDescriptorWrites[i].descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC))  {
+            VkDeviceSize storageAlignment = dev_data->physDevPropertyMap[device].limits.minStorageBufferOffsetAlignment;
+            for (auto j = 0; j < pDescriptorWrites[i].descriptorCount; j++) {
+                if (vk_safe_modulo(pDescriptorWrites[i].pBufferInfo[j].offset, storageAlignment) != 0) {
+                    skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, 0,
+                    __LINE__, DEVLIMITS_INVALID_STORAGE_BUFFER_OFFSET, "DL",
+                    "vkUpdateDescriptorSets(): pDescriptorWrites[%d].pBufferInfo[%d].offset (%#" PRIxLEAST64 ") must be a multiple of device limit minStorageBufferOffsetAlignment %#" PRIxLEAST64,
+                    i, j, pDescriptorWrites[i].pBufferInfo[j].offset, storageAlignment);
+                }
+            }
+        }
+    }
+    if (skipCall == VK_FALSE) {
+        dev_data->device_dispatch_table->UpdateDescriptorSets(device, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, pDescriptorCopies);
+    }
+}
+
 VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdUpdateBuffer(
     VkCommandBuffer commandBuffer,
     VkBuffer dstBuffer,
@@ -648,6 +716,10 @@ VK_LAYER_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL vkGetDeviceProcAddr(VkD
         return (PFN_vkVoidFunction) vkFreeCommandBuffers;
     if (!strcmp(funcName, "vkCmdUpdateBuffer"))
         return (PFN_vkVoidFunction) vkCmdUpdateBuffer;
+    if (!strcmp(funcName, "vkBindBufferMemory"))
+        return (PFN_vkVoidFunction) vkBindBufferMemory;
+    if (!strcmp(funcName, "vkUpdateDescriptorSets"))
+        return (PFN_vkVoidFunction) vkUpdateDescriptorSets;
     if (!strcmp(funcName, "vkCmdFillBuffer"))
         return (PFN_vkVoidFunction) vkCmdFillBuffer;
 
diff --git a/layers/device_limits.h b/layers/device_limits.h
index 944d5b71..5caba24c 100644
--- a/layers/device_limits.h
+++ b/layers/device_limits.h
@@ -41,6 +41,8 @@ typedef enum _DEV_LIMITS_ERROR
     DEVLIMITS_COUNT_MISMATCH,                   // App requesting a count value different than actual value
     DEVLIMITS_INVALID_QUEUE_CREATE_REQUEST,     // Invalid queue requested based on queue family properties
     DEVLIMITS_LIMITS_VIOLATION,                 // Driver-specified limits/properties were exceeded
+    DEVLIMITS_INVALID_UNIFORM_BUFFER_OFFSET,    // Uniform buffer offset violates device limit granularity
+    DEVLIMITS_INVALID_STORAGE_BUFFER_OFFSET,    // Storage buffer offset violates device limit granularity
 } DEV_LIMITS_ERROR;
 
 typedef enum _CALL_STATE
diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp
index a9c809bd..e2a81fb1 100644
--- a/layers/draw_state.cpp
+++ b/layers/draw_state.cpp
@@ -123,6 +123,7 @@ struct layer_data {
     // Current render pass
     VkRenderPassBeginInfo                renderPassBeginInfo;
     uint32_t                             currentSubpass;
+    unordered_map<VkDevice,              VkPhysicalDeviceProperties>         physDevPropertyMap;
 
     layer_data() :
         report_data(nullptr),
@@ -2931,13 +2932,15 @@ static void createDeviceRegisterExtensions(const VkDeviceCreateInfo* pCreateInfo
 
 VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateDevice(VkPhysicalDevice gpu, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice)
 {
-    layer_data* dev_data = get_my_data_ptr(get_dispatch_key(*pDevice), layer_data_map);
-    VkResult result = dev_data->device_dispatch_table->CreateDevice(gpu, pCreateInfo, pAllocator, pDevice);
-    // TODOSC : shouldn't need any customization here
+    layer_data *instance_data = get_my_data_ptr(get_dispatch_key(gpu), layer_data_map);
+    layer_data *dev_data      = get_my_data_ptr(get_dispatch_key(*pDevice), layer_data_map);
+    VkResult    result        = dev_data->device_dispatch_table->CreateDevice(gpu, pCreateInfo, pAllocator, pDevice);
+
     if (result == VK_SUCCESS) {
-        layer_data *my_instance_data = get_my_data_ptr(get_dispatch_key(gpu), layer_data_map);
-        dev_data->report_data = layer_debug_report_create_device(my_instance_data->report_data, *pDevice);
+        dev_data->report_data = layer_debug_report_create_device(instance_data->report_data, *pDevice);
         createDeviceRegisterExtensions(pCreateInfo, *pDevice);
+        // Get physical device limits for this device
+        instance_data->instance_dispatch_table->GetPhysicalDeviceProperties(gpu, &(instance_data->physDevPropertyMap[*pDevice]));
     }
     return result;
 }
@@ -4002,6 +4005,7 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkAllocateCommandBuffers(VkDevice
                 resetCB(dev_data, pCommandBuffer[i]);
                 pCB->commandBuffer = pCommandBuffer[i];
                 pCB->createInfo    = *pCreateInfo;
+                pCB->device        = device;
             }
         }
     }
@@ -4371,7 +4375,29 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdBindDescriptorSets(VkCommandBuff
                                 skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, (uint64_t) pDescriptorSets[i], __LINE__, DRAWSTATE_INVALID_DYNAMIC_OFFSET_COUNT, "DS",
                                     "descriptorSet #%u (%#" PRIxLEAST64 ") requires %u dynamicOffsets, but only %u dynamicOffsets are left in pDynamicOffsets array. There must be one dynamic offset for each dynamic descriptor being bound.",
                                         i, (uint64_t) pDescriptorSets[i], pSet->pLayout->dynamicDescriptorCount, (dynamicOffsetCount - totalDynamicDescriptors));
-                            } else { // Store dynamic offsets with the set
+                            } else { // Validate and store dynamic offsets with the set
+                                // Validate Dynamic Offset Minimums
+                                uint32_t cur_dyn_offset = totalDynamicDescriptors;
+                                for (uint32_t d = 0; d < pSet->descriptorCount; d++) {
+                                    if (pSet->pLayout->descriptorTypes[i] == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC) {
+                                        if (vk_safe_modulo(pDynamicOffsets[cur_dyn_offset], dev_data->physDevPropertyMap[pCB->device].limits.minUniformBufferOffsetAlignment) != 0) {
+                                            skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, 0,
+                                            __LINE__, DRAWSTATE_INVALID_UNIFORM_BUFFER_OFFSET, "DS",
+                                            "vkCmdBindDescriptorSets(): pDynamicOffsets[%d] is %d but must be a multiple of device limit minUniformBufferOffsetAlignment %#" PRIxLEAST64,
+                                            cur_dyn_offset, pDynamicOffsets[cur_dyn_offset], dev_data->physDevPropertyMap[pCB->device].limits.minUniformBufferOffsetAlignment);
+                                        }
+                                        cur_dyn_offset++;
+                                    } else if (pSet->pLayout->descriptorTypes[i] == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC) {
+                                        if (vk_safe_modulo(pDynamicOffsets[cur_dyn_offset], dev_data->physDevPropertyMap[pCB->device].limits.minStorageBufferOffsetAlignment) != 0) {
+                                            skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, 0,
+                                            __LINE__, DRAWSTATE_INVALID_STORAGE_BUFFER_OFFSET, "DS",
+                                            "vkCmdBindDescriptorSets(): pDynamicOffsets[%d] is %d but must be a multiple of device limit minStorageBufferOffsetAlignment %#" PRIxLEAST64,
+                                            cur_dyn_offset, pDynamicOffsets[cur_dyn_offset], dev_data->physDevPropertyMap[pCB->device].limits.minStorageBufferOffsetAlignment);
+                                        }
+                                        cur_dyn_offset++;
+                                    }
+                                }
+                                // Store offsets
                                 const uint32_t* pStartDynOffset = pDynamicOffsets + totalDynamicDescriptors;
                                 pSet->dynamicOffsets.assign(pStartDynOffset, pStartDynOffset + pSet->pLayout->dynamicDescriptorCount);
                                 totalDynamicDescriptors += pSet->pLayout->dynamicDescriptorCount;
diff --git a/layers/draw_state.h b/layers/draw_state.h
index e632e7e1..4f20d474 100755
--- a/layers/draw_state.h
+++ b/layers/draw_state.h
@@ -99,6 +99,8 @@ typedef enum _DRAW_STATE_ERROR
     DRAWSTATE_DOUBLE_DESTROY,                   // Destroying an object twice
     DRAWSTATE_OBJECT_INUSE,                     // Destroying an object in use by a command buffer
     DRAWSTATE_QUEUE_FORWARD_PROGRESS,           // Queue cannot guarantee forward progress
+    DRAWSTATE_INVALID_UNIFORM_BUFFER_OFFSET,    // Dynamic Uniform Buffer Offsets violate device limit
+    DRAWSTATE_INVALID_STORAGE_BUFFER_OFFSET,    // Dynamic Storage Buffer Offsets violate device limit
 } DRAW_STATE_ERROR;
 
 typedef enum _SHADER_CHECKER_ERROR {
@@ -476,6 +478,7 @@ typedef struct _GLOBAL_CB_NODE {
     VkCommandBufferAllocateInfo  createInfo;
     VkCommandBufferBeginInfo     beginInfo;
     VkFence                      fence;    // fence tracking this cmd buffer
+    VkDevice                     device;   // device this DB belongs to
     uint64_t                     numCmds;  // number of cmds in this CB
     uint64_t                     drawCount[NUM_DRAW_TYPES]; // Count of each type of draw in this CB
     CB_STATE                     state;  // Track cmd buffer update state
diff --git a/layers/vk_layer_utils.cpp b/layers/vk_layer_utils.cpp
index 4eb1e905..cfa2f9b7 100644
--- a/layers/vk_layer_utils.cpp
+++ b/layers/vk_layer_utils.cpp
@@ -584,3 +584,15 @@ unsigned int vk_format_get_channel_count(VkFormat format)
 {
     return vk_format_table[format].channel_count;
 }
+
+// Perform a zero-tolerant modulo operation
+VkDeviceSize vk_safe_modulo(VkDeviceSize dividend, VkDeviceSize divisor)
+{
+    VkDeviceSize result = 0;
+    if (divisor != 0) {
+        result = dividend % divisor;
+    }
+    return result;
+}
+
+
diff --git a/layers/vk_layer_utils.h b/layers/vk_layer_utils.h
index 5e345025..c3759954 100644
--- a/layers/vk_layer_utils.h
+++ b/layers/vk_layer_utils.h
@@ -111,6 +111,7 @@ bool                       vk_format_is_compressed(VkFormat format);
 size_t                     vk_format_get_size(VkFormat format);
 unsigned int               vk_format_get_channel_count(VkFormat format);
 VkFormatCompatibilityClass vk_format_get_compatibility_class(VkFormat format);
+VkDeviceSize               vk_safe_modulo(VkDeviceSize dividend, VkDeviceSize divisor);
 
 static inline int u_ffs(int val)
 {
diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md
index 1011c3e5..7cde37d2 100644
--- a/layers/vk_validation_layer_details.md
+++ b/layers/vk_validation_layer_details.md
@@ -71,6 +71,8 @@ The VK_LAYER_LUNARG_draw_state layer tracks state leading into Draw cmds. This i
 | Verify Memory Buffer Not In Use | Validate that memory buffer being freed is not in use | OBJECT_INUSE | vkDestroyBuffer | TBD | None |
 | Verify Get Queries| Validate that that queries are properly initialized and synchronized | INVALID_QUERY | vkGetFenceStatus vkQueueWaitIdle vkWaitForFences vkDeviceWaitIdle | TBD | None |
 | Live Semaphore  | When waiting on a semaphore, need to make sure that the semaphore is live and therefore can be signalled, otherwise queue is stalled and cannot make forward progress. | QUEUE_FORWARD_PROGRESS | vkQueueSubmit vkQueueBindSparse vkQueuePresentKHR vkAcquireNextImageKHR | TODO | Create test |
+| Storage Buffer Alignment  | Storage Buffer offsets in BindDescriptorSets must agree with offset alignment device limit | INVALID_STORAGE_BUFFER_OFFSET | vkCmdBindDescriptorSets | TODO | Create test |
+| Uniform Buffer Alignment  | Uniform Buffer offsets in BindDescriptorSets must agree with offset alignment device limit | INVALID_UNIFORM_BUFFER_OFFSET | vkCmdBindDescriptorSets | TODO | Create test |
 | NA | Enum used for informational messages | NONE | | NA | None |
 | NA | Enum used for errors in the layer itself. This does not indicate an app issue, but instead a bug in the layer. | INTERNAL_ERROR | | NA | None |
 | NA | Enum used when VK_LAYER_LUNARG_draw_state attempts to allocate memory for its own internal use and is unable to. | OUT_OF_MEMORY | | NA | None |
@@ -322,6 +324,8 @@ For the second category of errors, VK_LAYER_LUNARG_device_limits stores its own
 | Valid Image Resource Size | When creating an image, ensure the the total image resource size is less than the queried device maximum resource size  | LIMITS_VIOLATION | vkCreateImage | CreateImageResourceSizeViolation | NA |
 | Alignment | When updating a buffer, data should be aligned on 4 byte boundaries  | LIMITS_VIOLATION | vkCmdUpdateBuffer | UpdateBufferAlignment | NA |
 | Alignment | When filling a buffer, data should be aligned on 4 byte boundaries  | LIMITS_VIOLATION | vkCmdFillBuffer | UpdateBufferAlignment | NA |
+| Storage Buffer Alignment  | Storage Buffer offsets must agree with offset alignment device limit | INVALID_STORAGE_BUFFER_OFFSET | vkBindBufferMemory vkUpdateDescriptorSets | TODO | Create test |
+| Uniform Buffer Alignment  | Uniform Buffer offsets must agree with offset alignment device limit | INVALID_UNIFORM_BUFFER_OFFSET | vkBindBufferMemory vkUpdateDescriptorSets | TODO | Create test |
 | NA | Enum used for informational messages | NONE | | NA | None |
 
 ### VK_LAYER_LUNARG_device_limits Pending Work