Skip to content

Commit

Permalink
gpu: Add NonBindlessOOBBuffer Pass
Browse files Browse the repository at this point in the history
  • Loading branch information
spencer-lunarg committed Aug 22, 2024
1 parent 7da6943 commit dac5088
Show file tree
Hide file tree
Showing 29 changed files with 1,073 additions and 202 deletions.
4 changes: 4 additions & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ vvl_sources = [
"layers/gpu/shaders/gpu_shaders_constants.h",
"layers/gpu/spirv/bindless_descriptor_pass.cpp",
"layers/gpu/spirv/bindless_descriptor_pass.h",
"layers/gpu/spirv/non_bindless_oob_buffer_pass.cpp",
"layers/gpu/spirv/non_bindless_oob_buffer_pass.h",
"layers/gpu/spirv/buffer_device_address_pass.cpp",
"layers/gpu/spirv/buffer_device_address_pass.h",
"layers/gpu/spirv/function_basic_block.cpp",
Expand Down Expand Up @@ -352,6 +354,8 @@ vvl_sources = [
"layers/vulkan/generated/gpu_av_shader_hash.h",
"layers/vulkan/generated/instrumentation_bindless_descriptor_comp.cpp",
"layers/vulkan/generated/instrumentation_bindless_descriptor_comp.h",
"layers/vulkan/generated/instrumentation_non_bindless_oob_buffer_comp.cpp",
"layers/vulkan/generated/instrumentation_non_bindless_oob_buffer_comp.h",
"layers/vulkan/generated/instrumentation_buffer_device_address_comp.cpp",
"layers/vulkan/generated/instrumentation_buffer_device_address_comp.h",
"layers/vulkan/generated/instrumentation_ray_query_comp.cpp",
Expand Down
2 changes: 1 addition & 1 deletion layers/core_checks/cc_descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ bool CoreChecks::ValidateWriteUpdate(const DescriptorSet &dst_set, const VkWrite
}

const auto *used_handle = dst_set.InUse();
if (used_handle && !(dest->IsBindless())) {
if (used_handle && !vvl::IsBindless(dest->binding_flags)) {
skip |= LogError("VUID-vkUpdateDescriptorSets-None-03047", objlist, dst_binding_loc,
"(%" PRIu32 ") was created with %s, but %s is in use by %s.", update.dstBinding,
string_VkDescriptorBindingFlags(dest->binding_flags).c_str(), FormatHandle(update.dstSet).c_str(),
Expand Down
43 changes: 40 additions & 3 deletions layers/gpu/instrumentation/gpu_shader_instrumentor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "gpu/shaders/gpu_error_codes.h"
#include "spirv-tools/optimizer.hpp"
#include "utils/vk_layer_utils.h"
#include "state_tracker/descriptor_sets.h"

#include <cassert>
#include <regex>
Expand Down Expand Up @@ -499,10 +500,24 @@ void GpuShaderInstrumentor::PreCallRecordShaderObjectInstrumentation(
unique_shader_id = unique_shader_module_id_++;
}

bool has_bindless_descriptors = false;
for (const auto [layout_i, set_layout] : vvl::enumerate(create_info.pSetLayouts, create_info.setLayoutCount)) {
if (auto descriptor_set = Get<vvl::DescriptorSetLayout>(*set_layout)) {
for (uint32_t i = 0; i < descriptor_set->GetBindingCount(); i++) {
const VkDescriptorBindingFlags flags = descriptor_set->GetDescriptorBindingFlagsFromIndex(i);
if (vvl::IsBindless(flags)) {
has_bindless_descriptors = true;
break;
}
}
if (has_bindless_descriptors) break;
}
}

if (!cached) {
pass = InstrumentShader(
vvl::make_span(static_cast<const uint32_t *>(create_info.pCode), create_info.codeSize / sizeof(uint32_t)),
unique_shader_id, create_info_loc, instrumented_spirv);
unique_shader_id, has_bindless_descriptors, create_info_loc, instrumented_spirv);
}

if (cached || pass) {
Expand Down Expand Up @@ -939,6 +954,24 @@ void GpuShaderInstrumentor::PreCallRecordPipelineCreationShaderInstrumentation(

if (!instrument_shader) return;

// TODO - measure and see if would be better to make a gpuav subclasses of pipeline layout and store this information once there
// (not sure how much pipeline layout re-usage there is)
bool has_bindless_descriptors = false;
if (pipeline_layout) {
for (const auto &descriptor_set : pipeline_layout->set_layouts) {
if (descriptor_set) {
for (uint32_t i = 0; i < descriptor_set->GetBindingCount(); i++) {
const VkDescriptorBindingFlags flags = descriptor_set->GetDescriptorBindingFlagsFromIndex(i);
if (vvl::IsBindless(flags)) {
has_bindless_descriptors = true;
break;
}
}
}
if (has_bindless_descriptors) break;
}
}

for (uint32_t i = 0; i < static_cast<uint32_t>(pipeline_state.stage_states.size()); ++i) {
const auto &stage_state = pipeline_state.stage_states[i];
auto module_state = std::const_pointer_cast<vvl::ShaderModule>(stage_state.module_state);
Expand Down Expand Up @@ -980,7 +1013,8 @@ void GpuShaderInstrumentor::PreCallRecordPipelineCreationShaderInstrumentation(
unique_shader_id = unique_shader_module_id_++;
}
if (!cached) {
pass = InstrumentShader(module_state->spirv->words_, unique_shader_id, loc, instrumented_spirv);
pass =
InstrumentShader(module_state->spirv->words_, unique_shader_id, has_bindless_descriptors, loc, instrumented_spirv);
}
if (cached || pass) {
shader_instrumentation_metadata.spirv_unique_id_map[i] = unique_shader_id;
Expand Down Expand Up @@ -1135,7 +1169,8 @@ static bool GpuValidateShader(const std::vector<uint32_t> &input, bool SetRelaxB

// Call the SPIR-V Optimizer to run the instrumentation pass on the shader.
bool GpuShaderInstrumentor::InstrumentShader(const vvl::span<const uint32_t> &input_spirv, uint32_t unique_shader_id,
const Location &loc, std::vector<uint32_t> &out_instrumented_spirv) {
bool has_bindless_descriptors, const Location &loc,
std::vector<uint32_t> &out_instrumented_spirv) {
if (input_spirv[0] != spv::MagicNumber) return false;

if (gpuav_settings.debug_dump_instrumented_shaders) {
Expand All @@ -1153,6 +1188,7 @@ bool GpuShaderInstrumentor::InstrumentShader(const vvl::span<const uint32_t> &in
module_settings.max_instrumented_count = gpuav_settings.debug_max_instrumented_count;
module_settings.support_int64 = enabled_features.shaderInt64;
module_settings.support_memory_model_device_scope = enabled_features.vulkanMemoryModelDeviceScope;
module_settings.has_bindless_descriptors = has_bindless_descriptors;

gpu::spirv::Module module(input_spirv, debug_report, module_settings);

Expand All @@ -1167,6 +1203,7 @@ bool GpuShaderInstrumentor::InstrumentShader(const vvl::span<const uint32_t> &in
// If descriptor indexing is enabled, enable length checks and updated descriptor checks
if (shader_instrumentation.bindless_descriptor) {
modified |= module.RunPassBindlessDescriptor();
modified |= module.RunPassNonBindlessOOBBuffer();
}

if (shader_instrumentation.buffer_device_address) {
Expand Down
4 changes: 2 additions & 2 deletions layers/gpu/instrumentation/gpu_shader_instrumentor.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ class GpuShaderInstrumentor : public ValidationStateTracker {

// GPU-AV and DebugPrint are using the same way to do the actual shader instrumentation logic
// Returns if shader was instrumented successfully or not
bool InstrumentShader(const vvl::span<const uint32_t> &input_spirv, uint32_t unique_shader_id, const Location &loc,
std::vector<uint32_t> &out_instrumented_spirv);
bool InstrumentShader(const vvl::span<const uint32_t> &input_spirv, uint32_t unique_shader_id, bool has_bindless_descriptors,
const Location &loc, std::vector<uint32_t> &out_instrumented_spirv);

VkDescriptorSetLayout GetDebugDescriptorSetLayout() { return debug_desc_layout_; }

Expand Down
56 changes: 56 additions & 0 deletions layers/gpu/instrumentation/gpuav_instrumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,58 @@ bool LogMessageInstBindlessDescriptor(Validator &gpuav, const uint32_t *error_re
return error_found;
}

bool LogMessageInstNonBindlessOOB(Validator &gpuav, const uint32_t *error_record, std::string &out_error_msg,
std::string &out_vuid_msg, const std::vector<DescSetState> &descriptor_sets, const Location &loc,
bool uses_shader_object, bool &out_oob_access) {
using namespace glsl;
bool error_found = true;
out_oob_access = true;
std::ostringstream strm;
const GpuVuid &vuid = GetGpuVuid(loc.function);

const uint32_t set_num = error_record[kInstNonBindlessOOBDescSetOffset];
const uint32_t binding_num = error_record[kInstNonBindlessOOBDescBindingOffset];
const uint32_t desc_index = error_record[kInstNonBindlessOOBDescIndexOffset];

strm << "(set = " << set_num << ", binding = " << binding_num << ", index " << desc_index << ") ";
switch (error_record[kHeaderErrorSubCodeOffset]) {
case kErrorSubCodeNonBindlessOOBBufferArrays: {
const uint32_t desc_array_size = error_record[kInstNonBindlessOOBParamOffset0];
strm << " access out of bounds. The descriptor buffer array is " << desc_array_size
<< " large, but as accessed at index [" << desc_index << "]";
out_vuid_msg = "UNASSIGNED-Descriptor Buffer index out of bounds";
} break;

case kErrorSubCodeNonBindlessOOBBufferBounds: {
const auto *binding_state = descriptor_sets[set_num].state->GetBinding(binding_num);
const vvl::Buffer *buffer_state =
static_cast<const vvl::BufferBinding *>(binding_state)->descriptors[desc_index].GetBufferState();
assert(buffer_state);
const uint32_t byte_offset = error_record[kInstNonBindlessOOBParamOffset0];
const uint32_t resource_size = error_record[kInstNonBindlessOOBParamOffset1];

strm << " access out of bounds. The descriptor buffer (" << gpuav.FormatHandle(buffer_state->Handle()) << ") size is "
<< buffer_state->create_info.size << " bytes, " << resource_size
<< " bytes were bound, and the highest out of bounds access was at [" << byte_offset << "] bytes";

if (binding_state->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER ||
binding_state->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC) {
out_vuid_msg = uses_shader_object ? vuid.uniform_access_oob_08612 : vuid.uniform_access_oob_06935;
} else {
out_vuid_msg = uses_shader_object ? vuid.storage_access_oob_08613 : vuid.storage_access_oob_06936;
}
} break;

default:
error_found = false;
out_oob_access = false;
assert(false); // other OOB checks are not implemented yet
}

out_error_msg = strm.str();
return error_found;
}

bool LogMessageInstBufferDeviceAddress(const uint32_t *error_record, std::string &out_error_msg, std::string &out_vuid_msg,
bool &out_oob_access) {
using namespace glsl;
Expand Down Expand Up @@ -439,6 +491,10 @@ bool LogInstrumentationError(Validator &gpuav, VkCommandBuffer cmd_buffer, const
error_found = LogMessageInstBindlessDescriptor(gpuav, error_record, error_msg, vuid_msg, descriptor_sets, loc,
uses_shader_object, oob_access);
break;
case glsl::kErrorGroupInstNonBindlessOOB:
error_found = LogMessageInstNonBindlessOOB(gpuav, error_record, error_msg, vuid_msg, descriptor_sets, loc,
uses_shader_object, oob_access);
break;
case glsl::kErrorGroupInstBufferDeviceAddress:
error_found = LogMessageInstBufferDeviceAddress(error_record, error_msg, vuid_msg, oob_access);
break;
Expand Down
3 changes: 3 additions & 0 deletions layers/gpu/instrumentation/gpuav_instrumentation.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ bool LogInstrumentationError(Validator& gpuav, VkCommandBuffer cmd_buffer, const
bool LogMessageInstBindlessDescriptor(Validator& gpuav, const uint32_t* error_record, std::string& out_error_msg,
std::string& out_vuid_msg, const std::vector<DescSetState>& descriptor_sets,
const Location& loc, bool uses_shader_object, bool& out_oob_access);
bool LogMessageInstNonBindlessOOB(Validator& gpuav, const uint32_t* error_record, std::string& out_error_msg,
std::string& out_vuid_msg, const std::vector<DescSetState>& descriptor_sets, const Location& loc,
bool uses_shader_object, bool& out_oob_access);
bool LogMessageInstBufferDeviceAddress(const uint32_t* error_record, std::string& out_error_msg, std::string& out_vuid_msg,
bool& out_oob_access);
bool LogMessageInstRayQuery(const uint32_t* error_record, std::string& out_error_msg, std::string& out_vuid_msg);
Expand Down
7 changes: 7 additions & 0 deletions layers/gpu/shaders/gpu_error_codes.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const int kErrorGroupGpuPreDraw = 4;
const int kErrorGroupGpuPreDispatch = 5;
const int kErrorGroupGpuPreTraceRays = 6;
const int kErrorGroupGpuCopyBufferToImage = 7;
const int kErrorGroupInstNonBindlessOOB = 8;

// Used for MultiEntry and there is no single stage set
const int kHeaderStageIdMultiEntryPoint = 0x7fffffff; // same as spv::ExecutionModelMax
Expand All @@ -46,6 +47,12 @@ const int kErrorSubCodeBindlessDescriptorOOB = 3;
const int kErrorSubCodeBindlessDescriptorDestroyed = 4;
const int kErrorSubCodeBindlessDescriptorNullPointer = 5;

// Non-Bindless OOB
//
// Buffers
const int kErrorSubCodeNonBindlessOOBBufferArrays = 1;
const int kErrorSubCodeNonBindlessOOBBufferBounds = 2;

// Buffer Device Address
//
const int kErrorSubCodeBufferDeviceAddressUnallocRef = 1;
Expand Down
9 changes: 9 additions & 0 deletions layers/gpu/shaders/gpu_error_header.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ const int kInstBindlessBuffOOBDescIndexOffset = kInstBindlessDescIndexOffset;
const int kInstBindlessBuffOOBBuffOffOffset = kInstBindlessCustomOffset_0;
const int kInstBindlessBuffOOBBuffSizeOffset = kInstBindlessCustomOffset_1;

// Non-Bindless OOB
// ---

const int kInstNonBindlessOOBDescSetOffset = kHeaderSize;
const int kInstNonBindlessOOBDescBindingOffset = kHeaderSize + 1;
const int kInstNonBindlessOOBDescIndexOffset = kHeaderSize + 2;
const int kInstNonBindlessOOBParamOffset0 = kHeaderSize + 3;
const int kInstNonBindlessOOBParamOffset1 = kHeaderSize + 4;

// Buffer device addresses
// ---
// A buffer address unalloc error will output the 64-bit pointer in
Expand Down
135 changes: 135 additions & 0 deletions layers/gpu/shaders/instrumentation/non_bindless_oob_buffer.comp
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// Copyright (c) 2024 The Khronos Group Inc.
// Copyright (c) 2024 Valve Corporation
// Copyright (c) 2024 LunarG, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// NOTE: This file doesn't contain any entrypoints and should be compiled with then new --no-link option for glslang

#version 450
#extension GL_GOOGLE_include_directive : enable
#extension GL_EXT_buffer_reference : require
#extension GL_EXT_buffer_reference_uvec2 : require
#if defined(GL_ARB_gpu_shader_int64)
#extension GL_ARB_gpu_shader_int64 : require
#else
#error No extension available for 64-bit integers.
#endif

#include "gpu_error_header.h"
#include "gpu_shaders_constants.h"
#include "common_descriptor_sets.h"

layout(buffer_reference, buffer_reference_align = 8, std430) buffer DescriptorLayoutData {
// BindingLayout[0]
uint num_bindings;
uint pad; // always zero, used to keep things aligned

// BindingLayout[1] - BindingLayout[N]
// struct glsl::BindingLayout {
// x: count
// y: state_start
// }
uvec2 data[];
};

layout(buffer_reference, buffer_reference_align = 8, std430) buffer DescriptorSetInData {
// struct glsl::DescriptorState {
// x: id
// y: extra data depending on the descriptor type
// }
uvec2 data[];
};

layout(buffer_reference, buffer_reference_align = 8, std430) buffer GlobalState {
// Maps to DescriptorHeap and used to detect if descriptor is still valid on CPU
uint data[];
};

struct DescriptorSetRecord {
DescriptorLayoutData layout_data;
DescriptorSetInData in_data;
uvec2 out_data; // unused BDA pointer
};

layout(set = kInstDefaultDescriptorSet, binding = kBindingInstBindlessDescriptor, std430) buffer BindlessStateBuffer {
GlobalState global_state;
DescriptorSetRecord desc_sets[kDebugInputBindlessMaxDescSets];
} bindless_state_buffer;

bool inst_non_bindless_oob_buffer(const uint inst_num, const uvec4 stage_info, const uint desc_array_size,
const uint desc_set, const uint binding, const uint desc_index, const uint byte_offset) {
uint error = 0u;
uint param_0 = 0u;
uint param_1 = 0u;
do {
// For non-array this should hopefully optimized out as "if (0 > 1)"
if (desc_index > desc_array_size) {
error = kErrorSubCodeNonBindlessOOBBufferArrays;
param_0 = desc_array_size;
break;
}

DescriptorLayoutData layout_data = bindless_state_buffer.desc_sets[desc_set].layout_data;
uvec2 binding_state = layout_data.data[binding];

DescriptorSetInData in_data = bindless_state_buffer.desc_sets[desc_set].in_data;

uint state_index = binding_state.y + desc_index;

// check that the offset is in bounds
uint resource_size = in_data.data[state_index].y;
if (byte_offset >= resource_size) {
error = kErrorSubCodeNonBindlessOOBBufferBounds;
param_0 = byte_offset;
param_1 = resource_size;
break;
}
} while (false);

if (0u != error) {

const uint cmd_id = inst_cmd_resource_index_buffer.index[0];
const uint cmd_errors_count = atomicAdd(inst_cmd_errors_count_buffer.errors_count[cmd_id], 1);
const bool max_cmd_errors_count_reached = cmd_errors_count >= kMaxErrorsPerCmd;

if (max_cmd_errors_count_reached) return false;

uint write_pos = atomicAdd(inst_errors_buffer.written_count, kErrorRecordSize);
const bool errors_buffer_not_filled = (write_pos + kErrorRecordSize) <= uint(inst_errors_buffer.data.length());

if (errors_buffer_not_filled) {
inst_errors_buffer.data[write_pos + kHeaderErrorRecordSizeOffset] = kErrorRecordSize;
inst_errors_buffer.data[write_pos + kHeaderShaderIdOffset] = kLinkShaderId;
inst_errors_buffer.data[write_pos + kHeaderInstructionIdOffset] = inst_num;
inst_errors_buffer.data[write_pos + kHeaderStageIdOffset] = stage_info.x;
inst_errors_buffer.data[write_pos + kHeaderStageInfoOffset_0] = stage_info.y;
inst_errors_buffer.data[write_pos + kHeaderStageInfoOffset_1] = stage_info.z;
inst_errors_buffer.data[write_pos + kHeaderStageInfoOffset_2] = stage_info.w;

inst_errors_buffer.data[write_pos + kHeaderErrorGroupOffset] = kErrorGroupInstNonBindlessOOB;
inst_errors_buffer.data[write_pos + kHeaderErrorSubCodeOffset] = error;

inst_errors_buffer.data[write_pos + kHeaderActionIdOffset] = inst_action_index_buffer.index[0];
inst_errors_buffer.data[write_pos + kHeaderCommandResourceIdOffset] = inst_cmd_resource_index_buffer.index[0];

inst_errors_buffer.data[write_pos + kInstBindlessDescSetOffset] = desc_set;
inst_errors_buffer.data[write_pos + kInstBindlessDescBindingOffset] = binding;
inst_errors_buffer.data[write_pos + kInstBindlessDescIndexOffset] = desc_index;
inst_errors_buffer.data[write_pos + kInstBindlessCustomOffset_0] = param_0;
inst_errors_buffer.data[write_pos + kInstBindlessCustomOffset_1] = param_1;
}
return false;
}
return true;
}
Loading

0 comments on commit dac5088

Please sign in to comment.