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 11, 2024
1 parent 87deb4e commit ec752ae
Show file tree
Hide file tree
Showing 12 changed files with 257 additions and 33 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();
}
}

} // 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,17 @@ class Validator;

class RestorablePipelineState {
public:
RestorablePipelineState(vvl::CommandBuffer& cb_state, VkPipelineBindPoint bind_point) { Create(cb_state, bind_point); }
RestorablePipelineState(CommandBuffer& cb_state, VkPipelineBindPoint bind_point) : 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_;
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 All @@ -44,6 +47,8 @@ class RestorablePipelineState {
std::vector<vku::safe_VkWriteDescriptorSet> push_descriptor_set_writes_;
std::vector<vvl::CommandBuffer::PushConstantData> push_constants_data_;
std::vector<vvl::ShaderObject*> shader_objects_;
// Need to reset dynamic states (x_x)
// Copy vvl::CommandBuffer::dynamic_state_value
};

void BindErrorLoggingDescriptorSet(Validator& gpuav, CommandBuffer& cb_state, VkPipelineBindPoint bind_point,
Expand Down
51 changes: 48 additions & 3 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,6 +282,11 @@ void FirstInstance(Validator &gpuav, CommandBuffer &cb_state, const Location &lo

if (gpuav.enabled_features.drawIndirectFirstInstance) return;

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

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

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

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

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

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

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

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

const auto lv_bind_point = ConvertToLvlBindPoint(VK_PIPELINE_BIND_POINT_GRAPHICS);
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,6 +850,11 @@ 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
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,6 +1030,11 @@ void DrawIndexedIndirectIndexBuffer(Validator &gpuav, CommandBuffer &cb_state, c
return;
}

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

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
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,6 +1202,11 @@ void DrawIndexedIndirectVertexBuffer(Validator &gpuav, CommandBuffer &cb_state,
return;
}

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

RestorablePipelineState restorable_state(cb_state, VK_PIPELINE_BIND_POINT_GRAPHICS);

auto &shared_draw_validation_resources = gpuav.shared_resources_manager.Get<SharedDrawValidationResources>(gpuav, loc);
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 ec752ae

Please sign in to comment.