Skip to content

Commit

Permalink
layers: eradicate remaining VUID enums
Browse files Browse the repository at this point in the history
Improved format stripping of error text strings in vk_validation_stats
script and added functionality to export a new header file that
maps text VUIDs to these spec error strings. Removed lingering
references to UNIQUE_VALIDATION_ERROR_CODE enums and functions that
were still passing (now unused) msg_code parameters.

Updated CMakeLists to avoid build errors on Win64 related to the size
of the new error map.

Change-Id: I1ff74c7b57ce5021271842da5419055820bd4730
  • Loading branch information
daveh-lunarg committed Aug 10, 2018
1 parent 40c0cba commit fab7c18
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 45 deletions.
2 changes: 1 addition & 1 deletion layers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ if(WIN32)
# Applies to all configurations
add_definitions(-D_CRT_SECURE_NO_WARNINGS)
# Avoid: fatal error C1128: number of sections exceeded object file format limit: compile with /bigobj
set_source_files_properties(core_validation.cpp threading.cpp PROPERTIES COMPILE_FLAGS "/bigobj")
set_source_files_properties(core_validation.cpp threading.cpp parameter_validation_utils.cpp PROPERTIES COMPILE_FLAGS "/bigobj")
# Turn off transitional "changed behavior" warning message for Visual Studio versions prior to 2015. The changed behavior is
# that constructor initializers are now fixed to clear the struct members.
add_compile_options("$<$<AND:$<CXX_COMPILER_ID:MSVC>,$<VERSION_LESS:$<CXX_COMPILER_VERSION>,19>>:/wd4351>")
Expand Down
53 changes: 28 additions & 25 deletions layers/vk_layer_logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ static inline void DebugReportFlagsToAnnotFlags(VkDebugReportFlagsEXT dr_flags,

// Forward Declarations
static inline bool debug_log_msg(const debug_report_data *debug_data, VkFlags msg_flags, VkDebugReportObjectTypeEXT object_type,
uint64_t src_object, size_t location, int32_t msg_code, const char *layer_prefix,
const char *message, const char *text_vuid = NULL);
uint64_t src_object, size_t location, const char *layer_prefix, const char *message,
const char *text_vuid);

// Add a debug message callback node structure to the specified callback linked list
static inline void AddDebugCallbackNode(debug_report_data *debug_data, VkLayerDbgFunctionNode **list_head,
Expand All @@ -140,8 +140,8 @@ static inline void RemoveDebugUtilsMessenger(debug_report_data *debug_data, VkLa
*list_head = cur_callback->pNext;
}
debug_log_msg(debug_data, VK_DEBUG_REPORT_DEBUG_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEBUG_REPORT_EXT,
reinterpret_cast<uint64_t &>(cur_callback->messenger.messenger), 0, 0, "DebugUtilsMessenger",
"Destroyed messenger\n");
reinterpret_cast<uint64_t &>(cur_callback->messenger.messenger), 0, "DebugUtilsMessenger",
"Destroyed messenger\n", kVUIDUndefined);
} else {
matched = false;
local_severities |= cur_callback->messenger.messageSeverity;
Expand Down Expand Up @@ -174,8 +174,8 @@ static inline void RemoveDebugUtilsMessageCallback(debug_report_data *debug_data
*list_head = cur_callback->pNext;
}
debug_log_msg(debug_data, VK_DEBUG_REPORT_DEBUG_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEBUG_REPORT_EXT,
reinterpret_cast<uint64_t &>(cur_callback->report.msgCallback), 0, 0, "DebugReport",
"Destroyed callback\n");
reinterpret_cast<uint64_t &>(cur_callback->report.msgCallback), 0, "DebugReport", "Destroyed callback\n",
kVUIDUndefined);
} else {
matched = false;
VkFlags this_severities = 0;
Expand Down Expand Up @@ -203,23 +203,22 @@ static inline void RemoveAllMessageCallbacks(debug_report_data *debug_data, VkLa
prev_callback = current_callback->pNext;
if (!current_callback->is_messenger) {
debug_log_msg(debug_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEBUG_REPORT_EXT,
(uint64_t)current_callback->report.msgCallback, 0, 0, "DebugReport",
"Debug Report callbacks not removed before DestroyInstance");
(uint64_t)current_callback->report.msgCallback, 0, "DebugReport",
"Debug Report callbacks not removed before DestroyInstance", kVUIDUndefined);
} else {
debug_log_msg(debug_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEBUG_REPORT_EXT,
(uint64_t)current_callback->messenger.messenger, 0, 0, "Messenger",
"Debug messengers not removed before DestroyInstance");
(uint64_t)current_callback->messenger.messenger, 0, "Messenger",
"Debug messengers not removed before DestroyInstance", kVUIDUndefined);
}
free(current_callback);
current_callback = prev_callback;
}
*list_head = NULL;
}

// Note that text_vuid is a default parameter, and is optional. See the above forward declaration
static inline bool debug_log_msg(const debug_report_data *debug_data, VkFlags msg_flags, VkDebugReportObjectTypeEXT object_type,
uint64_t src_object, size_t location, int32_t msg_code, const char *layer_prefix,
const char *message, const char *text_vuid) {
uint64_t src_object, size_t location, const char *layer_prefix, const char *message,
const char *text_vuid) {
bool bail = false;
VkLayerDbgFunctionNode *layer_dbg_node = NULL;

Expand All @@ -246,7 +245,7 @@ static inline bool debug_log_msg(const debug_report_data *debug_data, VkFlags ms
callback_data.pNext = NULL;
callback_data.flags = 0;
callback_data.pMessageIdName = text_vuid;
callback_data.messageIdNumber = msg_code;
callback_data.messageIdNumber = 0; // deprecated, validation layers use only the pMessageIdName
callback_data.pMessage = message;
callback_data.queueLabelCount = 0;
callback_data.pQueueLabels = NULL;
Expand Down Expand Up @@ -343,7 +342,7 @@ static inline bool debug_log_msg(const debug_report_data *debug_data, VkFlags ms
new_debug_report_message.insert(0, " [ ");
}

if (layer_dbg_node->report.pfnMsgCallback(msg_flags, object_type, src_object, location, msg_code, layer_prefix,
if (layer_dbg_node->report.pfnMsgCallback(msg_flags, object_type, src_object, location, 0, layer_prefix,
new_debug_report_message.c_str(), layer_dbg_node->pUserData)) {
bail = true;
}
Expand Down Expand Up @@ -583,7 +582,7 @@ static inline VkResult layer_create_report_callback(debug_report_data *debug_dat
}

debug_log_msg(debug_data, VK_DEBUG_REPORT_DEBUG_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEBUG_REPORT_EXT, (uint64_t)*callback, 0,
0, "DebugReport", "Added callback");
"DebugReport", "Added callback", kVUIDUndefined);
return VK_SUCCESS;
}

Expand Down Expand Up @@ -856,19 +855,23 @@ static inline bool log_msg(const debug_report_data *debug_data, VkFlags msg_flag
}
va_end(argptr);

std::string str_plus_spec_text(str);
std::string str_plus_spec_text(str ? str : "Allocation failure");

// If the vuid string is in the error map: find the legacy enum, look up spec text, and tack it onto error message.
int32_t legacy_vuid_enum = VALIDATION_ERROR_UNDEFINED;
if (validation_error_text_map.find(vuid_text.c_str()) != validation_error_text_map.end()) {
legacy_vuid_enum = validation_error_text_map[vuid_text.c_str()];
str_plus_spec_text += " ";
str_plus_spec_text += validation_error_map[legacy_vuid_enum];
// Append the spec error text to the error message, unless it's an UNASSIGNED or UNDEFINED vuid
if ((vuid_text.find("UNASSIGNED-") == std::string::npos) && (vuid_text.find(kVUIDUndefined) == std::string::npos)) {
if (vuid_to_error_text_map.find(vuid_text) == vuid_to_error_text_map.end()) {
// If this happens, you've hit a VUID string that isn't defined in the spec's json file
// Try running 'vk_validation_stats -c' to look for invalid VUID strings in the repo code
assert(0);
} else {
str_plus_spec_text += " The Vulkan spec states: ";
str_plus_spec_text += vuid_to_error_text_map[vuid_text];
}
}

// Append layer prefix with VUID string, pass in recovered legacy numerical VUID
bool result = debug_log_msg(debug_data, msg_flags, object_type, src_object, 0, legacy_vuid_enum, "Validation",
str_plus_spec_text.c_str() ? str_plus_spec_text.c_str() : "Allocation failure", vuid_text.c_str());
bool result = debug_log_msg(debug_data, msg_flags, object_type, src_object, 0, "Validation", str_plus_spec_text.c_str(),
vuid_text.c_str());

free(str);
return result;
Expand Down
94 changes: 83 additions & 11 deletions scripts/vk_validation_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
txt_filename = "validation_error_database.txt"
csv_filename = "validation_error_database.csv"
html_filename = "validation_error_database.html"
# header_file = '../layers/vk_validation_error_messages.h'
header_filename = "../layers/vk_validation_error_messages.h"
test_file = '../tests/layer_validation_tests.cpp'
vuid_prefixes = ['VUID-', 'UNASSIGNED-']

Expand All @@ -64,6 +64,7 @@
'../layers/buffer_validation.cpp',
]

# This needs to be updated as new extensions roll in
khr_aliases = {
'VUID-vkBindBufferMemory2KHR-device-parameter' : 'VUID-vkBindBufferMemory2-device-parameter',
'VUID-vkBindBufferMemory2KHR-pBindInfos-parameter' : 'VUID-vkBindBufferMemory2-pBindInfos-parameter',
Expand Down Expand Up @@ -142,6 +143,7 @@ def printHelp():
print (" [ -text [ <text_out_filename>] ]")
print (" [ -csv [ <csv_out_filename>] ]")
print (" [ -html [ <html_out_filename>] ]")
print (" [ -export_header ]")
print (" [ -verbose ]")
print (" [ -help ]")
print ("\n The vk_validation_stats script parses validation layer source files to")
Expand All @@ -159,6 +161,7 @@ def printHelp():
print (" defaults to 'validation_error_database.csv'")
print (" -html [filename] output the error database in html to <html_database_filename>,")
print (" defaults to 'validation_error_database.html'")
print (" -export_header export a new VUID error text header file to <%s>" % header_filename)
print (" -verbose show your work (to stdout)")

class ValidationJSON:
Expand All @@ -169,13 +172,22 @@ def __init__(self, filename):
self.all_vuids = set()
self.vuid_db = defaultdict(list) # Maps VUID string to list of json-data dicts
self.apiversion = ""
self.re_striptags = re.compile('<.*?>|&(amp;)+lt;|&(amp;)+gt;')
self.duplicate_vuids = set()

# A set of specific regular expression substitutions needed to clean up VUID text
self.regex_dict = {}
self.regex_dict[re.compile('<.*?>|&(amp;)+lt;|&(amp;)+gt;')] = ""
self.regex_dict[re.compile(r'\\\(codeSize \\over 4\\\)')] = "(codeSize/4)"
self.regex_dict[re.compile(r'\\\(\\lceil\{\\mathit\{rasterizationSamples} \\over 32}\\rceil\\\)')] = "(rasterizationSamples/32)"
# Some fancy punctuation chars that break the Android build...
self.regex_dict[re.compile('&#8594;')] = "->" # Arrow char
self.regex_dict[re.compile('&#8217;')] = "'" # Left-slanting apostrophe to apostrophe
self.regex_dict[re.compile('&#822(0|1);')] = "'" # L/R-slanting quotes to apostrophe

def read(self):
self.json_dict = {}
if os.path.isfile(self.filename):
json_file = open(self.filename, 'r')
json_file = open(self.filename, 'r', encoding='utf-8')
self.json_dict = json.load(json_file)
json_file.close()
if len(self.json_dict) == 0:
Expand Down Expand Up @@ -204,15 +216,15 @@ def read(self):
self.implicit_vuids.add(vuid_string) # otherwise, implicit
vtype = 'implicit'
vuid_text = ventry['text']
#if 'amp;' in vuid_text:
# print(vuid_text)
stripped = re.sub(self.re_striptags, '', vuid_text) # strip tags & literals
stripped = html.unescape(stripped) # anything missed by the regex
#if 'amp;' in stripped:
# print(" %s" % stripped)
self.vuid_db[vuid_string].append({'api':apiname, 'ext':ext, 'type':vtype, 'text':stripped})
for regex, replacement in self.regex_dict.items():
vuid_text = re.sub(regex, replacement, vuid_text) # do regex substitution
vuid_text = html.unescape(vuid_text) # anything missed by the regex
self.vuid_db[vuid_string].append({'api':apiname, 'ext':ext, 'type':vtype, 'text':vuid_text})
self.all_vuids = self.explicit_vuids | self.implicit_vuids
self.duplicate_vuids = set({v for v in self.vuid_db if len(self.vuid_db[v]) > 1})
if len(self.duplicate_vuids) > 0:
print("Warning: duplicate VUIDs found in validusage.json")


class ValidationSource:
def __init__(self, source_file_list, generated_source_file_list, generated_source_directories):
Expand Down Expand Up @@ -452,6 +464,47 @@ def __init__(self, val_json, val_source, val_tests):
self.vj = val_json
self.vs = val_source
self.vt = val_tests
self.header_preamble = """/* THIS FILE IS GENERATED. DO NOT EDIT. */
/* (scripts/vk_validation_stats.py) */
/*
* Vulkan
*
* Copyright (c) 2016-2018 Google Inc.
* Copyright (c) 2016-2018 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.
*
* Author: Tobin Ehlis <[email protected]>
* Author: Dave Houlton <[email protected]>
*/
#pragma once
// Disable auto-formatting for generated file
// clang-format off
#include <string>
#include <unordered_map>
// Mapping from VUID string to the corresponding spec text
#ifdef VALIDATION_ERROR_MAP_IMPL
std::unordered_map<std::string, std::string> vuid_to_error_text_map {
"""
self.header_postamble = """};
#else
extern std::unordered_map<std::string, std::string> vuid_to_error_text_map;
#endif"""
self.spec_url = "https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html"

def dump_txt(self):
print("\n Dumping database to text file: %s" % txt_filename)
Expand Down Expand Up @@ -528,6 +581,21 @@ def dump_html(self):
hfile.write('<th>%s</th></tr>\n' % db_entry['text'])
hfile.write('</table>\n</body>\n</html>\n')

def export_header(self):
print("\n Exporting header file to: %s" % header_filename)
with open (header_filename, 'w') as hfile:
hfile.write(self.header_preamble)
vuid_list = list(self.vj.all_vuids)
vuid_list.sort()
for vuid in vuid_list:
db_entry = self.vj.vuid_db[vuid][0]
hfile.write(' {"%s", "%s (%s#%s)"},\n' % (vuid, db_entry['text'].strip(' '), self.spec_url, vuid))
# For multiply-defined VUIDs, include versions with extension appended
if len(self.vj.vuid_db[vuid]) > 1:
for db_entry in self.vj.vuid_db[vuid]:
hfile.write(' {"%s[%s]", "%s (%s#%s)"},\n' % (vuid, db_entry['ext'].strip(' '), db_entry['text'].strip(' '), self.spec_url, vuid))
hfile.write(self.header_postamble)

def main(argv):
global verbose_mode
global txt_filename
Expand All @@ -540,6 +608,7 @@ def main(argv):
txt_out = False
csv_out = False
html_out = False
header_out = False

if (1 > len(argv)):
printHelp()
Expand Down Expand Up @@ -576,6 +645,8 @@ def main(argv):
if i < len(argv) and not argv[i].startswith('-'):
html_filename = argv[i]
i = i + 1
elif (arg == '-export_header'):
header_out = True
elif (arg in ['-verbose']):
verbose_mode = True
elif (arg in ['-help', '-h']):
Expand Down Expand Up @@ -700,7 +771,8 @@ def main(argv):
db_out.dump_csv()
if html_out:
db_out.dump_html()

if header_out:
db_out.export_header()
return result

if __name__ == "__main__":
Expand Down
10 changes: 2 additions & 8 deletions tests/layer_validation_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class ErrorMonitor {
test_platform_thread_unlock_mutex(&mutex_);
}

VkBool32 CheckForDesiredMsg(const uint32_t message_code, const char *const msgString) {
VkBool32 CheckForDesiredMsg(const char *const msgString) {
VkBool32 result = VK_FALSE;
test_platform_thread_lock_mutex(&mutex_);
if (bailout_ != nullptr) {
Expand Down Expand Up @@ -415,13 +415,7 @@ static VKAPI_ATTR VkBool32 VKAPI_CALL myDbgFunc(VkFlags msgFlags, VkDebugReportO
void *pUserData) {
ErrorMonitor *errMonitor = (ErrorMonitor *)pUserData;
if (msgFlags & errMonitor->GetMessageFlags()) {
#ifdef _DEBUG
char embedded_code_string[2048];
snprintf(embedded_code_string, 2048, "%s [%08x]", pMsg, msgCode);
return errMonitor->CheckForDesiredMsg(msgCode, embedded_code_string);
#else
return errMonitor->CheckForDesiredMsg(msgCode, pMsg);
#endif
return errMonitor->CheckForDesiredMsg(pMsg);
}
return VK_FALSE;
}
Expand Down

0 comments on commit fab7c18

Please sign in to comment.