gpu: Check index buffer contents in pre-draw validation

This involves splitting up draw.vert into separate shaders for non-indexed indirect, indexed (both indirect and direct) and mesh draw calls. Fixes #2492
KhronosGroup · Aug 13, 2024 · fd882c7 · fd882c7
1 parent 2d73636
commit fd882c7
Show file tree

Hide file tree

Showing 28 changed files with 1,974 additions and 609 deletions.
diff --git a/BUILD.gn b/BUILD.gn
@@ -153,8 +153,12 @@ vvl_sources = [
   "layers/gpu/cmd_validation/gpuav_copy_buffer_to_image.h",
   "layers/gpu/cmd_validation/gpuav_dispatch.cpp",
   "layers/gpu/cmd_validation/gpuav_dispatch.h",
-  "layers/gpu/cmd_validation/gpuav_draw.cpp",
-  "layers/gpu/cmd_validation/gpuav_draw.h",
+  "layers/gpu/cmd_validation/gpuav_indexed_draw.cpp",
+  "layers/gpu/cmd_validation/gpuav_indexed_draw.h",
+  "layers/gpu/cmd_validation/gpuav_indirect_draw.cpp",
+  "layers/gpu/cmd_validation/gpuav_indirect_draw.h",
+  "layers/gpu/cmd_validation/gpuav_mesh_draw.cpp",
+  "layers/gpu/cmd_validation/gpuav_mesh_draw.h",
   "layers/gpu/cmd_validation/gpuav_trace_rays.cpp",
   "layers/gpu/cmd_validation/gpuav_trace_rays.h",
   "layers/gpu/core/gpu_settings.h",

diff --git a/layers/CMakeLists.txt b/layers/CMakeLists.txt
@@ -213,8 +213,12 @@ target_sources(vvl PRIVATE
     ${API_TYPE}/generated/cmd_validation_copy_buffer_to_image_comp.cpp
     ${API_TYPE}/generated/cmd_validation_dispatch_comp.h
     ${API_TYPE}/generated/cmd_validation_dispatch_comp.cpp
-    ${API_TYPE}/generated/cmd_validation_draw_vert.h
-    ${API_TYPE}/generated/cmd_validation_draw_vert.cpp
+    ${API_TYPE}/generated/cmd_validation_indexed_draw_vert.h
+    ${API_TYPE}/generated/cmd_validation_indexed_draw_vert.cpp
+    ${API_TYPE}/generated/cmd_validation_indirect_draw_vert.h
+    ${API_TYPE}/generated/cmd_validation_indirect_draw_vert.cpp
+    ${API_TYPE}/generated/cmd_validation_mesh_draw_vert.h
+    ${API_TYPE}/generated/cmd_validation_mesh_draw_vert.cpp
     ${API_TYPE}/generated/cmd_validation_trace_rays_rgen.h
     ${API_TYPE}/generated/cmd_validation_trace_rays_rgen.cpp
     gpu/core/gpu_settings.h

diff --git a/layers/gpu/cmd_validation/gpuav_draw.cpp b/layers/gpu/cmd_validation/gpuav_draw.cpp
diff --git a/layers/gpu/cmd_validation/gpuav_draw.h b/layers/gpu/cmd_validation/gpuav_draw.h
@@ -21,13 +21,25 @@
 
 struct Location;
 
+namespace vvl {
+class Buffer;
+}
+
 namespace gpuav {
 class Validator;
 
 void DestroyRenderPassMappedResources(Validator &gpuav, VkRenderPass render_pass);
 
-void InsertIndirectDrawValidation(Validator &gpuav, const Location &loc, CommandBuffer &cb_state, VkBuffer indirect_buffer,
-                                  VkDeviceSize indirect_offset, uint32_t draw_count, VkBuffer count_buffer,
-                                  VkDeviceSize count_buffer_offset, uint32_t stride);
+void InsertIndirectDrawValidation(Validator &gpuav, const Location &loc, CommandBuffer &cb_state,
+                                  vvl::Buffer &indirect_buffer_state, VkDeviceSize indirect_offset, uint32_t draw_count,
+                                  VkBuffer count_buffer, VkDeviceSize count_buffer_offset, uint32_t stride);
+
+void InsertIndexedDrawValidation(Validator &gpuav, const Location &loc, CommandBuffer &cb_state, VkBuffer indirect_buffer,
+                                 VkDeviceSize indirect_offset, uint32_t draw_count, VkBuffer count_buffer,
+                                 VkDeviceSize count_buffer_offset, uint32_t stride, uint32_t first_index, uint32_t index_count);
+
+void InsertMeshDrawValidation(Validator &gpuav, const Location &loc, CommandBuffer &cb_state, vvl::Buffer &indirect_buffer_state,
+                              VkDeviceSize indirect_offset, uint32_t draw_count, VkBuffer count_buffer,
+                              VkDeviceSize count_buffer_offset, uint32_t stride);
 
 }  // namespace gpuav
diff --git a/layers/gpu/core/gpuav_record.cpp b/layers/gpu/core/gpuav_record.cpp
@@ -337,6 +337,8 @@ void Validator::PreCallRecordCmdDrawIndexed(VkCommandBuffer commandBuffer, uint3
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
+    InsertIndexedDrawValidation(*this, record_obj.location, *cb_state, VK_NULL_HANDLE, 0, 0, VK_NULL_HANDLE, 0, 0, firstIndex,
+                                indexCount);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -367,7 +369,14 @@ void Validator::PreCallRecordCmdDrawIndirect(VkCommandBuffer commandBuffer, VkBu
         return;
     }
 
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, count, VK_NULL_HANDLE, 0, stride);
+    auto indirect_buffer_state = Get<vvl::Buffer>(buffer);
+    if (!indirect_buffer_state) {
+        InternalError(LogObjectList(commandBuffer, buffer), record_obj.location, "buffer must be a valid VkBuffer handle");
+        return;
+    }
+
+    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, *indirect_buffer_state, offset, count, VK_NULL_HANDLE, 0,
+                                 stride);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -381,7 +390,7 @@ void Validator::PreCallRecordCmdDrawIndexedIndirect(VkCommandBuffer commandBuffe
         return;
     }
 
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, count, VK_NULL_HANDLE, 0, stride);
+    InsertIndexedDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, count, VK_NULL_HANDLE, 0, stride, 0, 0);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -403,7 +412,14 @@ void Validator::PreCallRecordCmdDrawIndirectCount(VkCommandBuffer commandBuffer,
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, maxDrawCount, countBuffer,
+
+    auto indirect_buffer_state = Get<vvl::Buffer>(buffer);
+    if (!indirect_buffer_state) {
+        InternalError(LogObjectList(commandBuffer, buffer), record_obj.location, "buffer must be a valid VkBuffer handle");
+        return;
+    }
+
+    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, *indirect_buffer_state, offset, maxDrawCount, countBuffer,
                                  countBufferOffset, stride);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
@@ -441,8 +457,8 @@ void Validator::PreCallRecordCmdDrawIndexedIndirectCount(VkCommandBuffer command
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, maxDrawCount, countBuffer,
-                                 countBufferOffset, stride);
+    InsertIndexedDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, maxDrawCount, countBuffer, countBufferOffset,
+                                stride, 0, 0);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -466,7 +482,13 @@ void Validator::PreCallRecordCmdDrawMeshTasksIndirectNV(VkCommandBuffer commandB
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, drawCount, VK_NULL_HANDLE, 0, stride);
+    auto indirect_buffer_state = Get<vvl::Buffer>(buffer);
+    if (!indirect_buffer_state) {
+        InternalError(LogObjectList(commandBuffer, buffer), record_obj.location, "buffer must be a valid VkBuffer handle");
+        return;
+    }
+    InsertMeshDrawValidation(*this, record_obj.location, *cb_state, *indirect_buffer_state, offset, drawCount, VK_NULL_HANDLE, 0,
+                             stride);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -482,8 +504,13 @@ void Validator::PreCallRecordCmdDrawMeshTasksIndirectCountNV(VkCommandBuffer com
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, maxDrawCount, countBuffer,
-                                 countBufferOffset, stride);
+    auto indirect_buffer_state = Get<vvl::Buffer>(buffer);
+    if (!indirect_buffer_state) {
+        InternalError(LogObjectList(commandBuffer, buffer), record_obj.location, "buffer must be a valid VkBuffer handle");
+        return;
+    }
+    InsertMeshDrawValidation(*this, record_obj.location, *cb_state, *indirect_buffer_state, offset, maxDrawCount, countBuffer,
+                             countBufferOffset, stride);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -507,7 +534,13 @@ void Validator::PreCallRecordCmdDrawMeshTasksIndirectEXT(VkCommandBuffer command
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, drawCount, VK_NULL_HANDLE, 0, stride);
+    auto indirect_buffer_state = Get<vvl::Buffer>(buffer);
+    if (!indirect_buffer_state) {
+        InternalError(LogObjectList(commandBuffer, buffer), record_obj.location, "buffer must be a valid VkBuffer handle");
+        return;
+    }
+    InsertMeshDrawValidation(*this, record_obj.location, *cb_state, *indirect_buffer_state, offset, drawCount, VK_NULL_HANDLE, 0,
+                             stride);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 
@@ -523,8 +556,13 @@ void Validator::PreCallRecordCmdDrawMeshTasksIndirectCountEXT(VkCommandBuffer co
         InternalError(commandBuffer, record_obj.location, "Unrecognized command buffer.");
         return;
     }
-    InsertIndirectDrawValidation(*this, record_obj.location, *cb_state, buffer, offset, maxDrawCount, countBuffer,
-                                 countBufferOffset, stride);
+    auto indirect_buffer_state = Get<vvl::Buffer>(buffer);
+    if (!indirect_buffer_state) {
+        InternalError(LogObjectList(commandBuffer, buffer), record_obj.location, "buffer must be a valid VkBuffer handle");
+        return;
+    }
+    InsertMeshDrawValidation(*this, record_obj.location, *cb_state, *indirect_buffer_state, offset, maxDrawCount, countBuffer,
+                             countBufferOffset, stride);
     SetupShaderInstrumentationResources(*this, *cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS, record_obj.location);
 }
 

diff --git a/layers/gpu/error_message/gpuav_vuids.cpp b/layers/gpu/error_message/gpuav_vuids.cpp
@@ -46,6 +46,9 @@ struct GpuVuidsCmdDrawIndexed : GpuVuid {
         uniform_access_oob_08612 = "VUID-vkCmdDrawIndexed-None-08612";
         storage_access_oob_08613 = "VUID-vkCmdDrawIndexed-None-08613";
         invalid_descriptor = "VUID-vkCmdDrawIndexed-None-08114";
+        // This should be detected on the host by core validation but just in case...
+        index_buffer_size = "VUID-vkCmdDrawIndexedEXT-robustBufferAccess2-08798";
+        vertex_index_oob = "VUID-vkCmdDrawIndexed-None-02721";
     }
 };
 
@@ -56,6 +59,8 @@ struct GpuVuidsCmdDrawMultiIndexedEXT : GpuVuid {
         uniform_access_oob_08612 = "VUID-vkCmdDrawMultiIndexedEXT-None-08612";
         storage_access_oob_08613 = "VUID-vkCmdDrawMultiIndexedEXT-None-08613";
         invalid_descriptor = "VUID-vkCmdDrawMultiIndexedEXT-None-08114";
+        index_buffer_size = "VUID-vkCmdDrawMultiIndexedEXT-robustBufferAccess2-08798";
+        vertex_index_oob = "VUID-vkCmdDrawMultiIndexedEXT-None-02721";
     }
 };
 
@@ -78,6 +83,8 @@ struct GpuVuidsCmdDrawIndexedIndirect : GpuVuid {
         storage_access_oob_08613 = "VUID-vkCmdDrawIndexedIndirect-None-08613";
         invalid_descriptor = "VUID-vkCmdDrawIndexedIndirect-None-08114";
         first_instance_not_zero = "VUID-VkDrawIndexedIndirectCommand-firstInstance-00554";
+        index_buffer_size = "VUID-vkCmdDrawIndexedIndirect-robustBufferAccess2-08798";
+        vertex_index_oob = "VUID-vkCmdDrawIndexedIndirect-None-02721";
     }
 };
 
@@ -128,6 +135,8 @@ struct GpuVuidsCmdDrawIndexedIndirectCount : GpuVuid {
         count_exceeds_bufsize_1 = "VUID-vkCmdDrawIndexedIndirectCount-countBuffer-03153";
         count_exceeds_bufsize = "VUID-vkCmdDrawIndexedIndirectCount-countBuffer-03154";
         count_exceeds_device_limit = "VUID-vkCmdDrawIndexedIndirectCount-countBuffer-02717";
+        index_buffer_size = "VUID-vkCmdDrawIndexedIndirectCount-robustBufferAccess2-08798";
+        vertex_index_oob = "VUID-vkCmdDrawIndexedIndirectCount-None-02721";
     }
 };
 

diff --git a/layers/gpu/error_message/gpuav_vuids.h b/layers/gpu/error_message/gpuav_vuids.h
@@ -46,6 +46,8 @@ struct GpuVuid {
     const char* trace_rays_width_exceeds_device_limit = kVUIDUndefined;
     const char* trace_rays_height_exceeds_device_limit = kVUIDUndefined;
     const char* trace_rays_depth_exceeds_device_limit = kVUIDUndefined;
+    const char* index_buffer_size = kVUIDUndefined;
+    const char* vertex_index_oob = kVUIDUndefined;
 };
 
 // Getter function to provide kVUIDUndefined in case an invalid function is passed in

diff --git a/layers/gpu/shaders/cmd_validation/draw.vert b/layers/gpu/shaders/cmd_validation/draw.vert