Skip to content

Commit

Permalink
gpu: Check index buffer contents in pre-draw validation
Browse files Browse the repository at this point in the history
This involves splitting up draw.vert into separate shaders for
non-indexed indirect, indexed (both indirect and direct) and mesh
draw calls.

Fixes #2492
  • Loading branch information
jeremyg-lunarg authored and arno-lunarg committed Aug 19, 2024
1 parent 8d902c7 commit 37ae96d
Show file tree
Hide file tree
Showing 31 changed files with 2,621 additions and 616 deletions.
8 changes: 6 additions & 2 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,12 @@ vvl_sources = [
"layers/vulkan/generated/cmd_validation_copy_buffer_to_image_comp.h",
"layers/vulkan/generated/cmd_validation_dispatch_comp.cpp",
"layers/vulkan/generated/cmd_validation_dispatch_comp.h",
"layers/vulkan/generated/cmd_validation_draw_vert.cpp",
"layers/vulkan/generated/cmd_validation_draw_vert.h",
"layers/vulkan/generated/cmd_validation_indexed_draw_vert.h",
"layers/vulkan/generated/cmd_validation_indexed_draw_vert.cpp",
"layers/vulkan/generated/cmd_validation_indirect_draw_vert.h",
"layers/vulkan/generated/cmd_validation_indirect_draw_vert.cpp",
"layers/vulkan/generated/cmd_validation_mesh_draw_vert.h",
"layers/vulkan/generated/cmd_validation_mesh_draw_vert.cpp",
"layers/vulkan/generated/cmd_validation_trace_rays_rgen.cpp",
"layers/vulkan/generated/cmd_validation_trace_rays_rgen.h",
"layers/vulkan/generated/command_validation.cpp",
Expand Down
8 changes: 6 additions & 2 deletions layers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
838 changes: 629 additions & 209 deletions layers/gpu/cmd_validation/gpuav_draw.cpp

Large diffs are not rendered by default.

18 changes: 15 additions & 3 deletions layers/gpu/cmd_validation/gpuav_draw.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 InsertIndirectMeshDrawValidation(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
2 changes: 1 addition & 1 deletion layers/gpu/core/gpuav.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ class Validator : public gpu::GpuShaderInstrumentor {

public:
std::optional<DescriptorHeap> desc_heap_{}; // optional only to defer construction
gpu::SharedResourcesManager shared_resources_manager;
gpu::SharedResourcesCache shared_resources_manager;

private:
std::string instrumented_shader_cache_path_{};
Expand Down
67 changes: 55 additions & 12 deletions layers/gpu/core/gpuav_record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,15 @@ void Validator::PreCallRecordCreateBuffer(VkDevice device, const VkBufferCreateI

// Indirect buffers will require validation shader to bind the indirect buffers as a storage buffer.
if (gpuav_settings.IsBufferValidationEnabled() &&
chassis_state.modified_create_info.usage & VK_BUFFER_USAGE_INDIRECT_BUFFER_BIT) {
(chassis_state.modified_create_info.usage & (VK_BUFFER_USAGE_INDIRECT_BUFFER_BIT | VK_BUFFER_USAGE_INDEX_BUFFER_BIT))) {
chassis_state.modified_create_info.usage |= VK_BUFFER_USAGE_STORAGE_BUFFER_BIT;
}

// Align index buffer size to 4: validation shader reads DWORDS
if (gpuav_settings.IsBufferValidationEnabled()) {
chassis_state.modified_create_info.size = Align<VkDeviceSize>(chassis_state.modified_create_info.size, 4);
}

BaseClass::PreCallRecordCreateBuffer(device, pCreateInfo, pAllocator, pBuffer, record_obj, chassis_state);
}

Expand Down Expand Up @@ -337,6 +342,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);
}

Expand Down Expand Up @@ -367,7 +374,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);
}

Expand All @@ -381,7 +395,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);
}

Expand All @@ -403,7 +417,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);
}
Expand Down Expand Up @@ -441,8 +462,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);
}

Expand All @@ -466,7 +487,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;
}
InsertIndirectMeshDrawValidation(*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);
}

Expand All @@ -482,8 +509,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;
}
InsertIndirectMeshDrawValidation(*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);
}

Expand All @@ -507,7 +539,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;
}
InsertIndirectMeshDrawValidation(*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);
}

Expand All @@ -523,8 +561,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;
}
InsertIndirectMeshDrawValidation(*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);
}

Expand Down
9 changes: 9 additions & 0 deletions layers/gpu/error_message/gpuav_vuids.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
};

Expand All @@ -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";
}
};

Expand All @@ -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";
}
};

Expand Down Expand Up @@ -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";
}
};

Expand Down
2 changes: 2 additions & 0 deletions layers/gpu/error_message/gpuav_vuids.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion layers/gpu/resources/gpu_resources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ void DescriptorSetManager::PutBackDescriptorSet(VkDescriptorPool desc_pool, VkDe
return;
}

void SharedResourcesManager::Clear() {
void SharedResourcesCache::Clear() {
for (auto &[key, value] : shared_validation_resources_map_) {
auto &[object, destructor] = value;
destructor(object);
Expand Down
7 changes: 6 additions & 1 deletion layers/gpu/resources/gpu_resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ class GpuResourcesManager {
std::vector<gpu::DeviceMemoryBlock> mem_blocks_;
};

class SharedResourcesManager {
// Cache a single object of type T. Key is *only* based on typeid(T)
class SharedResourcesCache {
public:
template <typename T>
T *TryGet() {
Expand All @@ -84,6 +85,10 @@ class SharedResourcesManager {
return t;
}

// First call to Get<T> will create the object, subsequent calls will retrieve the cached entry.
// /!\ The cache key is only based on the type T, not on the passed parameters
// => Successive calls to Get<T> with different parameters will NOT give different objects,
// only the entry cached upon the first call to Get<T> will be retrieved
template <typename T, class... ConstructorTypes>
T &Get(ConstructorTypes &&...args) {
T *t = TryGet<T>();
Expand Down
Loading

0 comments on commit 37ae96d

Please sign in to comment.