From 9f9419c21fc597b4e71cfdde526bfdf124ee4179 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 27 Apr 2022 13:46:02 +0100 Subject: [PATCH] Preprocess #line directives in SPIR-V debug info to get referenced files * This matches the line number and source file references generated in the debug info --- .../driver/shaders/dxbc/dxbc_bytecode_ops.cpp | 13 +- renderdoc/driver/shaders/dxbc/dxbc_common.h | 2 +- .../driver/shaders/dxbc/dxbc_container.cpp | 221 +---------------- .../driver/shaders/dxbc/dxbc_reflect.cpp | 7 +- renderdoc/driver/shaders/dxbc/dxbc_sdbg.cpp | 2 +- renderdoc/driver/shaders/dxbc/dxbc_spdb.cpp | 16 +- .../driver/shaders/dxil/dxil_reflect.cpp | 2 +- .../driver/shaders/spirv/spirv_reflect.cpp | 11 +- renderdoc/replay/replay_driver.cpp | 223 ++++++++++++++++++ renderdoc/replay/replay_driver.h | 1 + 10 files changed, 254 insertions(+), 244 deletions(-) diff --git a/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp b/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp index d4c214329..222b77b11 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp @@ -894,7 +894,7 @@ void Program::MakeDisassemblyString() fileLines.resize(m_DebugInfo->Files.size()); for(size_t i = 0; i < m_DebugInfo->Files.size(); i++) - split(m_DebugInfo->Files[i].second, fileLines[i], '\n'); + split(m_DebugInfo->Files[i].contents, fileLines[i], '\n'); } for(size_t d = 0; d < m_Declarations.size(); d++) @@ -977,15 +977,16 @@ void Program::MakeDisassemblyString() if(!func.empty()) { - m_Disassembly += StringFormat::Fmt("%s:%d - %s()\n", - m_DebugInfo->Files[lineInfo.fileIndex].first.c_str(), - lineInfo.lineStart, func.c_str()); + m_Disassembly += StringFormat::Fmt( + "%s:%d - %s()\n", m_DebugInfo->Files[lineInfo.fileIndex].filename.c_str(), + lineInfo.lineStart, func.c_str()); linenum++; } else { - m_Disassembly += StringFormat::Fmt( - "%s:%d\n", m_DebugInfo->Files[lineInfo.fileIndex].first.c_str(), lineInfo.lineStart); + m_Disassembly += + StringFormat::Fmt("%s:%d\n", m_DebugInfo->Files[lineInfo.fileIndex].filename.c_str(), + lineInfo.lineStart); linenum++; } } diff --git a/renderdoc/driver/shaders/dxbc/dxbc_common.h b/renderdoc/driver/shaders/dxbc/dxbc_common.h index 488420e9a..a15db9ae6 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_common.h +++ b/renderdoc/driver/shaders/dxbc/dxbc_common.h @@ -384,7 +384,7 @@ public: virtual ShaderCompileFlags GetShaderCompileFlags() const = 0; - rdcarray> Files; // + rdcarray Files; virtual void GetLineInfo(size_t instruction, uintptr_t offset, LineColumnInfo &lineInfo) const = 0; virtual void GetCallstack(size_t instruction, uintptr_t offset, diff --git a/renderdoc/driver/shaders/dxbc/dxbc_container.cpp b/renderdoc/driver/shaders/dxbc/dxbc_container.cpp index 534c7a5d2..601198b5f 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_container.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_container.cpp @@ -31,8 +31,8 @@ #include "driver/shaders/dxil/dxil_bytecode.h" #include "lz4/lz4.h" #include "md5/md5.h" +#include "replay/replay_driver.h" #include "serialise/serialiser.h" -#include "strings/string_utils.h" #include "dxbc_bytecode.h" #include "driver/dx/official/d3dcompiler.h" @@ -1911,224 +1911,7 @@ DXBCContainer::DXBCContainer(const bytebuf &ByteCode, const rdcstr &debugInfoPat if(m_DXBCByteCode) m_DXBCByteCode->SetDebugInfo(m_DebugInfo); - struct SplitFile - { - rdcstr filename; - rdcarray lines; - bool modified = false; - }; - - rdcarray splitFiles; - - splitFiles.resize(m_DebugInfo->Files.size()); - - for(size_t i = 0; i < m_DebugInfo->Files.size(); i++) - splitFiles[i].filename = m_DebugInfo->Files[i].first; - - for(size_t i = 0; i < m_DebugInfo->Files.size(); i++) - { - rdcarray srclines; - - // start off writing to the corresponding output file. - SplitFile *dstFile = &splitFiles[i]; - bool changedFile = false; - - size_t dstLine = 0; - - // skip this file if it doesn't contain #line - if(!m_DebugInfo->Files[i].second.contains("#line")) - continue; - - split(m_DebugInfo->Files[i].second, srclines, '\n'); - srclines.push_back(""); - - // handle #line directives by inserting empty lines or erasing as necessary - bool seenLine = false; - - for(size_t srcLine = 0; srcLine < srclines.size(); srcLine++) - { - if(srclines[srcLine].empty()) - { - dstLine++; - continue; - } - - char *c = &srclines[srcLine][0]; - char *end = c + srclines[srcLine].size(); - - while(*c == '\t' || *c == ' ' || *c == '\r') - c++; - - if(c == end) - { - // blank line, just advance line counter - dstLine++; - continue; - } - - if(c + 5 > end || strncmp(c, "#line", 5) != 0) - { - // only actually insert the line if we've seen a #line statement. Otherwise we're just - // doing an identity copy. This can lead to problems e.g. if there are a few statements in - // a file before the #line we then create a truncated list of lines with only those and - // then start spitting the #line directives into other files. We still want to have the - // original file as-is. - if(seenLine) - { - // resize up to account for the current line, if necessary - dstFile->lines.resize(RDCMAX(dstLine + 1, dstFile->lines.size())); - - // if non-empty, append this line (to allow multiple lines on the same line - // number to be concatenated). To avoid screwing up line numbers we have to append with - // a comment and not a newline. - if(dstFile->lines[dstLine].empty()) - dstFile->lines[dstLine] = srclines[srcLine]; - else - dstFile->lines[dstLine] += " /* multiple #lines overlapping */ " + srclines[srcLine]; - - dstFile->modified = true; - } - - // advance line counter - dstLine++; - - continue; - } - - seenLine = true; - - // we have a #line directive - c += 5; - - if(c >= end) - { - // invalid #line, just advance line counter - dstLine++; - continue; - } - - while(*c == '\t' || *c == ' ') - c++; - - if(c >= end) - { - // invalid #line, just advance line counter - dstLine++; - continue; - } - - // invalid #line, no line number. Skip/ignore and just advance line counter - if(*c < '0' || *c > '9') - { - dstLine++; - continue; - } - - size_t newLineNum = 0; - while(*c >= '0' && *c <= '9') - { - newLineNum *= 10; - newLineNum += int((*c) - '0'); - c++; - } - - // convert to 0-indexed line number - if(newLineNum > 0) - newLineNum--; - - while(*c == '\t' || *c == ' ') - c++; - - if(*c == '"') - { - // filename - c++; - - char *filename = c; - - // parse out filename - while(*c != '"' && *c != 0) - { - if(*c == '\\') - { - // skip escaped characters - c += 2; - } - else - { - c++; - } - } - - // parsed filename successfully - if(*c == '"') - { - *c = 0; - - rdcstr fname = filename; - if(fname.empty()) - fname = "shader"; - - // find the new destination file - bool found = false; - size_t dstFileIdx = 0; - - for(size_t f = 0; f < splitFiles.size(); f++) - { - if(splitFiles[f].filename == fname) - { - found = true; - dstFileIdx = f; - break; - } - } - - if(found) - { - changedFile = (dstFile != &splitFiles[dstFileIdx]); - dstFile = &splitFiles[dstFileIdx]; - } - else - { - RDCWARN("Couldn't find filename '%s' in #line directive in debug info", fname.c_str()); - - // make a dummy file to write into that won't be used. - splitFiles.push_back(SplitFile()); - splitFiles.back().filename = fname; - splitFiles.back().modified = true; - - changedFile = true; - dstFile = &splitFiles.back(); - } - - // set the next line number, and continue processing - dstLine = newLineNum; - - continue; - } - else - { - // invalid #line, ignore - RDCERR("Couldn't parse #line directive: '%s'", srclines[srcLine].c_str()); - continue; - } - } - else - { - // No filename. Set the next line number, and continue processing - dstLine = newLineNum; - continue; - } - } - } - - for(size_t i = 0; i < m_DebugInfo->Files.size(); i++) - { - if(m_DebugInfo->Files[i].second.empty() || splitFiles[i].modified) - { - merge(splitFiles[i].lines, m_DebugInfo->Files[i].second, '\n'); - } - } + PreprocessLineDirectives(m_DebugInfo->Files); } // if we had bytecode in this container, ensure we had reflection. If it's a blob with only an diff --git a/renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp b/renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp index 3a1c4812e..53c9b531f 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp @@ -255,12 +255,7 @@ void MakeShaderReflection(DXBC::DXBCContainer *dxbc, ShaderReflection *refl, refl->debugInfo.compileFlags = dxbc->GetDebugInfo()->GetShaderCompileFlags(); - refl->debugInfo.files.resize(dxbc->GetDebugInfo()->Files.size()); - for(size_t i = 0; i < dxbc->GetDebugInfo()->Files.size(); i++) - { - refl->debugInfo.files[i].filename = dxbc->GetDebugInfo()->Files[i].first; - refl->debugInfo.files[i].contents = dxbc->GetDebugInfo()->Files[i].second; - } + refl->debugInfo.files = dxbc->GetDebugInfo()->Files; rdcstr entry = dxbc->GetDebugInfo()->GetEntryFunction(); if(entry.empty()) diff --git a/renderdoc/driver/shaders/dxbc/dxbc_sdbg.cpp b/renderdoc/driver/shaders/dxbc/dxbc_sdbg.cpp index d66a03849..8abc624cf 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_sdbg.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_sdbg.cpp @@ -87,7 +87,7 @@ SDBGChunk::SDBGChunk(void *data) rdcstr(asciiDatabase + m_FileHeaders[i].filenameOffset, m_FileHeaders[i].filenameLen); rdcstr source = rdcstr(asciiDatabase + m_FileHeaders[i].sourceOffset, m_FileHeaders[i].sourceLen); - this->Files.push_back(make_rdcpair(filename, source)); + this->Files.push_back({filename, source}); } // successful grab of info diff --git a/renderdoc/driver/shaders/dxbc/dxbc_spdb.cpp b/renderdoc/driver/shaders/dxbc/dxbc_spdb.cpp index 402bb0d30..6fc7057cc 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_spdb.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_spdb.cpp @@ -1427,7 +1427,7 @@ SPDBChunk::SPDBChunk(byte *data, uint32_t spdblength) for(size_t i = 0; i < Files.size(); i++) { - if(!_stricmp(Files[i].first.c_str(), name.c_str())) + if(!_stricmp(Files[i].filename.c_str(), name.c_str())) { fileIdx = (int32_t)i; break; @@ -1443,7 +1443,7 @@ SPDBChunk::SPDBChunk(byte *data, uint32_t spdblength) for(size_t i = 0; i < Files.size(); i++) { - rdcstr normalised = Files[i].first; + rdcstr normalised = Files[i].filename; for(char &c : normalised) if(c == '\\') c = '/'; @@ -1649,12 +1649,12 @@ SPDBChunk::SPDBChunk(byte *data, uint32_t spdblength) rdcarray filenames; filenames.reserve(Files.size()); for(size_t i = 0; i < Files.size(); i++) - filenames.push_back(Files[i].first); + filenames.push_back(Files[i].filename); // Sort files according to the order they come in the Names array, this seems to be more reliable // about placing the main file first. std::sort(Files.begin(), Files.end(), - [&Names](const rdcpair &a, const rdcpair &b) { + [&Names](const ShaderSourceFile &a, const ShaderSourceFile &b) { // any entries that aren't found in Names at all (like @cmdline that we add) will be // sorted to // the end. @@ -1663,9 +1663,9 @@ SPDBChunk::SPDBChunk(byte *data, uint32_t spdblength) size_t i = 0; for(auto it = Names.begin(); it != Names.end(); ++it) { - if(it->second == a.first) + if(it->second == a.filename) aIdx = i; - if(it->second == b.first) + if(it->second == b.filename) bIdx = i; i++; @@ -1673,7 +1673,7 @@ SPDBChunk::SPDBChunk(byte *data, uint32_t spdblength) // if neither were found, sort by filename if(aIdx == bIdx) - return a.first < b.first; + return a.filename < b.filename; return aIdx < bIdx; }); @@ -1681,7 +1681,7 @@ SPDBChunk::SPDBChunk(byte *data, uint32_t spdblength) // create a map from filename -> index std::map remapping; for(size_t i = 0; i < Files.size(); i++) - remapping[Files[i].first] = (int32_t)i; + remapping[Files[i].filename] = (int32_t)i; // remap the line info by looking up the original intended filename, then looking up the new index for(auto it = m_InstructionInfo.begin(); it != m_InstructionInfo.end(); ++it) diff --git a/renderdoc/driver/shaders/dxil/dxil_reflect.cpp b/renderdoc/driver/shaders/dxil/dxil_reflect.cpp index 0536c2792..2e8e17f1f 100644 --- a/renderdoc/driver/shaders/dxil/dxil_reflect.cpp +++ b/renderdoc/driver/shaders/dxil/dxil_reflect.cpp @@ -833,7 +833,7 @@ DXBC::Reflection *Program::GetReflection() { for(size_t i = 1; i < Files.size(); i++) { - if(Files[i].first == mainFile) + if(Files[i].filename == mainFile) { std::swap(Files[0], Files[i]); break; diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index 96dcb9a43..97ddc6456 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -719,10 +719,17 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st case SourceLanguage::Max: break; } - if(!sources[i].contents.empty()) - reflection.debugInfo.files.push_back({sources[i].name, sources[i].contents}); + reflection.debugInfo.files.push_back({sources[i].name, sources[i].contents}); } + PreprocessLineDirectives(reflection.debugInfo.files); + + // we do a mini-preprocess of the files from the debug info to handle #line directives. + // This means that any lines that our source file declares to be in another filename via a #line + // get put in the right place for what the debug information hopefully matches. + // We also concatenate duplicate lines and display them all, to handle edge cases where #lines + // declare duplicates. + if(knownExtSet[ExtSet_ShaderDbg] != Id() && !reflection.debugInfo.files.empty()) { reflection.debugInfo.compileFlags.flags.push_back({"preferSourceDebug", "1"}); diff --git a/renderdoc/replay/replay_driver.cpp b/renderdoc/replay/replay_driver.cpp index 9cc86c415..598f3e545 100644 --- a/renderdoc/replay/replay_driver.cpp +++ b/renderdoc/replay/replay_driver.cpp @@ -27,6 +27,7 @@ #include "maths/formatpacking.h" #include "maths/half_convert.h" #include "serialise/serialiser.h" +#include "strings/string_utils.h" template <> rdcstr DoStringise(const RemapTexture &el) @@ -545,6 +546,228 @@ static void StandardFillCBufferVariables(ResourceId shader, const rdcarray &sourceFiles) +{ + struct SplitFile + { + rdcstr filename; + rdcarray lines; + bool modified = false; + }; + + rdcarray splitFiles; + + splitFiles.resize(sourceFiles.size()); + + for(size_t i = 0; i < sourceFiles.size(); i++) + splitFiles[i].filename = sourceFiles[i].filename; + + for(size_t i = 0; i < sourceFiles.size(); i++) + { + rdcarray srclines; + + // start off writing to the corresponding output file. + SplitFile *dstFile = &splitFiles[i]; + bool changedFile = false; + + size_t dstLine = 0; + + // skip this file if it doesn't contain #line + if(!sourceFiles[i].contents.contains("#line")) + continue; + + split(sourceFiles[i].contents, srclines, '\n'); + srclines.push_back(""); + + // handle #line directives by inserting empty lines or erasing as necessary + bool seenLine = false; + + for(size_t srcLine = 0; srcLine < srclines.size(); srcLine++) + { + if(srclines[srcLine].empty()) + { + dstLine++; + continue; + } + + char *c = &srclines[srcLine][0]; + char *end = c + srclines[srcLine].size(); + + while(*c == '\t' || *c == ' ' || *c == '\r') + c++; + + if(c == end) + { + // blank line, just advance line counter + dstLine++; + continue; + } + + if(c + 5 > end || strncmp(c, "#line", 5) != 0) + { + // only actually insert the line if we've seen a #line statement. Otherwise we're just + // doing an identity copy. This can lead to problems e.g. if there are a few statements in + // a file before the #line we then create a truncated list of lines with only those and + // then start spitting the #line directives into other files. We still want to have the + // original file as-is. + if(seenLine) + { + // resize up to account for the current line, if necessary + dstFile->lines.resize(RDCMAX(dstLine + 1, dstFile->lines.size())); + + // if non-empty, append this line (to allow multiple lines on the same line + // number to be concatenated). To avoid screwing up line numbers we have to append with + // a comment and not a newline. + if(dstFile->lines[dstLine].empty()) + dstFile->lines[dstLine] = srclines[srcLine]; + else + dstFile->lines[dstLine] += " /* multiple #lines overlapping */ " + srclines[srcLine]; + + dstFile->modified = true; + } + + // advance line counter + dstLine++; + + continue; + } + + seenLine = true; + + // we have a #line directive + c += 5; + + if(c >= end) + { + // invalid #line, just advance line counter + dstLine++; + continue; + } + + while(*c == '\t' || *c == ' ') + c++; + + if(c >= end) + { + // invalid #line, just advance line counter + dstLine++; + continue; + } + + // invalid #line, no line number. Skip/ignore and just advance line counter + if(*c < '0' || *c > '9') + { + dstLine++; + continue; + } + + size_t newLineNum = 0; + while(*c >= '0' && *c <= '9') + { + newLineNum *= 10; + newLineNum += int((*c) - '0'); + c++; + } + + // convert to 0-indexed line number + if(newLineNum > 0) + newLineNum--; + + while(*c == '\t' || *c == ' ') + c++; + + if(*c == '"') + { + // filename + c++; + + char *filename = c; + + // parse out filename + while(*c != '"' && *c != 0) + { + if(*c == '\\') + { + // skip escaped characters + c += 2; + } + else + { + c++; + } + } + + // parsed filename successfully + if(*c == '"') + { + *c = 0; + + rdcstr fname = filename; + if(fname.empty()) + fname = "shader"; + + // find the new destination file + bool found = false; + size_t dstFileIdx = 0; + + for(size_t f = 0; f < splitFiles.size(); f++) + { + if(splitFiles[f].filename == fname) + { + found = true; + dstFileIdx = f; + break; + } + } + + if(found) + { + changedFile = (dstFile != &splitFiles[dstFileIdx]); + dstFile = &splitFiles[dstFileIdx]; + } + else + { + RDCWARN("Couldn't find filename '%s' in #line directive in debug info", fname.c_str()); + + // make a dummy file to write into that won't be used. + splitFiles.push_back(SplitFile()); + splitFiles.back().filename = fname; + splitFiles.back().modified = true; + + changedFile = true; + dstFile = &splitFiles.back(); + } + + // set the next line number, and continue processing + dstLine = newLineNum; + + continue; + } + else + { + // invalid #line, ignore + RDCERR("Couldn't parse #line directive: '%s'", srclines[srcLine].c_str()); + continue; + } + } + else + { + // No filename. Set the next line number, and continue processing + dstLine = newLineNum; + continue; + } + } + } + + for(size_t i = 0; i < sourceFiles.size(); i++) + { + if(sourceFiles[i].contents.empty() || splitFiles[i].modified) + { + merge(splitFiles[i].lines, sourceFiles[i].contents, '\n'); + } + } +} + void StandardFillCBufferVariables(ResourceId shader, const rdcarray &invars, rdcarray &outvars, const bytebuf &data) { diff --git a/renderdoc/replay/replay_driver.h b/renderdoc/replay/replay_driver.h index 08b457d38..6fae5b905 100644 --- a/renderdoc/replay/replay_driver.h +++ b/renderdoc/replay/replay_driver.h @@ -317,6 +317,7 @@ void StandardFillCBufferVariable(ResourceId shader, const ShaderConstantDescript uint32_t matStride); void StandardFillCBufferVariables(ResourceId shader, const rdcarray &invars, rdcarray &outvars, const bytebuf &data); +void PreprocessLineDirectives(rdcarray &sourceFiles); // simple cache for when we need buffer data for highlighting // vertices, typical use will be lots of vertices in the same