From 8cca8e4580e374b4deb1342f38aa00c32eb74660 Mon Sep 17 00:00:00 2001 From: spencer-lunarg Date: Sun, 9 Jun 2024 20:37:33 -0500 Subject: [PATCH] layers: Remove safe_VkDeviceCreateInfo --- layers/core_checks/cc_device.cpp | 4 +- layers/core_checks/core_validation.h | 2 +- layers/gpu/core/gpuav.h | 12 +- layers/gpu/core/gpuav_setup.cpp | 123 ++++++++++++++++-- layers/gpu/debug_printf/debug_printf.cpp | 31 ++++- layers/gpu/debug_printf/debug_printf.h | 11 +- .../gpu_shader_instrumentor.cpp | 121 +---------------- .../instrumentation/gpu_shader_instrumentor.h | 7 +- layers/state_tracker/state_tracker.cpp | 4 +- layers/state_tracker/state_tracker.h | 2 +- layers/sync/sync_validation.cpp | 4 +- layers/sync/sync_validation.h | 2 +- layers/vulkan/generated/chassis.cpp | 29 ++++- layers/vulkan/generated/chassis.h | 2 +- scripts/generators/feature_requirements.py | 88 ++++++------- scripts/generators/layer_chassis_generator.py | 31 ++++- 16 files changed, 264 insertions(+), 209 deletions(-) diff --git a/layers/core_checks/cc_device.cpp b/layers/core_checks/cc_device.cpp index 3982ca74e21..92927f1be6b 100644 --- a/layers/core_checks/cc_device.cpp +++ b/layers/core_checks/cc_device.cpp @@ -437,9 +437,9 @@ bool CoreChecks::PreCallValidateCreateDevice(VkPhysicalDevice gpu, const VkDevic return skip; } -void CoreChecks::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { +void CoreChecks::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { // The state tracker sets up the device state (also if extension and/or features are enabled) - StateTracker::CreateDevice(pCreateInfo, loc); + StateTracker::PostCreateDevice(pCreateInfo, loc); // Add the callback hooks for the functions that are either broadly or deeply used and that the ValidationStateTracker refactor // would be messier without. diff --git a/layers/core_checks/core_validation.h b/layers/core_checks/core_validation.h index 5f461799be2..1fd28a522a5 100644 --- a/layers/core_checks/core_validation.h +++ b/layers/core_checks/core_validation.h @@ -1258,7 +1258,7 @@ class CoreChecks : public ValidationStateTracker { bool PreCallValidateCreateDevice(VkPhysicalDevice gpu, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const ErrorObject& error_obj) const override; - void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override; + void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override; bool PreCallValidateCmdUpdateBuffer(VkCommandBuffer commandBuffer, VkBuffer dstBuffer, VkDeviceSize dstOffset, VkDeviceSize dataSize, const void* pData, const ErrorObject& error_obj) const override; bool PreCallValidateGetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue* pQueue, diff --git a/layers/gpu/core/gpuav.h b/layers/gpu/core/gpuav.h index 8779d7cafe5..498aebef68a 100644 --- a/layers/gpu/core/gpuav.h +++ b/layers/gpu/core/gpuav.h @@ -60,9 +60,6 @@ class Validator : public gpu::GpuShaderInstrumentor { public: Validator() { container_type = LayerObjectTypeGpuAssisted; - desired_features_.vertexPipelineStoresAndAtomics = true; - desired_features_.fragmentStoresAndAtomics = true; - desired_features_.shaderInt64 = true; force_buffer_device_address_ = true; } @@ -110,7 +107,10 @@ class Validator : public gpu::GpuShaderInstrumentor { std::shared_ptr CreateQueue(VkQueue q, uint32_t index, VkDeviceQueueCreateFlags flags, const VkQueueFamilyProperties& queueFamilyProperties) final; - void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) final; + void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo, + const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj, + VkDeviceCreateInfo* modified_create_info) final; + void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) final; // gpuav_record.cpp // -------------- @@ -363,6 +363,10 @@ class Validator : public gpu::GpuShaderInstrumentor { private: std::string instrumented_shader_cache_path_{}; + + // Need to live for the scope of vkCreateDevice + VkPhysicalDeviceBufferDeviceAddressFeatures new_bda_features; + VkPhysicalDeviceTimelineSemaphoreFeatures new_ts_features; }; } // namespace gpuav diff --git a/layers/gpu/core/gpuav_setup.cpp b/layers/gpu/core/gpuav_setup.cpp index 81527c165ac..ffe368d29de 100644 --- a/layers/gpu/core/gpuav_setup.cpp +++ b/layers/gpu/core/gpuav_setup.cpp @@ -34,6 +34,7 @@ #include "generated/layer_chassis_dispatch.h" #include "containers/custom_containers.h" #include "gpu_shaders/gpu_error_header.h" +#include "generated/chassis.h" namespace gpuav { @@ -74,8 +75,107 @@ std::shared_ptr Validator::CreateQueue(VkQueue q, uint32_t index, Vk return std::static_pointer_cast(std::make_shared(*this, q, index, flags, queueFamilyProperties)); } +static void AddExtension(VkDeviceCreateInfo &ci, const char *extension_name) { + char **exts = new char *[ci.enabledExtensionCount + 1]; + memcpy(exts, ci.ppEnabledExtensionNames, sizeof(char *) * ci.enabledExtensionCount); + exts[ci.enabledExtensionCount] = vku::SafeStringCopy(extension_name); + delete[] ci.ppEnabledExtensionNames; + ci.ppEnabledExtensionNames = exts; + ci.enabledExtensionCount++; + return; +} + +void Validator::PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, + const VkAllocationCallbacks *pAllocator, VkDevice *pDevice, + const RecordObject &record_obj, VkDeviceCreateInfo *modified_create_info) { + BaseClass::PreCallRecordCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice, record_obj, modified_create_info); + + // Force enable required features + // If the features are not supported, can't internal error until post device creation + VkPhysicalDeviceFeatures supported_features{}; + DispatchGetPhysicalDeviceFeatures(physicalDevice, &supported_features); + + if (auto enabled_features = const_cast(modified_create_info->pEnabledFeatures)) { + if (supported_features.fragmentStoresAndAtomics) { + enabled_features->fragmentStoresAndAtomics = VK_TRUE; + } + if (supported_features.vertexPipelineStoresAndAtomics) { + enabled_features->vertexPipelineStoresAndAtomics = VK_TRUE; + } + } + + auto add_missing_features = [this, modified_create_info]() { + if (force_buffer_device_address_) { + // Add buffer device address feature + auto *bda_features = const_cast( + vku::FindStructInPNextChain(modified_create_info)); + if (bda_features) { + bda_features->bufferDeviceAddress = VK_TRUE; + } else { + new_bda_features = vku::InitStructHelper(); + new_bda_features.bufferDeviceAddress = VK_TRUE; + auto *chain_end = vku::FindLastStructInPNextChain((void *)modified_create_info->pNext); + chain_end->pNext = reinterpret_cast(&new_bda_features); + } + } + + // Add timeline semaphore feature + auto *ts_features = const_cast( + vku::FindStructInPNextChain(modified_create_info)); + if (ts_features) { + ts_features->timelineSemaphore = VK_TRUE; + } else { + new_ts_features = vku::InitStructHelper(); + new_ts_features.timelineSemaphore = VK_TRUE; + auto *chain_end = vku::FindLastStructInPNextChain((void *)modified_create_info->pNext); + chain_end->pNext = reinterpret_cast(&new_ts_features); + } + }; + + // TODO - multi-device is not supported + if (api_version > VK_API_VERSION_1_1) { + auto *features12 = const_cast( + vku::FindStructInPNextChain(modified_create_info->pNext)); + if (features12) { + features12->timelineSemaphore = VK_TRUE; + if (force_buffer_device_address_) { + features12->bufferDeviceAddress = VK_TRUE; + } + } else { + add_missing_features(); + } + } else if (api_version == VK_API_VERSION_1_1) { + auto find_extension = [modified_create_info](const char *extension_name) { + for (uint32_t i = 0; i < modified_create_info->enabledExtensionCount; i++) { + if (strcmp(modified_create_info->ppEnabledExtensionNames[i], extension_name) == 0) { + return true; + } + } + return false; + }; + + const std::string_view bda_ext{"VK_KHR_buffer_device_address"}; + const bool found_bda = find_extension(bda_ext.data()); + + const std::string_view ts_ext{"VK_KHR_timeline_semaphore"}; + const bool found_ts = find_extension(ts_ext.data()); + + // Add our new extensions + if (!found_bda) { + AddExtension(*modified_create_info, bda_ext.data()); + } + if (!found_ts) { + AddExtension(*modified_create_info, ts_ext.data()); + } + + add_missing_features(); + } else { + force_buffer_device_address_ = false; + } +} + // Perform initializations that can be done at Create Device time. -void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { +void Validator::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { // Add the callback hooks for the functions that are either broadly or deeply used and that the ValidationStateTracker refactor // would be messier without. // TODO: Find a good way to do this hooklessly. @@ -104,28 +204,31 @@ void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Locati // TODO: Such a call is expected to be the first thing happening in this function, // but moving it at the top breaks GPU-AV. Try to fix it - BaseClass::CreateDevice(pCreateInfo, loc); + BaseClass::PostCreateDevice(pCreateInfo, loc); if (api_version < VK_API_VERSION_1_1) { InternalError(device, loc, "GPU-Assisted validation requires Vulkan 1.1 or later. Aborting GPU-AV."); return; } - DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features_); - if (!supported_features_.fragmentStoresAndAtomics || !supported_features_.vertexPipelineStoresAndAtomics) { - InternalError( - device, loc, - "GPU-Assisted validation requires fragmentStoresAndAtomics and vertexPipelineStoresAndAtomics. Aborting GPU-AV."); + VkPhysicalDeviceFeatures supported_features{}; + DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features); + + if (!supported_features.fragmentStoresAndAtomics) { + InternalError(device, loc, "GPU-Assisted validation requires fragmentStoresAndAtomics. Aborting GPU-AV."); + return; + } + if (!supported_features.vertexPipelineStoresAndAtomics) { + InternalError(device, loc, "GPU-Assisted validation requires vertexPipelineStoresAndAtomics. Aborting GPU-AV."); return; } - const VkBool32 shaderInt64 = supported_features_.shaderInt64; bda_validation_possible = ((IsExtEnabled(device_extensions.vk_ext_buffer_device_address) || IsExtEnabled(device_extensions.vk_khr_buffer_device_address)) && - shaderInt64 && enabled_features.bufferDeviceAddress); + supported_features.shaderInt64 && enabled_features.bufferDeviceAddress); if (!bda_validation_possible) { if (gpuav_settings.validate_bda) { - if (!shaderInt64) { + if (!supported_features.shaderInt64) { LogWarning("WARNING-GPU-Assisted-Validation", device, loc, "Buffer device address validation option was enabled, but required features shaderInt64 is not enabled. " "Disabling option."); diff --git a/layers/gpu/debug_printf/debug_printf.cpp b/layers/gpu/debug_printf/debug_printf.cpp index d597677911a..fdb52fa14a5 100644 --- a/layers/gpu/debug_printf/debug_printf.cpp +++ b/layers/gpu/debug_printf/debug_printf.cpp @@ -24,8 +24,28 @@ namespace debug_printf { +void Validator::PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, + const VkAllocationCallbacks *pAllocator, VkDevice *pDevice, + const RecordObject &record_obj, VkDeviceCreateInfo *modified_create_info) { + BaseClass::PreCallRecordCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice, record_obj, modified_create_info); + + // Force enable required features + // If the features are not supported, can't internal error until post device creation + VkPhysicalDeviceFeatures supported_features{}; + DispatchGetPhysicalDeviceFeatures(physicalDevice, &supported_features); + + if (auto enabled_features = const_cast(modified_create_info->pEnabledFeatures)) { + if (supported_features.fragmentStoresAndAtomics) { + enabled_features->fragmentStoresAndAtomics = VK_TRUE; + } + if (supported_features.vertexPipelineStoresAndAtomics) { + enabled_features->vertexPipelineStoresAndAtomics = VK_TRUE; + } + } +} + // Perform initializations that can be done at Create Device time. -void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { +void Validator::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { if (enabled[gpu_validation]) { InternalError(device, loc, "Debug Printf cannot be enabled when gpu assisted validation is enabled."); return; @@ -49,19 +69,20 @@ void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Locati nullptr}; instrumentation_bindings_.push_back(binding); - BaseClass::CreateDevice(pCreateInfo, loc); // will set up bindings + BaseClass::PostCreateDevice(pCreateInfo, loc); // will set up bindings if (api_version < VK_API_VERSION_1_1) { InternalError(device, loc, "Debug Printf requires Vulkan 1.1 or later."); return; } - DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features_); - if (!supported_features_.fragmentStoresAndAtomics) { + VkPhysicalDeviceFeatures supported_features{}; + DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features); + if (!supported_features.fragmentStoresAndAtomics) { InternalError(device, loc, "Debug Printf requires fragmentStoresAndAtomics."); return; } - if (!supported_features_.vertexPipelineStoresAndAtomics) { + if (!supported_features.vertexPipelineStoresAndAtomics) { InternalError(device, loc, "Debug Printf requires vertexPipelineStoresAndAtomics."); return; } diff --git a/layers/gpu/debug_printf/debug_printf.h b/layers/gpu/debug_printf/debug_printf.h index 2ce4fd85df2..427feb330d1 100644 --- a/layers/gpu/debug_printf/debug_printf.h +++ b/layers/gpu/debug_printf/debug_printf.h @@ -81,13 +81,12 @@ namespace debug_printf { class Validator : public gpu::GpuShaderInstrumentor { public: using BaseClass = gpu::GpuShaderInstrumentor; - Validator() { - container_type = LayerObjectTypeDebugPrintf; - desired_features_.vertexPipelineStoresAndAtomics = true; - desired_features_.fragmentStoresAndAtomics = true; - } + Validator() { container_type = LayerObjectTypeDebugPrintf; } - void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override; + void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo, + const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj, + VkDeviceCreateInfo* modified_create_info) override; + void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override; bool InstrumentShader(const vvl::span& input, uint32_t unique_shader_id, const Location& loc, std::vector& out_instrumented_spirv) override; void PreCallRecordCreateShaderModule(VkDevice device, const VkShaderModuleCreateInfo* pCreateInfo, diff --git a/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp b/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp index db1fe3279eb..3a70f2cd52b 100644 --- a/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp +++ b/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp @@ -178,123 +178,6 @@ WriteLockGuard GpuShaderInstrumentor::WriteLock() { } } -void GpuShaderInstrumentor::PreCallRecordCreateDevice(VkPhysicalDevice gpu, const VkDeviceCreateInfo *pCreateInfo, - const VkAllocationCallbacks *pAllocator, VkDevice *pDevice, - const RecordObject &record_obj, - vku::safe_VkDeviceCreateInfo *modified_create_info) { - BaseClass::PreCallRecordCreateDevice(gpu, pCreateInfo, pAllocator, pDevice, record_obj, modified_create_info); - VkPhysicalDeviceFeatures *features = nullptr; - // Use a local variable to query features since this method runs in the instance validation object. - // To avoid confusion and race conditions about which physical device's features are stored in the - // 'supported_devices' member variable, it will only be set in the device validation objects. - // See CreateDevice() below. - VkPhysicalDeviceFeatures gpu_supported_features; - DispatchGetPhysicalDeviceFeatures(gpu, &gpu_supported_features); - - // See CreateDevice() in chassis.cpp. modified_create_info is a pointer to a safe struct stored on the stack. - // This code follows the safe struct memory memory management scheme. That is, we must delete any memory - // remove from the safe struct, and any additions must be allocated in a way that is compatible with - // the safe struct destructor. - if (modified_create_info->pEnabledFeatures) { - // If pEnabledFeatures, VkPhysicalDeviceFeatures2 in pNext chain is not allowed - features = const_cast(modified_create_info->pEnabledFeatures); - } else { - auto *features2 = const_cast( - vku::FindStructInPNextChain(modified_create_info->pNext)); - if (features2) features = &features2->features; - } - VkPhysicalDeviceFeatures new_features = {}; - VkBool32 *desired = reinterpret_cast(&desired_features_); - VkBool32 *feature_ptr; - if (features) { - feature_ptr = reinterpret_cast(features); - } else { - feature_ptr = reinterpret_cast(&new_features); - } - VkBool32 *supported = reinterpret_cast(&supported_features_); - for (size_t i = 0; i < sizeof(VkPhysicalDeviceFeatures); i += (sizeof(VkBool32))) { - if (*supported && *desired) { - *feature_ptr = true; - } - supported++; - desired++; - feature_ptr++; - } - if (!features) { - delete modified_create_info->pEnabledFeatures; - modified_create_info->pEnabledFeatures = new VkPhysicalDeviceFeatures(new_features); - } - - auto add_missing_features = [this, modified_create_info]() { - if (force_buffer_device_address_) { - // Add buffer device address feature - auto *bda_features = const_cast( - vku::FindStructInPNextChain(modified_create_info)); - if (bda_features) { - bda_features->bufferDeviceAddress = VK_TRUE; - } else { - VkPhysicalDeviceBufferDeviceAddressFeatures new_bda_features = vku::InitStructHelper(); - new_bda_features.bufferDeviceAddress = VK_TRUE; - vku::AddToPnext(*modified_create_info, new_bda_features); - } - } - - // Add timeline semaphore feature - auto *ts_features = const_cast( - vku::FindStructInPNextChain(modified_create_info)); - if (ts_features) { - ts_features->timelineSemaphore = VK_TRUE; - } else { - VkPhysicalDeviceTimelineSemaphoreFeatures new_ts_features = vku::InitStructHelper(); - new_ts_features.timelineSemaphore = VK_TRUE; - vku::AddToPnext(*modified_create_info, new_ts_features); - } - }; - - // TODO How to handle multi-device - if (api_version > VK_API_VERSION_1_1) { - auto *features12 = const_cast( - vku::FindStructInPNextChain(modified_create_info->pNext)); - if (features12) { - features12->timelineSemaphore = VK_TRUE; - if (force_buffer_device_address_) { - features12->bufferDeviceAddress = VK_TRUE; - } - } else { - add_missing_features(); - } - } else if (api_version == VK_API_VERSION_1_1) { - const std::string_view bda_ext{"VK_KHR_buffer_device_address"}; - bool found_bda = false; - for (uint32_t i = 0; i < modified_create_info->enabledExtensionCount; i++) { - if (bda_ext == modified_create_info->ppEnabledExtensionNames[i]) { - found_bda = true; - break; - } - } - const std::string_view ts_ext{"VK_KHR_timeline_semaphore"}; - bool found_ts = false; - for (uint32_t i = 0; i < modified_create_info->enabledExtensionCount; i++) { - if (ts_ext == modified_create_info->ppEnabledExtensionNames[i]) { - found_ts = true; - break; - } - } - - // Add our new extensions - if (!found_bda) { - vku::AddExtension(*modified_create_info, bda_ext.data()); - } - if (!found_ts) { - vku::AddExtension(*modified_create_info, ts_ext.data()); - } - - add_missing_features(); - } else { - force_buffer_device_address_ = false; - } -} - std::shared_ptr GpuShaderInstrumentor::CreateQueue(VkQueue q, uint32_t index, VkDeviceQueueCreateFlags flags, const VkQueueFamilyProperties &queueFamilyProperties) { return std::static_pointer_cast( @@ -302,8 +185,8 @@ std::shared_ptr GpuShaderInstrumentor::CreateQueue(VkQueue q, uint32 } // In charge of getting things for shader instrumentation that both GPU-AV and DebugPrintF will need -void GpuShaderInstrumentor::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { - BaseClass::CreateDevice(pCreateInfo, loc); +void GpuShaderInstrumentor::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { + BaseClass::PostCreateDevice(pCreateInfo, loc); // If api version 1.1 or later, SetDeviceLoaderData will be in the loader auto chain_info = GetChainInfo(pCreateInfo, VK_LOADER_DATA_CALLBACK); assert(chain_info->u.pfnSetDeviceLoaderData); diff --git a/layers/gpu/instrumentation/gpu_shader_instrumentor.h b/layers/gpu/instrumentation/gpu_shader_instrumentor.h index 36fc7ab2b2e..89ea33d66bd 100644 --- a/layers/gpu/instrumentation/gpu_shader_instrumentor.h +++ b/layers/gpu/instrumentation/gpu_shader_instrumentor.h @@ -58,10 +58,7 @@ class GpuShaderInstrumentor : public ValidationStateTracker { ReadLockGuard ReadLock() const override; WriteLockGuard WriteLock() override; - void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, - const VkAllocationCallbacks *pAllocator, VkDevice *pDevice, const RecordObject &record_obj, - vku::safe_VkDeviceCreateInfo *modified_create_info) override; - void CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) override; + void PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) override; void PreCallRecordDestroyDevice(VkDevice device, const VkAllocationCallbacks *pAllocator, const RecordObject &record_obj) override; @@ -170,8 +167,6 @@ class GpuShaderInstrumentor : public ValidationStateTracker { bool force_buffer_device_address_; PFN_vkSetDeviceLoaderData vk_set_device_loader_data_; - VkPhysicalDeviceFeatures supported_features_{}; - VkPhysicalDeviceFeatures desired_features_{}; uint32_t adjusted_max_desc_sets_ = 0; std::atomic unique_shader_module_id_ = 1; // zero represents no shader module found uint32_t output_buffer_byte_size_ = 0; diff --git a/layers/state_tracker/state_tracker.cpp b/layers/state_tracker/state_tracker.cpp index 5c608d01dc3..acf0d69ebd7 100644 --- a/layers/state_tracker/state_tracker.cpp +++ b/layers/state_tracker/state_tracker.cpp @@ -692,7 +692,7 @@ void ValidationStateTracker::PostCallRecordCreateDevice(VkPhysicalDevice gpu, co // Save local link to this device's physical device state device_state->physical_device_state = Get(gpu).get(); // finish setup in the object representing the device - device_state->CreateDevice(pCreateInfo, record_obj.location); + device_state->PostCreateDevice(pCreateInfo, record_obj.location); } std::shared_ptr ValidationStateTracker::CreateQueue(VkQueue handle, uint32_t index, VkDeviceQueueCreateFlags flags, @@ -700,7 +700,7 @@ std::shared_ptr ValidationStateTracker::CreateQueue(VkQueue handle, return std::make_shared(*this, handle, index, flags, queueFamilyProperties); } -void ValidationStateTracker::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { +void ValidationStateTracker::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { GetEnabledDeviceFeatures(pCreateInfo, &enabled_features, api_version); const auto *device_group_ci = vku::FindStructInPNextChain(pCreateInfo->pNext); diff --git a/layers/state_tracker/state_tracker.h b/layers/state_tracker/state_tracker.h index 1dcbc899692..f4dd10f416b 100644 --- a/layers/state_tracker/state_tracker.h +++ b/layers/state_tracker/state_tracker.h @@ -605,7 +605,7 @@ class ValidationStateTracker : public ValidationObject { void PostCallRecordCreateDevice(VkPhysicalDevice gpu, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj) override; - virtual void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc); + virtual void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc); void PreCallRecordDestroyDevice(VkDevice device, const VkAllocationCallbacks* pAllocator, const RecordObject& record_obj) override; diff --git a/layers/sync/sync_validation.cpp b/layers/sync/sync_validation.cpp index a6cf3e15563..fa616effc5d 100644 --- a/layers/sync/sync_validation.cpp +++ b/layers/sync/sync_validation.cpp @@ -568,9 +568,9 @@ void SyncValidator::PreCallRecordCmdPipelineBarrier2(VkCommandBuffer commandBuff *pDependencyInfo); } -void SyncValidator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { +void SyncValidator::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) { // The state tracker sets up the device state - StateTracker::CreateDevice(pCreateInfo, loc); + StateTracker::PostCreateDevice(pCreateInfo, loc); ForEachShared([this](const std::shared_ptr &queue_state) { auto queue_flags = physical_device_state->queue_family_properties[queue_state->queueFamilyIndex].queueFlags; diff --git a/layers/sync/sync_validation.h b/layers/sync/sync_validation.h index 7ef81ead41b..70a14b17ace 100644 --- a/layers/sync/sync_validation.h +++ b/layers/sync/sync_validation.h @@ -105,7 +105,7 @@ class SyncValidator : public ValidationStateTracker, public SyncStageAccess { void RecordCmdEndRenderPass(VkCommandBuffer commandBuffer, const VkSubpassEndInfo *pSubpassEndInfo, Func command); bool SupressedBoundDescriptorWAW(const HazardResult &hazard) const; - void CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) override; + void PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) override; bool ValidateBeginRenderPass(VkCommandBuffer commandBuffer, const VkRenderPassBeginInfo *pRenderPassBegin, const VkSubpassBeginInfo *pSubpassBeginInfo, const ErrorObject &error_obj) const; diff --git a/layers/vulkan/generated/chassis.cpp b/layers/vulkan/generated/chassis.cpp index 70d681faa3a..45ca72cb5e9 100644 --- a/layers/vulkan/generated/chassis.cpp +++ b/layers/vulkan/generated/chassis.cpp @@ -593,7 +593,21 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice gpu, const VkDevice item->device_extensions = device_extensions; } - vku::safe_VkDeviceCreateInfo modified_create_info(pCreateInfo); + // Make copy to modify, note that we can't use safe_VkDeviceCreateInfo because it is very possible to have a stack overflow + // when trying to delete all the feature pNext strutures + // (see https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8110). + VkDeviceCreateInfo modified_create_info = *pCreateInfo; + // There might be multiple calls modifying the object, so make a deep copy where needed + if (pCreateInfo->pEnabledFeatures) { + modified_create_info.pEnabledFeatures = new VkPhysicalDeviceFeatures(*pCreateInfo->pEnabledFeatures); + } + if (pCreateInfo->ppEnabledExtensionNames) { + modified_create_info.ppEnabledExtensionNames = new char*[pCreateInfo->enabledExtensionCount]; + memcpy((char**)modified_create_info.ppEnabledExtensionNames, pCreateInfo->ppEnabledExtensionNames, + sizeof(char*) * pCreateInfo->enabledExtensionCount); + } + // for pNext just simply reset the last pNext to null, the ValidationObject is in charge of the memory + auto* chain_end = vku::FindLastStructInPNextChain((void*)modified_create_info.pNext); bool skip = false; ErrorObject error_obj(vvl::Func::vkCreateDevice, VulkanTypedHandle(gpu, kVulkanObjectTypePhysicalDevice)); @@ -609,7 +623,18 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice gpu, const VkDevice intercept->PreCallRecordCreateDevice(gpu, pCreateInfo, pAllocator, pDevice, record_obj, &modified_create_info); } - VkResult result = fpCreateDevice(gpu, reinterpret_cast(&modified_create_info), pAllocator, pDevice); + VkResult result = fpCreateDevice(gpu, &modified_create_info, pAllocator, pDevice); + + if (pCreateInfo->pEnabledFeatures) { + delete modified_create_info.pEnabledFeatures; + } + if (pCreateInfo->ppEnabledExtensionNames) { + delete modified_create_info.ppEnabledExtensionNames; + } + if (chain_end) { + chain_end->pNext = nullptr; + } + if (result != VK_SUCCESS) { return result; } diff --git a/layers/vulkan/generated/chassis.h b/layers/vulkan/generated/chassis.h index 1ac02e9ac42..22dcbc0f009 100644 --- a/layers/vulkan/generated/chassis.h +++ b/layers/vulkan/generated/chassis.h @@ -4583,7 +4583,7 @@ class ValidationObject { }; // Modify a parameter to CreateDevice - virtual void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj, vku::safe_VkDeviceCreateInfo *modified_create_info) { + virtual void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj, VkDeviceCreateInfo *modified_create_info) { PreCallRecordCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice, record_obj); }; }; diff --git a/scripts/generators/feature_requirements.py b/scripts/generators/feature_requirements.py index 49180a40002..45f62829e6b 100644 --- a/scripts/generators/feature_requirements.py +++ b/scripts/generators/feature_requirements.py @@ -25,7 +25,7 @@ class FeatureRequirementsGenerator(BaseGenerator): def __init__(self): BaseGenerator.__init__(self) - + # Features of the VkPhysicalDeviceBufferDeviceAddressFeaturesEXT have # the same name as Vulkan 1.2 and # VkPhysicalDeviceBufferDeviceAddressFeaturesKHR features, but are @@ -33,7 +33,7 @@ def __init__(self): self.identical_but_different_features = { 'VkPhysicalDeviceBufferDeviceAddressFeaturesEXT', } - + def generate(self): out = [] out.append(f'''// *** THIS FILE IS GENERATED - DO NOT EDIT *** @@ -68,9 +68,9 @@ def generate(self): out.append('\n// NOLINTEND') # Wrap for clang-tidy to ignore self.write("".join(out)) - + def getFeaturesAndOrigins(self) -> dict: - # Get all Vulkan Physical Device Features + # Get all Vulkan Physical Device Features featureMap = dict() feature_structs = self.vk.structs['VkPhysicalDeviceFeatures2'].extendedBy feature_structs.append('VkPhysicalDeviceFeatures') @@ -78,7 +78,7 @@ def getFeaturesAndOrigins(self) -> dict: if extending_struct_name in self.identical_but_different_features: continue extending_struct = self.vk.structs[extending_struct_name] - + for feature in extending_struct.members: if feature.type == 'VkBool32': if feature.name not in featureMap: @@ -89,7 +89,7 @@ def getFeaturesAndOrigins(self) -> dict: # features by that comment. That ensures features of the same struct end up together. featuresAndOrigins = sorted([(sorted(structs), feature) for feature, structs in featureMap.items()]) - + return featuresAndOrigins def generateHeader(self): @@ -97,9 +97,9 @@ def generateHeader(self): out.append(''' #include "vk_api_version.h" - #include + #include - namespace vkt { + namespace vkt { ''') # Physical device features enum @@ -108,22 +108,22 @@ def generateHeader(self): out.append(f'// {", ".join(origins)}\n') out.append(f'{feature},\n') out.append('};\n') - + # Functions declarations out.append(''' struct FeatureAndName { VkBool32 *feature; const char *name; }; - - // Find or add the correct VkPhysicalDeviceFeature struct in `pnext_chain` based on `feature`, + + // Find or add the correct VkPhysicalDeviceFeature struct in `pnext_chain` based on `feature`, // a vkt::Feature enum value, and set feature to VK_TRUE FeatureAndName AddFeature(APIVersion api_version, vkt::Feature feature, void **inout_pnext_chain); - + }// namespace vkt ''') return "".join(out) - + # Find the Vulkan version in a VkPhysicalDeviceVulkanFeatures struct def getApiVersion(self, structs): if structs.find('11') != -1: @@ -133,7 +133,7 @@ def getApiVersion(self, structs): if structs.find('13') != -1: return 'VK_API_VERSION_1_3' else: - assert False + assert False def generateSource(self): out = [] @@ -156,7 +156,7 @@ def generateSource(self): vulkan_feature_struct_i = i break if len(origins) == 1 or vulkan_feature_struct_i == -1: - feature_struct_name = origins[0] + feature_struct_name = origins[0] out.extend(guard_helper.add_guard(self.vk.structs[feature_struct_name].protect)) out.append(f''' case Feature::{feature}: {{ auto vk_struct = const_cast<{feature_struct_name} *>(vku::FindStructInPNextChain<{feature_struct_name}>(*inout_pnext_chain)); @@ -167,7 +167,7 @@ def generateSource(self): vvl::PnextChainAdd(*inout_pnext_chain, vk_struct); }} else {{ *inout_pnext_chain = vk_struct; - }} + }} }} return {{&vk_struct->{feature}, "{feature_struct_name}::{feature}"}}; }} @@ -180,41 +180,41 @@ def generateSource(self): ref_api_version = self.getApiVersion(api_struct_name) out.extend(guard_helper.add_guard(self.vk.structs[feature_struct_name].protect)) out.append(f''' -case Feature::{feature}: - if (api_version >= {ref_api_version}) {{ - auto vk_struct = const_cast<{api_struct_name} *>(vku::FindStructInPNextChain<{api_struct_name}>(*inout_pnext_chain)); - if (!vk_struct) {{ - vk_struct = new {api_struct_name}; - *vk_struct = vku::InitStructHelper(); - if (*inout_pnext_chain) {{ - vvl::PnextChainAdd(*inout_pnext_chain, vk_struct); - }} else {{ - *inout_pnext_chain = vk_struct; - }} - }} - return {{&vk_struct->{feature}, "{api_struct_name}::{feature}"}}; - }} else {{ - auto vk_struct = const_cast<{feature_struct_name} *>(vku::FindStructInPNextChain<{feature_struct_name}>(*inout_pnext_chain)); - if (!vk_struct) {{ - vk_struct = new {feature_struct_name}; - *vk_struct = vku::InitStructHelper(); - if (*inout_pnext_chain) {{ - vvl::PnextChainAdd(*inout_pnext_chain, vk_struct); - }} else {{ - *inout_pnext_chain = vk_struct; - }} - }} - return {{&vk_struct->{feature}, "{feature_struct_name}::{feature}"}}; - }}''') + case Feature::{feature}: + if (api_version >= {ref_api_version}) {{ + auto vk_struct = const_cast<{api_struct_name} *>(vku::FindStructInPNextChain<{api_struct_name}>(*inout_pnext_chain)); + if (!vk_struct) {{ + vk_struct = new {api_struct_name}; + *vk_struct = vku::InitStructHelper(); + if (*inout_pnext_chain) {{ + vvl::PnextChainAdd(*inout_pnext_chain, vk_struct); + }} else {{ + *inout_pnext_chain = vk_struct; + }} + }} + return {{&vk_struct->{feature}, "{api_struct_name}::{feature}"}}; + }} else {{ + auto vk_struct = const_cast<{feature_struct_name} *>(vku::FindStructInPNextChain<{feature_struct_name}>(*inout_pnext_chain)); + if (!vk_struct) {{ + vk_struct = new {feature_struct_name}; + *vk_struct = vku::InitStructHelper(); + if (*inout_pnext_chain) {{ + vvl::PnextChainAdd(*inout_pnext_chain, vk_struct); + }} else {{ + *inout_pnext_chain = vk_struct; + }} + }} + return {{&vk_struct->{feature}, "{feature_struct_name}::{feature}"}}; + }}''') out.extend(guard_helper.add_guard(None)) out.append('''default: assert(false); return {nullptr, ""}; }''') - + out.append('}\n') - + out.append('}// namespace vkt') return "".join(out) diff --git a/scripts/generators/layer_chassis_generator.py b/scripts/generators/layer_chassis_generator.py index 32bcec9d788..3288376c6ed 100644 --- a/scripts/generators/layer_chassis_generator.py +++ b/scripts/generators/layer_chassis_generator.py @@ -716,7 +716,7 @@ class ValidationObject { }; // Modify a parameter to CreateDevice - virtual void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj, vku::safe_VkDeviceCreateInfo *modified_create_info) { + virtual void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj, VkDeviceCreateInfo *modified_create_info) { PreCallRecordCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice, record_obj); }; }; @@ -1269,7 +1269,21 @@ def generateSource(self): item->device_extensions = device_extensions; } - vku::safe_VkDeviceCreateInfo modified_create_info(pCreateInfo); + // Make copy to modify, note that we can't use safe_VkDeviceCreateInfo because it is very possible to have a stack overflow + // when trying to delete all the feature pNext strutures + // (see https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8110). + VkDeviceCreateInfo modified_create_info = *pCreateInfo; + // There might be multiple calls modifying the object, so make a deep copy where needed + if (pCreateInfo->pEnabledFeatures) { + modified_create_info.pEnabledFeatures = new VkPhysicalDeviceFeatures(*pCreateInfo->pEnabledFeatures); + } + if (pCreateInfo->ppEnabledExtensionNames) { + modified_create_info.ppEnabledExtensionNames = new char*[pCreateInfo->enabledExtensionCount]; + memcpy((char**)modified_create_info.ppEnabledExtensionNames, pCreateInfo->ppEnabledExtensionNames, + sizeof(char*) * pCreateInfo->enabledExtensionCount); + } + // for pNext just simply reset the last pNext to null, the ValidationObject is in charge of the memory + auto* chain_end = vku::FindLastStructInPNextChain((void*)modified_create_info.pNext); bool skip = false; ErrorObject error_obj(vvl::Func::vkCreateDevice, VulkanTypedHandle(gpu, kVulkanObjectTypePhysicalDevice)); @@ -1285,7 +1299,18 @@ def generateSource(self): intercept->PreCallRecordCreateDevice(gpu, pCreateInfo, pAllocator, pDevice, record_obj, &modified_create_info); } - VkResult result = fpCreateDevice(gpu, reinterpret_cast(&modified_create_info), pAllocator, pDevice); + VkResult result = fpCreateDevice(gpu, &modified_create_info, pAllocator, pDevice); + + if (pCreateInfo->pEnabledFeatures) { + delete modified_create_info.pEnabledFeatures; + } + if (pCreateInfo->ppEnabledExtensionNames) { + delete modified_create_info.ppEnabledExtensionNames; + } + if (chain_end) { + chain_end->pNext = nullptr; + } + if (result != VK_SUCCESS) { return result; }