From 755291ad476d6dd76861e1baf37eb4aef4dfad96 Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 1 Mar 2024 12:11:24 +0000 Subject: [PATCH] Handle SPIR-V with empty and file contents source directives * This can happen if e.g. there's an OpSource with no contents, and then a DebugSource with contents. --- .../driver/shaders/spirv/spirv_reflect.cpp | 97 ++++++++++++------- .../driver/shaders/spirv/spirv_reflect.h | 2 +- 2 files changed, 63 insertions(+), 36 deletions(-) diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index 99a46d6cf..a32918826 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -621,13 +621,26 @@ void Reflector::RegisterOp(Iter it) } } - sources.push_back({source.sourceLanguage, strings[source.file], source.source}); + sourceLanguage = source.sourceLanguage; + + // don't add empty source statements as actual files + if(!source.source.empty()) + { + rdcstr name = strings[source.file]; + if(name.empty()) + name = "unnamed_shader"; + + sources.push_back({name, source.source}); + } } else if(opdata.op == Op::SourceContinued) { OpSourceContinued continued(it); - sources.back().contents += continued.continuedSource; + // in theory someone could do OpSource with no source then OpSourceContinued. We treat this mostly + // as garbage-in, garbage-out, but just in case we have no files don't crash here and instead ignore it. + if(!sources.empty()) + sources.back().contents += continued.continuedSource; } else if(opdata.op == Op::Label) { @@ -647,23 +660,37 @@ void Reflector::RegisterOp(Iter it) { if(dbg.inst == ShaderDbg::Source) { - debugSources[dbg.result] = sources.size(); - sources.push_back({ - SourceLanguage::Unknown, - strings[dbg.arg(0)], - dbg.params.size() > 1 ? strings[dbg.arg(1)] : rdcstr(), - }); + rdcstr name = strings[dbg.arg(0)]; + if(name.empty()) + name = "unnamed_shader"; + rdcstr source = dbg.params.size() > 1 ? strings[dbg.arg(1)] : rdcstr(); + + // don't add empty source statements as actual files + if(!source.empty()) + { + debugSources[dbg.result] = sources.size(); + sources.push_back({name, source}); + } } else if(dbg.inst == ShaderDbg::SourceContinued) { - sources.back().contents += strings[dbg.arg(0)]; + // in theory someone could do OpSource with no source then OpSourceContinued. We treat this + // mostly as garbage-in, garbage-out, but just in case we have no files don't crash here and + // instead ignore it. + if(!sources.empty()) + sources.back().contents += strings[dbg.arg(0)]; } else if(dbg.inst == ShaderDbg::Function) { LineColumnInfo &info = debugFuncToLocation[dbg.result]; - info.fileIndex = (int32_t)debugSources[dbg.arg(2)]; - info.lineStart = info.lineEnd = EvaluateConstant(dbg.arg(3), {}).value.u32v[0]; + // check this source file exists - we won't have registered it if there was no source code + auto srcit = debugSources.find(dbg.arg(2)); + if(srcit != debugSources.end()) + { + info.fileIndex = (int32_t)srcit->second; + info.lineStart = info.lineEnd = EvaluateConstant(dbg.arg(3), {}).value.u32v[0]; + } } else if(dbg.inst == ShaderDbg::FunctionDefinition) { @@ -671,10 +698,11 @@ void Reflector::RegisterOp(Iter it) } else if(dbg.inst == ShaderDbg::CompilationUnit) { - sources[debugSources[dbg.arg(2)]].lang = - (SourceLanguage)EvaluateConstant(dbg.arg(3), {}).value.u32v[0]; + sourceLanguage = (SourceLanguage)EvaluateConstant(dbg.arg(3), {}).value.u32v[0]; - compUnitToFileIndex[dbg.result] = debugSources[dbg.arg(2)]; + auto srcit = debugSources.find(dbg.arg(2)); + if(srcit != debugSources.end()) + compUnitToFileIndex[dbg.result] = srcit->second; } else if(dbg.inst == ShaderDbg::EntryPoint) { @@ -947,29 +975,28 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st reflection.dispatchThreadsDimension[2] = 0; } + switch(sourceLanguage) + { + case SourceLanguage::ESSL: + case SourceLanguage::GLSL: reflection.debugInfo.encoding = ShaderEncoding::GLSL; break; + case SourceLanguage::HLSL: reflection.debugInfo.encoding = ShaderEncoding::HLSL; break; + case SourceLanguage::Slang: reflection.debugInfo.encoding = ShaderEncoding::Slang; break; + case SourceLanguage::OpenCL_C: + case SourceLanguage::OpenCL_CPP: + case SourceLanguage::CPP_for_OpenCL: + case SourceLanguage::Unknown: + case SourceLanguage::Invalid: + case SourceLanguage::SYCL: + case SourceLanguage::HERO_C: + case SourceLanguage::NZSL: + case SourceLanguage::WGSL: + case SourceLanguage::Zig: + case SourceLanguage::Max: break; + } + for(size_t i = 0; i < sources.size(); i++) { - switch(sources[i].lang) - { - case SourceLanguage::ESSL: - case SourceLanguage::GLSL: reflection.debugInfo.encoding = ShaderEncoding::GLSL; break; - case SourceLanguage::HLSL: reflection.debugInfo.encoding = ShaderEncoding::HLSL; break; - case SourceLanguage::Slang: reflection.debugInfo.encoding = ShaderEncoding::Slang; break; - case SourceLanguage::OpenCL_C: - case SourceLanguage::OpenCL_CPP: - case SourceLanguage::CPP_for_OpenCL: - case SourceLanguage::Unknown: - case SourceLanguage::Invalid: - case SourceLanguage::SYCL: - case SourceLanguage::HERO_C: - case SourceLanguage::NZSL: - case SourceLanguage::WGSL: - case SourceLanguage::Zig: - case SourceLanguage::Max: break; - } - - if(!sources[i].name.empty() || !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}); } switch(m_Generator) diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.h b/renderdoc/driver/shaders/spirv/spirv_reflect.h index 3dcc6927e..b1d2c8d4f 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.h +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.h @@ -83,7 +83,6 @@ namespace rdcspv { struct SourceFile { - SourceLanguage lang; rdcstr name; rdcstr contents; }; @@ -134,6 +133,7 @@ private: rdcstr cmdline; DenseIdMap strings; + SourceLanguage sourceLanguage = SourceLanguage::Unknown; rdcarray sources; SparseIdMap debugSources; SparseIdMap compUnitToFileIndex;