From d92ce3710d7f439845ebf00f47b4db6fa4325438 Mon Sep 17 00:00:00 2001 From: John Zulauf Date: Mon, 5 Mar 2018 10:36:21 -0700 Subject: [PATCH 1/2] layers: Update CmdPushConstant Valid Usages Updated the core validation layer and matching units to reflect changes to the valid usage statements in the Vulkan specification to better support the use of overlapping push constant ranges. The following valid usage was removed from the specification and the core validation layer. VUID-vkCmdPushConstants-stageFlags-00367 VALIDATION_ERROR_1bc002de Two new valid usages have been added to the core validation layer. VALIDATION_ERROR_1bc00e06 VUID-vkCmdPushConstants-offset-01795 VALIDATION_ERROR_1bc00e08 VUID-vkCmdPushConstants-offset-01796 The individual unit tests within the larger InvalidPushConstants test case have been updated to reflect the changes to the valid usage checks. Change-Id: I9bba3efa19f96c52e82b328bee64f8f2fbb10342 --- layers/core_validation.cpp | 45 ++++++++++++++-------- layers/vk_validation_error_database.txt | 6 +-- tests/layer_validation_tests.cpp | 50 ++++++++++++++++--------- 3 files changed, 64 insertions(+), 37 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index e1feeadad4..a5f9b41580 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -8234,27 +8234,40 @@ VKAPI_ATTR void VKAPI_CALL CmdPushConstants(VkCommandBuffer commandBuffer, VkPip "vkCmdPushConstants() call has no stageFlags set. %s", validation_error_map[VALIDATION_ERROR_1bc2dc03]); } - // Check if specified push constant range falls within a pipeline-defined range which has matching stageFlags. - // The spec doesn't seem to disallow having multiple push constant ranges with the - // same offset and size, but different stageFlags. So we can't just check the - // stageFlags in the first range with matching offset and size. + // Check if pipeline_layout VkPushConstantRange(s) overlapping offset, size have stageFlags set for each stage in the command + // stageFlags argument, *and* that the command stageFlags argument has bits set for the stageFlags in each overlapping range. if (!skip) { const auto &ranges = *getPipelineLayout(dev_data, layout)->push_constant_ranges; - bool found_matching_range = false; + VkShaderStageFlags found_stages = 0; for (const auto &range : ranges) { - if ((stageFlags == range.stageFlags) && (offset >= range.offset) && (offset + size <= range.offset + range.size)) { - found_matching_range = true; - break; + if ((offset >= range.offset) && (offset + size <= range.offset + range.size)) { + VkShaderStageFlags matching_stages = range.stageFlags & stageFlags; + if (matching_stages != range.stageFlags) { + // VALIDATION_ERROR_1bc00e08 VUID-vkCmdPushConstants-offset-01796 + skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, + VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, HandleToUint64(commandBuffer), __LINE__, + VALIDATION_ERROR_1bc00e08, "DS", + "vkCmdPushConstants(): stageFlags (0x%" PRIx32 ", offset (%" PRIu32 "), and size (%" PRIu32 + "), " + "must contain all stages in overlapping VkPushConstantRange stageFlags (0x%" PRIx32 + "), offset (%" PRIu32 "), and size (%" PRIu32 ") in pipeline layout 0x%" PRIx64 ". %s", + (uint32_t)stageFlags, offset, size, (uint32_t)range.stageFlags, range.offset, range.size, + HandleToUint64(layout), validation_error_map[VALIDATION_ERROR_1bc00e08]); + } + + // Accumulate all stages we've found + found_stages = matching_stages | found_stages; } } - if (!found_matching_range) { - skip |= log_msg( - dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, - HandleToUint64(commandBuffer), __LINE__, VALIDATION_ERROR_1bc002de, "DS", - "vkCmdPushConstants() stageFlags = 0x%" PRIx32 - " do not match the stageFlags in any of the ranges with offset = %d and size = %d in pipeline layout 0x%" PRIx64 - ". %s", - (uint32_t)stageFlags, offset, size, HandleToUint64(layout), validation_error_map[VALIDATION_ERROR_1bc002de]); + if (found_stages != stageFlags) { + // VALIDATION_ERROR_1bc00e06 VUID-vkCmdPushConstants-offset-01795 + uint32_t missing_stages = ~found_stages & stageFlags; + skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, + HandleToUint64(commandBuffer), __LINE__, VALIDATION_ERROR_1bc00e06, "DS", + "vkCmdPushConstants(): stageFlags = 0x%" PRIx32 ", VkPushConstantRange in pipeline layout 0x%" PRIx64 + " overlapping offset = %d and size = %d, do not contain stageFlags 0x%" PRIx32 ". %s", + (uint32_t)stageFlags, HandleToUint64(layout), offset, size, missing_stages, + validation_error_map[VALIDATION_ERROR_1bc00e06]); } } lock.unlock(); diff --git a/layers/vk_validation_error_database.txt b/layers/vk_validation_error_database.txt index 791a4899c7..a5fba71b43 100644 --- a/layers/vk_validation_error_database.txt +++ b/layers/vk_validation_error_database.txt @@ -2382,13 +2382,13 @@ VALIDATION_ERROR_1ba02413~^~N~^~Unknown~^~vkCmdProcessCommandsNVX~^~VUID-vkCmdPr VALIDATION_ERROR_1ba02415~^~N~^~Unknown~^~vkCmdProcessCommandsNVX~^~VUID-vkCmdProcessCommandsNVX-commandBuffer-cmdpool~^~(VK_NVX_device_generated_commands)~^~The spec valid usage text states 'The VkCommandPool that commandBuffer was allocated from must support graphics, or compute operations' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VUID-vkCmdProcessCommandsNVX-commandBuffer-cmdpool)~^~implicit VALIDATION_ERROR_1ba1f201~^~N~^~Unknown~^~vkCmdProcessCommandsNVX~^~VUID-vkCmdProcessCommandsNVX-pProcessCommandsInfo-parameter~^~(VK_NVX_device_generated_commands)~^~The spec valid usage text states 'pProcessCommandsInfo must be a valid pointer to a valid VkCmdProcessCommandsInfoNVX structure' (https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VUID-vkCmdProcessCommandsNVX-pProcessCommandsInfo-parameter)~^~implicit VALIDATION_ERROR_1bc00009~^~Y~^~Unknown~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-commonparent~^~core~^~The spec valid usage text states 'Both of commandBuffer, and layout must have been created, allocated, or retrieved from the same VkDevice' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-commonparent)~^~implicit -VALIDATION_ERROR_1bc002de~^~Y~^~InvalidPushConstants~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-stageFlags-00367~^~core~^~The spec valid usage text states 'stageFlags must match exactly the shader stages used in layout for the range specified by offset and size' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-stageFlags-00367)~^~ +VALIDATION_ERROR_1bc002de~^~N~^~None~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-stageFlags-00367~^~core~^~The spec valid usage text states 'stageFlags must match exactly the shader stages used in layout for the range specified by offset and size' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-stageFlags-00367)~^~ VALIDATION_ERROR_1bc002e0~^~Y~^~Unknown~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-offset-00368~^~core~^~The spec valid usage text states 'offset must be a multiple of 4' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-offset-00368)~^~ VALIDATION_ERROR_1bc002e2~^~Y~^~Unknown~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-size-00369~^~core~^~The spec valid usage text states 'size must be a multiple of 4' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-size-00369)~^~ VALIDATION_ERROR_1bc002e4~^~Y~^~Unknown~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-offset-00370~^~core~^~The spec valid usage text states 'offset must be less than VkPhysicalDeviceLimits::maxPushConstantsSize' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-offset-00370)~^~ VALIDATION_ERROR_1bc002e6~^~Y~^~Unknown~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-size-00371~^~core~^~The spec valid usage text states 'size must be less than or equal to VkPhysicalDeviceLimits::maxPushConstantsSize minus offset' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-size-00371)~^~ -VALIDATION_ERROR_1bc00e06~^~N~^~None~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-offset-01795~^~core~^~The spec valid usage text states 'For each byte in the range specified by offset and size and for each shader stage in stageFlags, there must be a push constant range in layout that includes that byte and that stage' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-offset-01795)~^~ -VALIDATION_ERROR_1bc00e08~^~N~^~None~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-offset-01796~^~core~^~The spec valid usage text states 'For each byte in the range specified by offset and size and for each push constant range that overlaps that byte, stageFlags must include all stages in that push constant range's VkPushConstantRange::stageFlags' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-offset-01796)~^~ +VALIDATION_ERROR_1bc00e06~^~Y~^~InvalidPushConstants~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-offset-01795~^~core~^~The spec valid usage text states 'For each byte in the range specified by offset and size and for each shader stage in stageFlags, there must be a push constant range in layout that includes that byte and that stage' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-offset-01795)~^~ +VALIDATION_ERROR_1bc00e08~^~Y~^~InvalidPushConstants~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-offset-01796~^~core~^~The spec valid usage text states 'For each byte in the range specified by offset and size and for each push constant range that overlaps that byte, stageFlags must include all stages in that push constant range's VkPushConstantRange::stageFlags' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-offset-01796)~^~ VALIDATION_ERROR_1bc02401~^~Y~^~None~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-commandBuffer-parameter~^~core~^~The spec valid usage text states 'commandBuffer must be a valid VkCommandBuffer handle' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-commandBuffer-parameter)~^~implicit VALIDATION_ERROR_1bc02413~^~Y~^~None~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-commandBuffer-recording~^~core~^~The spec valid usage text states 'commandBuffer must be in the recording state' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-commandBuffer-recording)~^~implicit VALIDATION_ERROR_1bc02415~^~Y~^~Unknown~^~vkCmdPushConstants~^~VUID-vkCmdPushConstants-commandBuffer-cmdpool~^~core~^~The spec valid usage text states 'The VkCommandPool that commandBuffer was allocated from must support graphics, or compute operations' (https://www.khronos.org/registry/vulkan/specs/1.0/html/vkspec.html#VUID-vkCmdPushConstants-commandBuffer-cmdpool)~^~implicit diff --git a/tests/layer_validation_tests.cpp b/tests/layer_validation_tests.cpp index 402d7dfe1e..68e93c7c15 100644 --- a/tests/layer_validation_tests.cpp +++ b/tests/layer_validation_tests.cpp @@ -9187,9 +9187,9 @@ TEST_F(VkLayerTest, InvalidPushConstants) { // CmdPushConstants tests // - // Setup a pipeline layout with ranges: [0,16) [64,80) - const std::vector pc_range2 = {{VK_SHADER_STAGE_VERTEX_BIT, 64, 16}, - {VK_SHADER_STAGE_FRAGMENT_BIT, 0, 16}}; + // Setup a pipeline layout with ranges: [0,32) [16,80) + const std::vector pc_range2 = {{VK_SHADER_STAGE_VERTEX_BIT, 16, 64}, + {VK_SHADER_STAGE_FRAGMENT_BIT, 0, 32}}; const VkPipelineLayoutObj pipeline_layout_obj(m_device, {}, pc_range2); const uint8_t dummy_values[100] = {}; @@ -9207,22 +9207,36 @@ TEST_F(VkLayerTest, InvalidPushConstants) { vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), VK_SHADER_STAGE_FRAGMENT_BIT, 0, 16, dummy_values); m_errorMonitor->VerifyNotFound(); m_errorMonitor->ExpectSuccess(); - vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), VK_SHADER_STAGE_VERTEX_BIT, 64, 16, dummy_values); + vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), VK_SHADER_STAGE_VERTEX_BIT, 32, 48, dummy_values); m_errorMonitor->VerifyNotFound(); - const std::array cmd_range_tests = {{ - {VK_SHADER_STAGE_FRAGMENT_BIT, 64, 16}, - {VK_SHADER_STAGE_VERTEX_BIT, 0, 16}, - {VK_SHADER_STAGE_GEOMETRY_BIT, 0, 16}, - {VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT, 0, 16}, - {VK_SHADER_STAGE_VERTEX_BIT, 24, 16}, - {VK_SHADER_STAGE_VERTEX_BIT, 8, 4}, - }}; - for (const auto &iter : cmd_range_tests) { - m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1bc002de); - vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), iter.stageFlags, iter.offset, iter.size, - dummy_values); - m_errorMonitor->VerifyFound(); - } + m_errorMonitor->ExpectSuccess(); + vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), + VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT, 16, 16, dummy_values); + m_errorMonitor->VerifyNotFound(); + + // Wrong cmd stages for extant range + // No range for all cmd stages -- VALIDATION_ERROR_1bc00e06 VUID-vkCmdPushConstants-offset-01795 + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1bc00e06); + // Missing cmd stages for found overlapping range -- VALIDATION_ERROR_1bc00e08 VUID-vkCmdPushConstants-offset-01796 + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1bc00e08); + vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), VK_SHADER_STAGE_GEOMETRY_BIT, 0, 16, dummy_values); + m_errorMonitor->VerifyFound(); + + // Wrong no extant range + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1bc00e06); + vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), VK_SHADER_STAGE_FRAGMENT_BIT, 80, 4, dummy_values); + m_errorMonitor->VerifyFound(); + + // Wrong overlapping extent + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1bc00e06); + vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), + VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT, 0, 20, dummy_values); + m_errorMonitor->VerifyFound(); + + // Wrong stage flags for valid overlapping range + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1bc00e08); + vkCmdPushConstants(m_commandBuffer->handle(), pipeline_layout_obj.handle(), VK_SHADER_STAGE_VERTEX_BIT, 16, 16, dummy_values); + m_errorMonitor->VerifyFound(); m_commandBuffer->EndRenderPass(); m_commandBuffer->end(); From c513ac2ebac2e418a3b51a9654986bb46d848930 Mon Sep 17 00:00:00 2001 From: John Zulauf Date: Fri, 9 Mar 2018 14:45:22 -0700 Subject: [PATCH 2/2] layers: Strawman/dartboard new message logger TODO: Rebase this on top of MikeS recent changes. For your review/critique is a quick and dirty working protoptype of a new message logger for the validation layers. The goals were: * No repetition of known data (i.e. types/VUIDs) * No casts when constructing messages. * Flexible formatting options. * No formatting if message will not be emitted. * A starting place for more brainstorming * Quick and dirty Missing: * even better automatic casting/overload. (given further code generation) * detailed interface design (naming, and method vs. operator) Messy: * Use of MACRO for msg generation (to compress flags and __LINE__) * add_x and add_h for hex and handle handling... * transitions between built-in type support (<< operations) and .add* method operations * Using << style formatting. (maybe not familiar to maintainers) Pretty Cool: * Automatic object type detection * No-op output functions if messages are not enabled * Built-in sprintf conversions * Automatic update of "skip", and post to debug_log_msg in destructor * Automatic appending of validation_error_message (when avail) * No more PRI ... Change-Id: I5ea8f5e95c9d765ab846038d6ceab60fc111d3d4 --- layers/core_validation.cpp | 26 ++--- layers/vk_layer_logging.h | 157 +++++++++++++++++++++++++++++-- scripts/helper_file_generator.py | 48 +++++++--- 3 files changed, 193 insertions(+), 38 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index a5f9b41580..39a1f7731a 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -412,11 +412,10 @@ static bool ValidateMemoryIsValid(layer_data *dev_data, VkDeviceMemory mem, uint DEVICE_MEM_INFO *mem_info = GetMemObjInfo(dev_data, mem); if (mem_info) { if (!mem_info->bound_ranges[bound_object_handle].valid) { - return log_msg(dev_data->report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT, - HandleToUint64(mem), __LINE__, MEMTRACK_INVALID_MEM_REGION, "MEM", - "%s: Cannot read invalid region of memory allocation 0x%" PRIx64 " for bound %s object 0x%" PRIx64 - ", please fill the memory before using.", - functionName, HandleToUint64(mem), object_string[type], bound_object_handle); + LOG_WARN_MSG(nullptr, msg, dev_data->report_data, mem, MEMTRACK_INVALID_MEM_REGION, "MEM"); + msg.add(functionName).add(": Cannot read invalid region of memory allocation ").add_h(mem); + msg.add_fmt(" for bound %s object ", object_string[type]).add_h(bound_object_handle); + return msg.report(); } } return false; @@ -1303,11 +1302,8 @@ static bool ValidatePipelineLocked(layer_data *dev_data, std::vectorgraphicsPipelineCI.basePipelineIndex != -1) { if (pPipeline->graphicsPipelineCI.basePipelineIndex >= pipelineIndex) { - skip |= - log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT, - HandleToUint64(pPipeline->pipeline), __LINE__, VALIDATION_ERROR_208005a0, "DS", - "Invalid Pipeline CreateInfo: base pipeline must occur earlier in array than derivative pipeline. %s", - validation_error_map[VALIDATION_ERROR_208005a0]); + LOG_ERR_MSG(&skip, msg, dev_data->report_data, pPipeline->pipeline, VALIDATION_ERROR_208005a0, "DS"); + msg.add("Invalid Pipeline CreateInfo: base pipeline must occur earlier in array than derivative pipeline."); } else { pBasePipeline = pPipelines[pPipeline->graphicsPipelineCI.basePipelineIndex].get(); } @@ -2681,12 +2677,10 @@ static bool validateQueueFamilyIndices(layer_data *dev_data, GLOBAL_CB_NODE *pCB if (pPool && queue_state) { if (pPool->queueFamilyIndex != queue_state->queueFamilyIndex) { - skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, - HandleToUint64(pCB->commandBuffer), __LINE__, VALIDATION_ERROR_31a00094, "DS", - "vkQueueSubmit: Primary command buffer 0x%" PRIx64 - " created in queue family %d is being submitted on queue 0x%" PRIx64 " from queue family %d. %s", - HandleToUint64(pCB->commandBuffer), pPool->queueFamilyIndex, HandleToUint64(queue), - queue_state->queueFamilyIndex, validation_error_map[VALIDATION_ERROR_31a00094]); + LOG_ERR_MSG(&skip, msg, dev_data->report_data, pCB->commandBuffer, VALIDATION_ERROR_31a00094, "DS"); + msg.add("vkQueueSubmit: Primary command buffer ").add_h(pCB->commandBuffer); + msg << " created in queue family " << pPool->queueFamilyIndex << "is being submitted on queue "; + msg.add_h(queue) << " from queue family " << queue_state->queueFamilyIndex; } // Ensure that any bound images or buffers created with SHARING_MODE_CONCURRENT have access to the current queue family diff --git a/layers/vk_layer_logging.h b/layers/vk_layer_logging.h index 559b5a45ad..d5c163c6f2 100644 --- a/layers/vk_layer_logging.h +++ b/layers/vk_layer_logging.h @@ -26,13 +26,16 @@ #include "vk_layer_config.h" #include "vk_layer_data.h" #include "vk_layer_table.h" +#include "vk_validation_error_messages.h" #include "vk_loader_platform.h" +#include "vk_object_types.h" #include "vulkan/vk_layer.h" #include #include #include #include #include +#include #include #include @@ -314,7 +317,7 @@ static inline void layer_disable_tmp_callbacks(debug_report_data *debug_data, ui // Checks if the message will get logged. // Allows layer to defer collecting & formating data if the // message will be discarded. -static inline bool will_log_msg(debug_report_data *debug_data, VkFlags msgFlags) { +static inline bool will_log_msg(const debug_report_data *debug_data, VkFlags msgFlags) { if (!debug_data || !(debug_data->active_flags & msgFlags)) { // Message is not wanted return false; @@ -322,21 +325,34 @@ static inline bool will_log_msg(debug_report_data *debug_data, VkFlags msgFlags) return true; } + +// sprintf to std::string, concatenating onto to_string if non-null, returns formatted string by value #ifndef WIN32 -static inline int string_sprintf(std::string *output, const char *fmt, ...) __attribute__((format(printf, 2, 3))); +static inline std::string string_sprintf(std::string *to_string, const char *fmt, ...) __attribute__((format(printf, 2, 3))); #endif -static inline int string_sprintf(std::string *output, const char *fmt, ...) { - std::string &formatted = *output; +static inline std::string string_sprintf(std::string *to_string, const char *fmt, ...) { + std::string local; + to_string = (nullptr != to_string) ? to_string : &local; + std::string &formatted = *to_string; + size_t offset = formatted.size(); + + // Find out how many bytes we'll need. va_list argptr; va_start(argptr, fmt); int reserve = vsnprintf(nullptr, 0, fmt, argptr); va_end(argptr); - formatted.reserve(reserve + 1); + formatted.reserve(offset + reserve + 1); + formatted.resize(offset + reserve); + + // sprintf those bytes to the output va_start(argptr, fmt); - int result = vsnprintf((char *)formatted.data(), formatted.capacity(), fmt, argptr); +#ifndef NDEBUG + int result = +#endif + vsnprintf((char *)formatted.data() + offset, formatted.capacity(), fmt, argptr); va_end(argptr); assert(result == reserve); - return result; + return formatted; } #ifdef WIN32 @@ -439,4 +455,131 @@ uint64_t HandleToUint64(HANDLE_T h); static inline uint64_t HandleToUint64(uint64_t h) { return h; } +class LogMsg { + public: + template + LogMsg(bool *status, const debug_report_data *debug_data, VkFlags msg_flags, ObjectType src_object, size_t location, + int32_t msg_code, const char *layer_prefix) + : status_(status), + debug_data_(debug_data), + msg_flags_(msg_flags), + src_object_(HandleToUint64(src_object)), + src_object_type_(VkObjectTypeTraits::kObjectType), + location_(location), + msg_code_(msg_code), + msg_code_is_vu_(false), + layer_prefix_(layer_prefix), + will_report_(will_log_msg(debug_data, msg_flags)), + reported_(false) {} + template + LogMsg(bool *status, const debug_report_data *debug_data, VkFlags msg_flags, ObjectType src_object, size_t location, + UNIQUE_VALIDATION_ERROR_CODE msg_code, const char *layer_prefix) + : LogMsg(status, debug_data, msg_flags, src_object, location, static_cast(msg_code), layer_prefix) { + msg_code_is_vu_ = true; + } + + template + LogMsg &add_x(IntType out) { + if (will_report_) { + // We could use string_sprintf, but we'll try the awful native formatting support for this at least + report_ << "0x"; + auto save_fill = report_.fill('0'); + auto save_width = report_.width(2 * sizeof(IntType)); + report_ << std::hex << out << std::dec; + report_.fill(save_fill); + report_.width(save_width); + } + return *this; + } + + template + LogMsg &add(RefType *out) { + return add_x(out); + } + + template + LogMsg &add_h(const HandleType h) { + if (!will_report_) { + add_x(HandleToUint64(h)); + } + return *this; + } + + // Anything with an acceptable stringstream operation can just use add + template + LogMsg &add(const OutType out) { + if (will_report_) { + report_ << out; + } + return *this; + } + + // use sprintf to add formatted information +#ifndef WIN32 +#define LOG_MSG_ADD_FMT_ATTR __attribute__((format(printf, 2, 3))) +#else +#define LOG_MSG_ADD_FMT_ATTR +#endif + LogMsg &add_fmt(const char *fmt, ...) LOG_MSG_ADD_FMT_ATTR { + if (will_report_) { + va_list argptr; + va_start(argptr, fmt); + char *str; + if (-1 != vasprintf(&str, fmt, argptr)) { + report_ << str; + free(str); + } + va_end(argptr); + } + return *this; + } + + bool will_report() const { return will_report_; }; + bool report() { + bool result = false; + if (will_report_ && !reported_) { + if (msg_code_is_vu_) { + add(" "); + add(validation_error_map[msg_code_]); + } + result = debug_report_log_msg(debug_data_, msg_flags_, get_debug_report_enum[src_object_type_], src_object_, location_, + msg_code_, layer_prefix_, report_.str().c_str()); + reported_ = true; + if (status_) { + // Update this in the same chaining way as log_msg usage + *status_ |= result; + } + } + return result; + } + + ~LogMsg() { report(); } + + private: + bool *status_; + const debug_report_data *debug_data_; + VkFlags msg_flags_; + uint64_t src_object_; + VulkanObjectType src_object_type_; + size_t location_; + int32_t msg_code_; + bool msg_code_is_vu_; + const char *layer_prefix_; + bool will_report_; + std::stringstream report_; + bool reported_; +}; + +// stream out to log msg using the public interface +template +LogMsg &operator<<(LogMsg &logger, const OutType &out) { + return logger.add(out); +} + +#define LOG_ERR_MSG(skip, msg, report_data, object, code, prefix) \ + LogMsg msg(skip, report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, object, __LINE__, code, prefix); + +#define LOG_WARN_MSG(skip, msg, report_data, object, code, prefix) \ + LogMsg msg(skip, report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, object, __LINE__, code, prefix); + #endif // LAYER_LOGGING_H diff --git a/scripts/helper_file_generator.py b/scripts/helper_file_generator.py index f47857a82f..93eba865a9 100644 --- a/scripts/helper_file_generator.py +++ b/scripts/helper_file_generator.py @@ -682,6 +682,7 @@ def GenerateObjectTypesHelperHeader(self): object_types_helper_header += '#include \n\n' object_types_helper_header += self.GenerateObjectTypesHeader() return object_types_helper_header + # # Object types header: create object enum type header file def GenerateObjectTypesHeader(self): @@ -716,15 +717,15 @@ def GenerateObjectTypesHeader(self): object_types_header += '// Helper array to get Vulkan VK_EXT_debug_report object type enum from the internal layers version\n' object_types_header += 'const VkDebugReportObjectTypeEXT get_debug_report_enum[] = {\n' object_types_header += ' VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, // No Match\n' + + # Use map comprehensions to convert from k to VK with this helper + to_key = lambda regex, raw_key: re.search(regex, raw_key).group(1).lower().replace("_","") + + dbg_re = '^VK_DEBUG_REPORT_OBJECT_TYPE_(.*)_EXT$' + dbg_map = {to_key(dbg_re, dbg) : dbg for dbg in self.debug_report_object_types} for object_type in type_list: - search_type = object_type.replace("kVulkanObjectType", "").lower() - for vk_object_type in self.debug_report_object_types: - target_type = vk_object_type.replace("VK_DEBUG_REPORT_OBJECT_TYPE_", "").lower() - target_type = target_type[:-4] - target_type = target_type.replace("_", "") - if search_type == target_type: - object_types_header += ' %s, // %s\n' % (vk_object_type, object_type) - break + vk_object_type = dbg_map[object_type.replace("kVulkanObjectType", "").lower()] + object_types_header += ' %s, // %s\n' % (vk_object_type, object_type) object_types_header += '};\n' # Output a conversion routine from the layer object definitions to the core object type definitions @@ -732,16 +733,33 @@ def GenerateObjectTypesHeader(self): object_types_header += '// Helper array to get Official Vulkan VkObjectType enum from the internal layers version\n' object_types_header += 'const VkObjectType get_object_type_enum[] = {\n' object_types_header += ' VK_OBJECT_TYPE_UNKNOWN, // No Match\n' + vko_re = '^VK_OBJECT_TYPE_(.*)' + vko_map = {to_key(vko_re, vko) : vko for vko in self.core_object_types} for object_type in type_list: - search_type = object_type.replace("kVulkanObjectType", "").lower() - for vk_object_type in self.core_object_types: - target_type = vk_object_type.replace("VK_OBJECT_TYPE_", "").lower() - target_type = target_type.replace("_", "") - if search_type == target_type: - object_types_header += ' %s, // %s\n' % (vk_object_type, object_type) - break + vk_object_type = vko_map[object_type.replace("kVulkanObjectType", "").lower()] + object_types_header += ' %s, // %s\n' % (vk_object_type, object_type) object_types_header += '};\n' + # Output template cross reference between the various enums, types, and strings + object_types_header += '#ifdef __cplusplus\n' + object_types_header += '// Helper template to get object type enum from a given type\n' + object_types_header += 'template struct VkObjectTypeTraits {\n' + vko_template_body_fmt = (' static const VulkanObjectType kObjectType = {};\n' + + ' static const VkDebugReportObjectTypeEXT kDebugObjectType = {};\n' + + ' static const VkObjectType kCoreObjectType = {};\n') + unknown_enums = ( "kVulkanObjectTypeUnknown", "VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT", "VK_OBJECT_TYPE_UNKNOWN") + object_types_header += vko_template_body_fmt.format(*unknown_enums) + object_types_header += '};\n\n' + + for object_type in type_list: + ot_key = object_type.replace("kVulkanObjectType", "").lower() + vk_type = object_type.replace("kVulkanObjectType", "Vk") + object_types_header += '// Object type enums for %s\n' % (vk_type) + object_types_header += 'template <> struct VkObjectTypeTraits<%s> {\n' % (vk_type) + object_types_header += vko_template_body_fmt.format(object_type, dbg_map[ot_key], vko_map[ot_key]) + object_types_header += '};\n\n' + + object_types_header += '#endif // __cplusplus\n' return object_types_header # # Determine if a structure needs a safe_struct helper function