KhronosGroup / KhronosGroup/glslang
Wrong `NonSemantic.Shader.DebugInfo` source location for included struct
- Dominant language
- C++
- Stars
- 3.6k
- Forks
- 989
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 31
Description
Hi,
When a struct is defined in an included file and used in a push constant range (or presumably any other block), the `DebugTypeMember` instructions for that struct's members reference the correct lines and columns from the included file, but the wrong `DebugSource` instruction. The `DebugSource` instruction used is the one which references the file where the push constant range is defined (instead of where the struct is defined).
The `DebugTypeComposite` instruction for the struct itself references the source location of the push constant range (both file and line).
This issue manifests itself later as SPIR-V validation errors, e.g.
```
error: line 55: NonSemantic.Shader.DebugInfo.100 DebugTypeMember: operand Column End (11) is larger then Line 2 column length of 1 found in the DebugSource text
%32 = OpExtInst %void %1 DebugTypeMember %33 %30 %19 %uint_2 %uint_11 %uint_0 %uint_0 %uint_3
```
Here's a simple example:
`my_struct.glsl`:
```glsl
struct MyStruct {
float x;
};
```
`shader.comp`:
```glsl
#version 460
#extension GL_GOOGLE_include_directive : require
#include "my_struct.glsl"
layout(push_constant, std430) uniform PushConstRange {
MyStruct s;
};
void main() {}
```
```shell
glslang -o shader.comp.spv -gVS -Od --target-env vulkan1.3 shader.comp
spirv-val shader.comp.spv # fails
```
```llvm
%2 = OpString "shader.comp"
%3 = OpString "./my_struct.glsl"
%29 = OpString "float"
%33 = OpString "x"
%35 = OpString "MyStruct"
%19 = OpExtInst %void %1 DebugSource %2 %20
%30 = OpExtInst %void %1 DebugTypeBasic %29 %uint_32 %uint_3 %uint_0
%32 = OpExtInst %void %1 DebugTypeMember %33 %30 %19 %uint_2 %uint_11 %uint_0 %uint_0 %uint_3
%34 = OpExtInst %void %1 DebugTypeComposite %35 %uint_1 %19 %uint_7 %uint_0 %22 %35 %uint_0 %uint_3 %32
```
The issue is similar to #3862, but that deals with built-in variables, not included files. The same workaround works though (just zero out `Line` and `Column` in the `DebugTypeMember` instruction).
A better workaround I found for the `DebugTypeMember` issue is to simply pass the correct source ID as part of `StructMemberDebugInfo`:
```diff
diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp
@@ -1300,7 +1300,7 @@
type->addIdOperand(getStringId(debugTypeLoc.name)); // name id
type->addIdOperand(debugTypeLoc.debugTypeOverride != 0 ? debugTypeLoc.debugTypeOverride
: getDebugType(memberType)); // type id
- type->addIdOperand(makeDebugSource(currentFileId)); // source id
+ type->addIdOperand(makeDebugSource(debugTypeLoc.sourceIdOverride ? debugTypeLoc.sourceIdOverride : currentFileId)); // source id
type->addIdOperand(makeUintConstant(debugTypeLoc.line)); // line id TODO: currentLine is always zero
type->addIdOperand(makeUintConstant(debugTypeLoc.column)); // TODO: column id
type->addIdOperand(makeUintConstant(0)); // TODO: offset id
diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp
@@ -6412,6 +6412,9 @@
debugInfo.name = glslangMember.type->getFieldName();
debugInfo.line = glslangMember.loc.line;
debugInfo.column = glslangMember.loc.column;
+ if (const char *const filename = glslangMember.loc.getFilename()) {
+ debugInfo.sourceIdOverride = builder.getStringId(filename);
+ }
// Per the GLSL spec, bool variables inside of a uniform or buffer block are generated as uint.
// But for debug info, we want to represent them as bool because that is the original type in
diff --git a/SPIRV/SpvBuilder.h b/SPIRV/SpvBuilder.h
@@ -88,6 +88,7 @@
int column {0};
// Set if the caller knows a better debug type than what is associated with the functional SPIR-V type.
spv::Id debugTypeOverride {0};
+ spv::Id sourceIdOverride {0};
};
class Builder {
```
Of course neither workaround fixes the `DebugTypeComposite` instruction.
Contributor guide
Research direction
Reproduce the issue with the shown glslang and spirv-val commands, then inspect SPIRV/SpvBuilder.cpp, SPIRV/GlslangToSpv.cpp, and SPIRV/SpvBuilder.h around StructMemberDebugInfo and DebugTypeComposite generation. Ensure included-struct members and the composite use the included file's source location, then confirm the generated SPIR-V passes spirv-val.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100