[SPIR-V] Fixed a crash if encounter constant buffer fields with overlapping register assignments (microsoft#7636)

The issue:
simple vertex shader like so
```
uniform float4x4 gMVP : register(c0);
uniform float4   gFoo : register(c5);
uniform float4   gBar : register(c5);

float4 main(float4 pos : POSITION) : SV_Position {
    return mul(gMVP, pos * gFoo + gBar);
}
```
will result in an internal crash
```
dxc.exe -spirv -T vs_6_2 -E main test.hlsl -Fo test.spirv
Internal compiler error: access violation. Attempted to read from address 0x0000000000000000
```

Due to `LowerTypeVisitor` trying to assign offsets to fields without
explicit locations.
It'll sort fields first, which will fill the map with the fields first.
And since it's using `std::map` - if there's fields with the same
`register` number - it'll only insert first, other will be left out,
resulting nullptrs in the output vector.
We read the content of the vector down the road crashing.

My change fixes the crash and tries to output somewhat useful info about
compilation fail.

I hope this helps you in fixing it properly, or you can take it as it
is.

Author: iOrange

tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

@@ -37,33 +37,6 @@ inline uint32_t roundToPow2(uint32_t val, uint32_t pow2) {
 
 } // end anonymous namespace
 
-// This method sorts a field list in the following order:
-//  - fields with register annotation first, sorted by register index.
-//  - then fields without annotation, in order of declaration.
-static std::vector<const HybridStructType::FieldInfo *>
-sortFields(llvm::ArrayRef<HybridStructType::FieldInfo> fields) {
-  std::vector<const HybridStructType::FieldInfo *> output;
-  output.resize(fields.size());
-
-  auto back_inserter = output.rbegin();
-  std::map<uint32_t, const HybridStructType::FieldInfo *> fixed_fields;
-  for (auto it = fields.rbegin(); it < fields.rend(); it++) {
-    if (it->registerC) {
-      fixed_fields.insert({it->registerC->RegisterNumber, &*it});
-    } else {
-      *back_inserter = &*it;
-      back_inserter++;
-    }
-  }
-
-  auto front_inserter = output.begin();
-  for (const auto &item : fixed_fields) {
-    *front_inserter = item.second;
-    front_inserter++;
-  }
-  return output;
-}
-
 static void setDefaultFieldSize(const AlignmentSizeCalculator &alignmentCalc,
                                 const SpirvLayoutRule rule,
                                 const HybridStructType::FieldInfo *currentField,
@@ -292,6 +265,37 @@ bool LowerTypeVisitor::visitInstruction(SpirvInstruction *instr) {
   return true;
 }
 
+std::vector<const HybridStructType::FieldInfo *> LowerTypeVisitor::sortFields(
+    llvm::ArrayRef<HybridStructType::FieldInfo> fields) {
+  std::vector<const HybridStructType::FieldInfo *> output;
+  output.resize(fields.size());
+
+  auto back_inserter = output.rbegin();
+  std::map<uint32_t, const HybridStructType::FieldInfo *> fixed_fields;
+  for (auto it = fields.rbegin(); it < fields.rend(); it++) {
+    if (it->registerC) {
+      auto insertionResult =
+          fixed_fields.insert({it->registerC->RegisterNumber, &*it});
+      if (!insertionResult.second) {
+        emitError(
+            "field \"%0\" at register(c%1) overlaps with previous members",
+            it->registerC->Loc)
+            << it->name << it->registerC->RegisterNumber;
+      }
+    } else {
+      *back_inserter = &*it;
+      back_inserter++;
+    }
+  }
+
+  auto front_inserter = output.begin();
+  for (const auto &item : fixed_fields) {
+    *front_inserter = item.second;
+    front_inserter++;
+  }
+  return output;
+}
+
 const SpirvType *LowerTypeVisitor::lowerType(const SpirvType *type,
                                              SpirvLayoutRule rule,
                                              SourceLocation loc) {
@@ -1378,12 +1382,19 @@ LowerTypeVisitor::populateLayoutInformation(
   llvm::SmallVector<StructType::FieldInfo, 4> loweredFields;
   llvm::DenseMap<const HybridStructType::FieldInfo *, uint32_t> fieldToIndexMap;
 
+  llvm::SmallVector<StructType::FieldInfo, 4> result;
+
   // This stores the index of the field in the actual SPIR-V construct.
   // When bitfields are merged, this index will be the same for merged fields.
   uint32_t fieldIndexInConstruct = 0;
   for (size_t i = 0, iPrevious = -1; i < sortedFields.size(); iPrevious = i++) {
     const size_t fieldIndexForMap = loweredFields.size();
 
+    // Can happen if sortFields runs over fields with the same register(c#)
+    if (!sortedFields[i]) {
+      return result;
+    }
+
     loweredFields.emplace_back(fieldVisitor(
         (iPrevious < loweredFields.size() ? &loweredFields[iPrevious]
                                           : nullptr),
@@ -1397,7 +1408,6 @@ LowerTypeVisitor::populateLayoutInformation(
   }
 
   // Re-order the sorted fields back to their original order.
-  llvm::SmallVector<StructType::FieldInfo, 4> result;
   for (const auto &field : fields)
     result.push_back(loweredFields[fieldToIndexMap[&field]]);
   return result;

tools/clang/lib/SPIRV/LowerTypeVisitor.h

@@ -62,6 +62,12 @@ class LowerTypeVisitor : public Visitor {
     return astContext.getDiagnostics().Report(srcLoc, diagId);
   }
 
+  // This method sorts a field list in the following order:
+  //  - fields with register annotation first, sorted by register index.
+  //  - then fields without annotation, in order of declaration.
+  std::vector<const HybridStructType::FieldInfo *>
+  sortFields(llvm::ArrayRef<HybridStructType::FieldInfo> fields);
+
   /// Lowers the given Hybrid type into a SPIR-V type.
   ///
   /// Uses the above lowerType method to lower the QualType components of hybrid

tools/clang/test/CodeGenSPIRV/cbuffer.overlap.hlsl

@@ -0,0 +1,11 @@
+// RUN: not %dxc -T vs_6_2 -E main -fcgl  %s -spirv  2>&1 | FileCheck %s
+
+// CHECK: error: field "gFoo" at register(c5) overlaps with previous members
+
+uniform float4x4 gMVP : register(c0);
+uniform float4   gFoo : register(c5);
+uniform float4   gBar : register(c5);
+
+float4 main(float4 pos : POSITION) : SV_Position {
+    return mul(gMVP, pos * gFoo + gBar);
+}