Skip to content

Commit

Permalink
gpu: Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@arno-lunarg
arno-lunarg committed Sep 12, 2024
1 parent 84b847a commit e566158
Show file tree
Hide file tree
Showing 17 changed files with 578 additions and 185 deletions.
20 changes: 20 additions & 0 deletions layers/VkLayer_khronos_validation.json.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,26 @@
}
]
}
},
{
"key": "gpuav_index_buffers",
"label": "Index buffers",
"type": "BOOL",
"default": true,
"description": "Validate that indexed draws do not fetch indices outside of the bounds of the index buffer. Also validates that those indices are not out of the bounds of the fetched vertex buffers.",
"platforms": [
"WINDOWS",
"LINUX"
],
"dependence": {
"mode": "ALL",
"settings": [
{
"key": "gpuav_buffers_validation",
"value": true
}
]
}
}
]
},
Expand Down
43 changes: 33 additions & 10 deletions layers/gpu/cmd_validation/gpuav_cmd_validation_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "gpu/shaders/gpu_shaders_constants.h"

#include "state_tracker/descriptor_sets.h"
#include "state_tracker/render_pass_state.h"
#include "state_tracker/shader_object_state.h"

namespace gpuav {
Expand All @@ -38,8 +39,7 @@ void BindErrorLoggingDescriptorSet(Validator &gpuav, CommandBuffer &cb_state, Vk
dynamic_offsets.data());
}

void RestorablePipelineState::Create(vvl::CommandBuffer &cb_state, VkPipelineBindPoint bind_point) {
cmd_buffer_ = cb_state.VkHandle();
void RestorablePipelineState::Create(CommandBuffer &cb_state, VkPipelineBindPoint bind_point) {
pipeline_bind_point_ = bind_point;
const auto lv_bind_point = ConvertToLvlBindPoint(bind_point);

Expand Down Expand Up @@ -78,11 +78,29 @@ void RestorablePipelineState::Create(vvl::CommandBuffer &cb_state, VkPipelineBin
if (last_bound.push_descriptor_set) {
push_descriptor_set_writes_ = last_bound.push_descriptor_set->GetWrites();
}

// Do not handle cb_state.activeRenderPass->use_dynamic_rendering_inherited for now
if (bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS && cb_state.activeRenderPass->use_dynamic_rendering) {
rendering_info_ = &cb_state.activeRenderPass->dynamic_rendering_begin_rendering_info;
DispatchCmdEndRendering(cb_state.VkHandle());

VkRenderingInfo rendering_info = vku::InitStructHelper();
rendering_info.renderArea = {{0, 0}, {1, 1}};
rendering_info.layerCount = 1;
rendering_info.viewMask = 0;
rendering_info.colorAttachmentCount = 0;
DispatchCmdBeginRendering(cb_state.VkHandle(), &rendering_info);
}
}

void RestorablePipelineState::Restore() const {
if (rendering_info_) {
DispatchCmdEndRendering(cb_state_.VkHandle());
DispatchCmdBeginRendering(cb_state_.VkHandle(), rendering_info_->ptr());
}

if (pipeline_ != VK_NULL_HANDLE) {
DispatchCmdBindPipeline(cmd_buffer_, pipeline_bind_point_, pipeline_);
DispatchCmdBindPipeline(cb_state_.VkHandle(), pipeline_bind_point_, pipeline_);
}
if (!shader_objects_.empty()) {
std::vector<VkShaderStageFlagBits> stages;
Expand All @@ -91,29 +109,34 @@ void RestorablePipelineState::Restore() const {
stages.emplace_back(shader_obj->create_info.stage);
shaders.emplace_back(shader_obj->VkHandle());
}
DispatchCmdBindShadersEXT(cmd_buffer_, static_cast<uint32_t>(shader_objects_.size()), stages.data(), shaders.data());
DispatchCmdBindShadersEXT(cb_state_.VkHandle(), static_cast<uint32_t>(shader_objects_.size()), stages.data(),
shaders.data());
}

for (std::size_t i = 0; i < descriptor_sets_.size(); i++) {
VkDescriptorSet descriptor_set = descriptor_sets_[i].first;
if (descriptor_set != VK_NULL_HANDLE) {
DispatchCmdBindDescriptorSets(cmd_buffer_, pipeline_bind_point_, desc_set_pipeline_layout_, descriptor_sets_[i].second,
1, &descriptor_set, static_cast<uint32_t>(dynamic_offsets_[i].size()),
dynamic_offsets_[i].data());
DispatchCmdBindDescriptorSets(cb_state_.VkHandle(), pipeline_bind_point_, desc_set_pipeline_layout_,
descriptor_sets_[i].second, 1, &descriptor_set,
static_cast<uint32_t>(dynamic_offsets_[i].size()), dynamic_offsets_[i].data());
}
}

if (!push_descriptor_set_writes_.empty()) {
DispatchCmdPushDescriptorSetKHR(cmd_buffer_, pipeline_bind_point_, desc_set_pipeline_layout_, push_descriptor_set_index_,
static_cast<uint32_t>(push_descriptor_set_writes_.size()),
DispatchCmdPushDescriptorSetKHR(cb_state_.VkHandle(), pipeline_bind_point_, desc_set_pipeline_layout_,
push_descriptor_set_index_, static_cast<uint32_t>(push_descriptor_set_writes_.size()),
reinterpret_cast<const VkWriteDescriptorSet *>(push_descriptor_set_writes_.data()));
}

for (const auto &push_constant_range : push_constants_data_) {
DispatchCmdPushConstants(cmd_buffer_, push_constant_range.layout, push_constant_range.stage_flags,
DispatchCmdPushConstants(cb_state_.VkHandle(), push_constant_range.layout, push_constant_range.stage_flags,
push_constant_range.offset, static_cast<uint32_t>(push_constant_range.values.size()),
push_constant_range.values.data());
}

if (pipeline_bind_point_ == VK_PIPELINE_BIND_POINT_GRAPHICS) {
cb_state_.RestoreDynamicStates(gpuav_);
}
}

} // namespace gpuav
11 changes: 8 additions & 3 deletions layers/gpu/cmd_validation/gpuav_cmd_validation_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,19 @@ class Validator;

class RestorablePipelineState {
public:
RestorablePipelineState(vvl::CommandBuffer& cb_state, VkPipelineBindPoint bind_point) { Create(cb_state, bind_point); }
RestorablePipelineState(const Validator& gpuav, CommandBuffer& cb_state, VkPipelineBindPoint bind_point)
: gpuav_(gpuav), cb_state_(cb_state) {
Create(cb_state, bind_point);
}
~RestorablePipelineState() { Restore(); }

private:
void Create(vvl::CommandBuffer& cb_state, VkPipelineBindPoint bind_point);
void Create(CommandBuffer& cb_state, VkPipelineBindPoint bind_point);
void Restore() const;

VkCommandBuffer cmd_buffer_;
const Validator& gpuav_;
CommandBuffer& cb_state_;
const vku::safe_VkRenderingInfo* rendering_info_ = nullptr;
VkPipelineBindPoint pipeline_bind_point_ = VK_PIPELINE_BIND_POINT_MAX_ENUM;
VkPipeline pipeline_ = VK_NULL_HANDLE;
VkPipelineLayout desc_set_pipeline_layout_ = VK_NULL_HANDLE;
Expand Down
2 changes: 1 addition & 1 deletion layers/gpu/cmd_validation/gpuav_copy_buffer_to_image.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ void InsertCopyBufferToImageValidation(Validator &gpuav, const Location &loc, Co
DispatchUpdateDescriptorSets(gpuav.device, static_cast<uint32_t>(desc_writes.size()), desc_writes.data(), 0, nullptr);
}
// Save current graphics pipeline state
RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_COMPUTE);
RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_COMPUTE);

// Insert diagnostic dispatch
DispatchCmdBindPipeline(cb_state.VkHandle(), VK_PIPELINE_BIND_POINT_COMPUTE, shared_copy_validation_resources.pipeline);
Expand Down
2 changes: 1 addition & 1 deletion layers/gpu/cmd_validation/gpuav_dispatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ void InsertIndirectDispatchValidation(Validator &gpuav, const Location &loc, Com
DispatchUpdateDescriptorSets(gpuav.device, 1, &desc_write, 0, nullptr);

// Save current graphics pipeline state
RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_COMPUTE);
RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_COMPUTE);

// Insert diagnostic dispatch
if (use_shader_objects) {
Expand Down
63 changes: 54 additions & 9 deletions layers/gpu/cmd_validation/gpuav_draw.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,31 @@ static VkPipeline GetDrawValidationPipeline(Validator &gpuav,
pipeline_stage_ci.pName = "main";

VkGraphicsPipelineCreateInfo pipeline_ci = vku::InitStructHelper();
// As of writing, adding a VkPipelineRenderingCreateInfoKHR is not strictly needed,
// if dynamic rendering is used a default empty VkPipelineRenderingCreateInfoKHR
// is assumed. Might have to change in the future.
// VkPipelineRenderingCreateInfoKHR pipeline_rendering_ci = vku::InitStructHelper();
// if (using_dynamic_rendering) pipeline_ci.pNext = &pipeline_rendering_ci;

VkPipelineVertexInputStateCreateInfo vertex_input_state = vku::InitStructHelper();
VkPipelineInputAssemblyStateCreateInfo input_assembly_state = vku::InitStructHelper();
input_assembly_state.topology = VK_PRIMITIVE_TOPOLOGY_POINT_LIST;
VkPipelineRasterizationStateCreateInfo rasterization_state = vku::InitStructHelper();
rasterization_state.lineWidth = 1.0;
rasterization_state.rasterizerDiscardEnable = VK_TRUE;
VkPipelineColorBlendStateCreateInfo color_blend_state = vku::InitStructHelper();

pipeline_ci.pVertexInputState = &vertex_input_state;
pipeline_ci.pInputAssemblyState = &input_assembly_state;
pipeline_ci.pTessellationState =
nullptr; // If not null, need to update gpuav::CommandBuffer::RestoreDynamicStates with corresponding dynamic states
pipeline_ci.pViewportState =
nullptr; // If not null, need to update gpuav::CommandBuffer::RestoreDynamicStates with corresponding dynamic states
pipeline_ci.pRasterizationState = &rasterization_state;
pipeline_ci.pMultisampleState =
nullptr; // If not null, need to update gpuav::CommandBuffer::RestoreDynamicStates with corresponding dynamic states
pipeline_ci.pDepthStencilState =
nullptr; // If not null, need to update gpuav::CommandBuffer::RestoreDynamicStates with corresponding dynamic states
pipeline_ci.pColorBlendState = &color_blend_state;
pipeline_ci.renderPass = render_pass;
pipeline_ci.layout = pipeline_layout;
Expand Down Expand Up @@ -267,7 +282,12 @@ void FirstInstance(Validator &gpuav, CommandBuffer &cb_state, const Location &lo

if (gpuav.enabled_features.drawIndirectFirstInstance) return;

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);
if (cb_state.activeRenderPass->use_dynamic_rendering_inherited) {
// Unhandled for now
return;
}

RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
if (!shared_draw_validation_resources.valid) return;
Expand Down Expand Up @@ -411,7 +431,12 @@ void CountBuffer(Validator &gpuav, CommandBuffer &cb_state, const Location &loc,
return;
}

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);
if (cb_state.activeRenderPass->use_dynamic_rendering_inherited) {
// Unhandled for now
return;
}

RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
if (!shared_draw_validation_resources.valid) return;
Expand Down Expand Up @@ -552,7 +577,12 @@ void DrawMeshIndirect(Validator &gpuav, CommandBuffer &cb_state, const Location
return;
}

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);
if (cb_state.activeRenderPass->use_dynamic_rendering_inherited) {
// Unhandled for now
return;
}

RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

const auto lv_bind_point = ConvertToLvlBindPoint(VK_PIPELINE_BIND_POINT_GRAPHICS);
auto const &last_bound = cb_state.lastBound[lv_bind_point];
Expand Down Expand Up @@ -808,7 +838,7 @@ static SmallestVertexBufferBinding SmallestVertexAttributesCount(const vvl::Comm

void DrawIndexed(Validator &gpuav, CommandBuffer &cb_state, const Location &loc, uint32_t index_count, uint32_t first_index,
uint32_t vertex_offset, const char *vuid_oob_vertex) {
if (!gpuav.gpuav_settings.validate_indirect_draws_buffers) {
if (!gpuav.gpuav_settings.validate_index_buffers) {
return;
}

Expand All @@ -820,13 +850,18 @@ void DrawIndexed(Validator &gpuav, CommandBuffer &cb_state, const Location &loc,
return;
}

if (cb_state.activeRenderPass->use_dynamic_rendering_inherited) {
// Unhandled for now
return;
}

const SmallestVertexBufferBinding smallest_vertex_buffer_binding = SmallestVertexAttributesCount(cb_state);
if (smallest_vertex_buffer_binding.min_vertex_attributes_count == std::numeric_limits<VkDeviceSize>::max()) {
// cannot overrun index buffer, skip validation
return;
}

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);
RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
if (!shared_draw_validation_resources.valid) return;
Expand Down Expand Up @@ -983,7 +1018,7 @@ struct DrawIndexedIndirectIndexBufferShader {
void DrawIndexedIndirectIndexBuffer(Validator &gpuav, CommandBuffer &cb_state, const Location &loc, VkBuffer draw_buffer,
VkDeviceSize draw_buffer_offset, uint32_t draw_cmds_byte_stride, uint32_t draw_count,
VkBuffer count_buffer, VkDeviceSize count_buffer_offset, const char *vuid_oob_index) {
if (!gpuav.gpuav_settings.validate_indirect_draws_buffers) {
if (!gpuav.gpuav_settings.validate_index_buffers) {
return;
}

Expand All @@ -995,7 +1030,12 @@ void DrawIndexedIndirectIndexBuffer(Validator &gpuav, CommandBuffer &cb_state, c
return;
}

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);
if (cb_state.activeRenderPass->use_dynamic_rendering_inherited) {
// Unhandled for now
return;
}

RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
if (!shared_draw_validation_resources.valid) return;
Expand Down Expand Up @@ -1150,7 +1190,7 @@ struct DrawIndexedIndirectVertexBufferShader {
void DrawIndexedIndirectVertexBuffer(Validator &gpuav, CommandBuffer &cb_state, const Location &loc, VkBuffer draw_buffer,
VkDeviceSize draw_buffer_offset, uint32_t draw_cmds_byte_stride, uint32_t draw_count,
VkBuffer count_buffer, VkDeviceSize count_buffer_offset, const char *vuid_oob_vertex) {
if (!gpuav.gpuav_settings.validate_indirect_draws_buffers) {
if (!gpuav.gpuav_settings.validate_index_buffers) {
return;
}

Expand All @@ -1162,7 +1202,12 @@ void DrawIndexedIndirectVertexBuffer(Validator &gpuav, CommandBuffer &cb_state,
return;
}

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);
if (cb_state.activeRenderPass->use_dynamic_rendering_inherited) {
// Unhandled for now
return;
}

RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
if (!shared_draw_validation_resources.valid) return;
Expand Down
2 changes: 1 addition & 1 deletion layers/gpu/cmd_validation/gpuav_trace_rays.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ void InsertIndirectTraceRaysValidation(Validator &gpuav, const Location &loc, Co
}

// Save current ray tracing pipeline state
RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR);
RestorablePipelineState restorable_state(gpuav, cb_state, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR);

// Push info needed for validation:
// - the device address indirect data is read from
Expand Down
4 changes: 3 additions & 1 deletion layers/gpu/core/gpu_settings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ struct GpuAVSettings {
bool validate_indirect_dispatches_buffers = true;
bool validate_indirect_trace_rays_buffers = true;
bool validate_buffer_copies = true;
bool validate_index_buffers = true;

bool vma_linear_output = true;

Expand Down Expand Up @@ -62,13 +63,14 @@ struct GpuAVSettings {
}
bool IsBufferValidationEnabled() const {
return validate_indirect_draws_buffers || validate_indirect_dispatches_buffers || validate_indirect_trace_rays_buffers ||
validate_buffer_copies;
validate_buffer_copies || validate_index_buffers;
}
void SetBufferValidationEnabled(bool enabled) {
validate_indirect_draws_buffers = enabled;
validate_indirect_dispatches_buffers = enabled;
validate_indirect_trace_rays_buffers = enabled;
validate_buffer_copies = enabled;
validate_index_buffers = enabled;
}
};

Expand Down
Loading

0 comments on commit e566158

Please sign in to comment.