Skip to content

Commit

Permalink
scripts: Start to refactor ValidationObject
Browse files Browse the repository at this point in the history
Previously, there were top level ValidationObjects stored in
layer_data_map. These didn't do validation but contained more
ValidationObjects in the object_dispatch vector (and a few others).

This caused much confusion, since some members were only used
at the top level and others only at the lower level.  When uses
got mixed up, this could only be detected at runtime since the
members were always available even though they might not be set
up correctly.

Start to split this up by moving top level functionality to
a new DispatchObject class. This does handle wrapping and coordination
of validation by the child ValidationObjects. There are still many
members related to settings, extension status, logging  and dispatch
tables that are duplicated at both levels. Fixing this is a big
change that needs to be done separately.

Also note that DispatchObject and ValidationObject are still used
to represent both VkDevice and VkInstance. This will require much
more work to undo.
  • Loading branch information
jeremyg-lunarg committed Nov 20, 2024
1 parent 44b1bde commit 2db40c0
Show file tree
Hide file tree
Showing 13 changed files with 542 additions and 338 deletions.
18 changes: 6 additions & 12 deletions layers/chassis/layer_chassis_dispatch_manual.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,9 @@ VkResult DispatchCreateGraphicsPipelines(VkDevice device, VkPipelineCache pipeli
dynamic_rendering->stencilAttachmentFormat != VK_FORMAT_UNDEFINED);
}

auto &graphics_info = pCreateInfos[idx0];
auto state_info = dynamic_cast<ValidationStateTracker *>(layer_data);
vku::PNextCopyState pnext_copy_state = {
[state_info, &graphics_info](VkBaseOutStructure *safe_struct, const VkBaseOutStructure *in_struct) -> bool {
return vvl::Pipeline::PnextRenderingInfoCustomCopy(state_info, graphics_info, safe_struct, in_struct);
}};
local_pCreateInfos[idx0].initialize(&pCreateInfos[idx0], uses_color_attachment, uses_depthstencil_attachment,
&pnext_copy_state);
// TODO: this used to use vvl::Pipeline::PnextRenderingInfoCustomCopy() but it was effectively a no-op
// since the layer_data returned by GetLayerDataPtr() above was NEVER an instance of ValidationStateTracker
local_pCreateInfos[idx0].initialize(&pCreateInfos[idx0], uses_color_attachment, uses_depthstencil_attachment);

if (pCreateInfos[idx0].basePipelineHandle) {
local_pCreateInfos[idx0].basePipelineHandle = layer_data->Unwrap(pCreateInfos[idx0].basePipelineHandle);
Expand Down Expand Up @@ -236,7 +231,7 @@ VkResult DispatchCreateGraphicsPipelines(VkDevice device, VkPipelineCache pipeli
}

template <typename T>
static void UpdateCreateRenderPassState(ValidationObject *layer_data, const T *pCreateInfo, VkRenderPass renderPass) {
static void UpdateCreateRenderPassState(DispatchObject *layer_data, const T *pCreateInfo, VkRenderPass renderPass) {
auto &renderpass_state = layer_data->renderpasses_states[renderPass];

for (uint32_t subpass = 0; subpass < pCreateInfo->subpassCount; ++subpass) {
Expand All @@ -255,8 +250,7 @@ static void UpdateCreateRenderPassState(ValidationObject *layer_data, const T *p
}

template <>
void UpdateCreateRenderPassState(ValidationObject *layer_data, const VkRenderPassCreateInfo2 *pCreateInfo,
VkRenderPass renderPass) {
void UpdateCreateRenderPassState(DispatchObject *layer_data, const VkRenderPassCreateInfo2 *pCreateInfo, VkRenderPass renderPass) {
auto &renderpass_state = layer_data->renderpasses_states[renderPass];

for (uint32_t subpassIndex = 0; subpassIndex < pCreateInfo->subpassCount; ++subpassIndex) {
Expand Down Expand Up @@ -648,7 +642,7 @@ void DispatchDestroyDescriptorUpdateTemplateKHR(VkDevice device, VkDescriptorUpd
layer_data->device_dispatch_table.DestroyDescriptorUpdateTemplateKHR(device, descriptorUpdateTemplate, pAllocator);
}

void *BuildUnwrappedUpdateTemplateBuffer(ValidationObject *layer_data, uint64_t descriptorUpdateTemplate, const void *pData) {
void *BuildUnwrappedUpdateTemplateBuffer(DispatchObject *layer_data, uint64_t descriptorUpdateTemplate, const void *pData) {
auto const template_map_entry = layer_data->desc_template_createinfo_map.find(descriptorUpdateTemplate);
auto const &create_info = template_map_entry->second->create_info;
size_t allocation_size = 0;
Expand Down
9 changes: 3 additions & 6 deletions layers/core_checks/cc_wsi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1449,8 +1449,7 @@ bool CoreChecks::PreCallValidateGetDeviceGroupSurfacePresentModes2EXT(VkDevice d
bool skip = false;

if (physical_device_count == 1) {
ValidationObject *device_object = GetLayerDataPtr(GetDispatchKey(device), layer_data_map);
skip |= ValidatePhysicalDeviceSurfaceSupport(device_object->physical_device, pSurfaceInfo->surface,
skip |= ValidatePhysicalDeviceSurfaceSupport(physical_device, pSurfaceInfo->surface,
"VUID-vkGetDeviceGroupSurfacePresentModes2EXT-pSurfaceInfo-06213",
error_obj.location);
} else {
Expand Down Expand Up @@ -1486,10 +1485,8 @@ bool CoreChecks::PreCallValidateGetDeviceGroupSurfacePresentModesKHR(VkDevice de
bool skip = false;

if (physical_device_count == 1) {
ValidationObject *device_object = GetLayerDataPtr(GetDispatchKey(device), layer_data_map);
skip |=
ValidatePhysicalDeviceSurfaceSupport(device_object->physical_device, surface,
"VUID-vkGetDeviceGroupSurfacePresentModesKHR-surface-06212", error_obj.location);
skip |= ValidatePhysicalDeviceSurfaceSupport(
physical_device, surface, "VUID-vkGetDeviceGroupSurfacePresentModesKHR-surface-06212", error_obj.location);
} else {
for (uint32_t i = 0; i < physical_device_count; ++i) {
skip |= ValidatePhysicalDeviceSurfaceSupport(device_group_create_info.pPhysicalDevices[i], surface,
Expand Down
2 changes: 1 addition & 1 deletion layers/gpu/core/gpuav_setup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ void Validator::InternalVmaError(LogObjectList objlist, const Location &loc, con
// Once we encounter an internal issue disconnect everything.
// This prevents need to check "if (aborted)" (which is awful when we easily forget to check somewhere and the user gets spammed
// with errors making it hard to see the first error with the real source of the problem).
ReleaseDeviceDispatchObject(LayerObjectTypeGpuAssisted);
dispatch_->ReleaseDeviceValidationObject(LayerObjectTypeGpuAssisted);
}

VkDeviceAddress Validator::GetBufferDeviceAddressHelper(VkBuffer buffer) const {
Expand Down
2 changes: 1 addition & 1 deletion layers/gpu/instrumentation/gpuav_shader_instrumentor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,7 @@ void GpuShaderInstrumentor::InternalError(LogObjectList objlist, const Location
// Once we encounter an internal issue disconnect everything.
// This prevents need to check "if (aborted)" (which is awful when we easily forget to check somewhere and the user gets spammed
// with errors making it hard to see the first error with the real source of the problem).
ReleaseDeviceDispatchObject(LayerObjectTypeGpuAssisted);
dispatch_->ReleaseDeviceValidationObject(LayerObjectTypeGpuAssisted);
}

void GpuShaderInstrumentor::InternalWarning(LogObjectList objlist, const Location &loc, const char *const specific_message) const {
Expand Down
22 changes: 1 addition & 21 deletions layers/state_tracker/state_tracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ void ValidationStateTracker::PostCallRecordCreateDevice(VkPhysicalDevice gpu, co
if (VK_SUCCESS != record_obj.result) return;

// The current object represents the VkInstance, look up / create the object for the device.
ValidationObject *device_object = GetLayerDataPtr(GetDispatchKey(*pDevice), layer_data_map);
DispatchObject *device_object = GetLayerDataPtr(GetDispatchKey(*pDevice), layer_data_map);
ValidationObject *validation_data = device_object->GetValidationObject(this->container_type);
ValidationStateTracker *device_state = static_cast<ValidationStateTracker *>(validation_data);

Expand Down Expand Up @@ -3952,33 +3952,13 @@ std::shared_ptr<vvl::PhysicalDevice> ValidationStateTracker::CreatePhysicalDevic
return std::make_shared<vvl::PhysicalDevice>(handle);
}

// This is here as some applications will call exit() which results in all our static allocations (like std::map) having their
// destructor called and destroyed from under us. It is not possible to detect as sometimes (when using things like robin hood) the
// size()/empty() will give false positive that memory is there there. We add this global hook that will go through and remove all
// the function calls such that things can safely run in the case the applicaiton still wants to make Vulkan calls in their atexit()
// handler
void ApplicationAtExit() {
// On a "normal" application, this function is called after vkDestroyInstance and layer_data_map is empty
//
// If there are multiple devices we still want to delete them all as exit() is a global scope call
for (auto object : layer_data_map) {
// Should only be a instance and device object, but being safe.
// Need to clean up both the instance and device function hooks
if (object.second->container_type == LayerObjectTypeInstance || object.second->container_type == LayerObjectTypeDevice) {
object.second->ReleaseAllDispatchObjects();
}
}
}

void ValidationStateTracker::PostCallRecordCreateInstance(const VkInstanceCreateInfo *pCreateInfo,
const VkAllocationCallbacks *pAllocator, VkInstance *pInstance,
const RecordObject &record_obj) {
if (record_obj.result != VK_SUCCESS) {
return;
}

atexit(ApplicationAtExit);

instance_state = this;
uint32_t count = 0;
// this can fail if the allocator fails
Expand Down
5 changes: 5 additions & 0 deletions layers/thread_tracker/thread_safety_validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ class ThreadSafety : public ValidationObject {
vvl::concurrent_unordered_map<VkDescriptorSetLayout, bool, 4> dsl_read_only_map;
vvl::concurrent_unordered_map<VkDescriptorSet, bool, 6> ds_read_only_map;
bool DsReadOnly(VkDescriptorSet) const;
// Map of wrapped swapchain handles to arrays of wrapped swapchain image IDs
// Each swapchain has an immutable list of wrapped swapchain image IDs -- always return these IDs if they exist
vvl::unordered_map<VkSwapchainKHR, std::vector<VkImage>> swapchain_wrapped_image_handle_map;
// Map of wrapped descriptor pools to set of wrapped descriptor sets allocated from each pool
vvl::unordered_map<VkDescriptorPool, vvl::unordered_set<VkDescriptorSet>> pool_descriptor_sets_map;

counter<VkCommandBuffer> c_VkCommandBuffer;
counter<VkDevice> c_VkDevice;
Expand Down
Loading

0 comments on commit 2db40c0

Please sign in to comment.