fix crash from static members of cbuffers

docs/ReleaseNotes.md

@@ -34,6 +34,13 @@ line upon naming the release. Refer to previous for appropriate section names.
   instead of crashing
   [#6661](https://github.com/microsoft/DirectXShaderCompiler/issues/6661).
 
+#### SPIR-V
+
+- Fixed a crash from `static` members of a `cbuffer`/`tbuffer`, which were
+  incorrectly counted as buffer members and shifted the member indices of the
+  members declared after them
+  [#8537](https://github.com/microsoft/DirectXShaderCompiler/issues/8537).
+
 #### Bug Fixes
 
 - Fixed internal compiler errors when a member method is called on a ray payload

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -299,6 +299,14 @@ LocationAndComponent getLocationAndComponentCount(const ASTContext &astContext,
   return {0, 0, false};
 }
 
+/// Returns true if the given decl is a static variable declared inside
+/// a cbuffer/tbuffer/struct. These are not actually part of the
+/// cbuffer/tbuffer/struct, and should not consume a member index.
+bool isStaticBufferDecl(const Decl *decl) {
+  const auto *varDecl = dyn_cast<VarDecl>(decl);
+  return varDecl && varDecl->getStorageClass() == StorageClass::SC_Static;
+}
+
 bool shouldSkipInStructLayout(const Decl *decl) {
   // Ignore implicit generated struct declarations/constructors/destructors
   if (decl->isImplicit())
@@ -1455,7 +1463,7 @@ SpirvVariable *DeclResultIdMapper::createStructOrStructArrayVarOfExplicitLayout(
 
       // Static variables are not part of the struct from a layout perspective.
       // Thus, they should not be listed in the struct fields.
-      if (fieldVar->getStorageClass() == StorageClass::SC_Static) {
+      if (isStaticBufferDecl(fieldVar)) {
         continue;
       }
 
@@ -1558,8 +1566,11 @@ void DeclResultIdMapper::createCTBuffer(const HLSLBufferDecl *decl) {
     if (shouldSkipInStructLayout(subDecl))
       continue;
 
-    // If the subDecl is a resource, it is lowered as a standalone variable.
     const auto *varDecl = cast<VarDecl>(subDecl);
+
+    if (isStaticBufferDecl(varDecl))
+      continue;
+
     if (isResourceType(varDecl->getType())) {
       createExternVar(varDecl);
       continue;
@@ -1737,6 +1748,9 @@ DeclResultIdMapper::createShaderRecordBuffer(const HLSLBufferDecl *decl,
     if (isResourceType(varDecl->getType()))
       continue;
 
+    if (isStaticBufferDecl(varDecl))
+      continue;
+
     registerVariableForDecl(varDecl, createDeclSpirvInfo(bufferVar, index++));
   }
   return bufferVar;

tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -1941,6 +1941,9 @@ void SpirvEmitter::doHLSLBufferDecl(const HLSLBufferDecl *bufferDecl) {
   // supported in Vulkan
   for (const auto *member : bufferDecl->decls()) {
     if (const auto *varMember = dyn_cast<VarDecl>(member)) {
+      if (varMember->getStorageClass() == StorageClass::SC_Static)
+        continue;
+
       if (!spirvOptions.noWarnIgnoredFeatures) {
         if (const auto *init = varMember->getInit())
           emitWarning("%select{tbuffer|cbuffer}0 member initializer "
@@ -1970,6 +1973,12 @@ void SpirvEmitter::doHLSLBufferDecl(const HLSLBufferDecl *bufferDecl) {
   } else {
     declIdMapper.createCTBuffer(bufferDecl);
   }
+
+  for (const auto *member : bufferDecl->decls()) {
+    const auto *varMember = dyn_cast<VarDecl>(member);
+    if (varMember && varMember->getStorageClass() == StorageClass::SC_Static)
+      doVarDecl(varMember);
+  }
 }
 
 void SpirvEmitter::doClassTemplateDecl(
@@ -2165,9 +2174,11 @@ void SpirvEmitter::doVarDecl(const VarDecl *decl) {
   // ConstantBuffers and TextureBuffers are not HLSLBufferDecls.
   if (const auto *bufferDecl =
           dyn_cast<HLSLBufferDecl>(decl->getDeclContext())) {
-    // This is a VarDecl of cbuffer/tbuffer type.
-    doHLSLBufferDecl(bufferDecl);
-    return;
+    if (decl->getStorageClass() != StorageClass::SC_Static) {
+      // This is a VarDecl of cbuffer/tbuffer type.
+      doHLSLBufferDecl(bufferDecl);
+      return;
+    }
   }
 
   if (decl->getAttr<VKInputAttachmentIndexAttr>()) {

tools/clang/test/CodeGenSPIRV/cbuffer.static.member.hlsl

@@ -0,0 +1,34 @@
+// RUN: %dxc -T vs_6_0 -E main -fcgl  %s -spirv | FileCheck %s
+
+// A `static` variable declared inside a cbuffer is an ordinary global variable,
+// not a member of the buffer. It must be left out of the buffer's struct (it is
+// not part of the layout) *and* it must not consume a member index, otherwise
+// the access chains for the members declared after it are shifted, and the last
+// one ends up out of bounds.
+
+// CHECK:     OpMemberName %type_MyCBuffer 0 "a"
+// CHECK:     OpMemberName %type_MyCBuffer 1 "b"
+// CHECK:     OpMemberName %type_MyCBuffer 2 "c"
+// CHECK-NOT: OpMemberName %type_MyCBuffer 3
+
+// CHECK-DAG: %uint_10 = OpConstant %uint 10
+// CHECK-DAG: %uint_11 = OpConstant %uint 11
+// CHECK-DAG: %uint_12 = OpConstant %uint 12
+
+// CHECK: %type_MyCBuffer = OpTypeStruct %uint %uint %uint
+
+cbuffer MyCBuffer {
+  uint a;
+  static const uint a_mode = 10;
+  uint b;
+  static const uint b_mode = 11;
+  uint c;
+  static const uint c_mode = 12;
+};
+
+float4 main() : SV_Position {
+// CHECK: OpAccessChain %_ptr_Uniform_uint %MyCBuffer %int_0
+// CHECK: OpAccessChain %_ptr_Uniform_uint %MyCBuffer %int_1
+// CHECK: OpAccessChain %_ptr_Uniform_uint %MyCBuffer %int_2
+  return float4(a + b + c, a_mode, b_mode, c_mode);
+}

tools/clang/test/CodeGenSPIRV/vk.shader-record-ext.static.member.hlsl

@@ -0,0 +1,41 @@
+// RUN: %dxc -T lib_6_3 -fspv-target-env=vulkan1.2 -fcgl  %s -spirv | FileCheck %s
+
+// Same as cbuffer.static.member.hlsl, but for a shader record buffer, which
+// goes through createShaderRecordBuffer() instead of createCTBuffer(). A
+// `static` variable declared inside the buffer is an ordinary global variable,
+// not a member of the buffer. It must be left out of the buffer's struct (it is
+// not part of the layout) *and* it must not consume a member index, otherwise
+// the access chains for the members declared after it are shifted, and the last
+// one ends up out of bounds.
+
+// CHECK:     OpMemberName %type_ShaderRecordBufferKHR_block 0 "a"
+// CHECK:     OpMemberName %type_ShaderRecordBufferKHR_block 1 "b"
+// CHECK:     OpMemberName %type_ShaderRecordBufferKHR_block 2 "c"
+// CHECK-NOT: OpMemberName %type_ShaderRecordBufferKHR_block 3
+
+// CHECK-DAG: %uint_10 = OpConstant %uint 10
+// CHECK-DAG: %uint_11 = OpConstant %uint 11
+// CHECK-DAG: %uint_12 = OpConstant %uint 12
+
+// CHECK: %type_ShaderRecordBufferKHR_block = OpTypeStruct %uint %uint %uint
+
+[[vk::shader_record_ext]]
+cbuffer block {
+  uint a;
+  static const uint a_mode = 10;
+  uint b;
+  static const uint b_mode = 11;
+  uint c;
+  static const uint c_mode = 12;
+}
+
+struct Payload { float p; };
+struct Attr    { float a; };
+
+[shader("closesthit")]
+void main(inout Payload P, in Attr A) {
+// CHECK: OpAccessChain %_ptr_ShaderRecordBufferKHR_uint %block %int_0
+// CHECK: OpAccessChain %_ptr_ShaderRecordBufferKHR_uint %block %int_1
+// CHECK: OpAccessChain %_ptr_ShaderRecordBufferKHR_uint %block %int_2
+  P.p = a + b + c + a_mode + b_mode + c_mode;
+}