[clang] [HLSL] Fix miscompile of small structs after arrays in cbuffers (PR #179768)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-hlsl

Author: Justin Bogner (bogner)

<details>
<summary>Changes</summary>

We weren't accounting for skipped fields correctly when emitting struct member exprs, which could lead to us reading padding instead of the member itself when a struct followed an array.

Fixes #<!-- -->179716

---
Full diff: https://github.com/llvm/llvm-project/pull/179768.diff


2 Files Affected:

- (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+16-3) 
- (modified) clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl (+3-3) 


``````````diff
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index cff2824d29dc5..985bc8640118e 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -1556,10 +1556,7 @@ LValue CGHLSLRuntime::emitBufferMemberExpr(CodeGenFunction &CGF,
   auto *Field = dyn_cast<FieldDecl>(E->getMemberDecl());
   assert(Field && "Unexpected access into HLSL buffer");
 
-  // Get the field index for the struct.
   const RecordDecl *Rec = Field->getParent();
-  unsigned FieldIdx =
-      CGM.getTypes().getCGRecordLayout(Rec).getLLVMFieldNo(Field);
 
   // Work out the buffer layout type to index into.
   QualType RecType = CGM.getContext().getCanonicalTagType(Rec);
@@ -1571,6 +1568,22 @@ LValue CGHLSLRuntime::emitBufferMemberExpr(CodeGenFunction &CGF,
   llvm::StructType *LayoutTy = HLSLBufferLayoutBuilder(CGM).layOutStruct(
       RecType->getAsCanonical<RecordType>(), EmptyOffsets);
 
+  // Get the field index for the layout struct, accounting for padding.
+  unsigned FieldIdx =
+      CGM.getTypes().getCGRecordLayout(Rec).getLLVMFieldNo(Field);
+  assert(FieldIdx < LayoutTy->getNumElements() &&
+         "Layout struct is smaller than member struct");
+  unsigned Skipped = 0;
+  for (unsigned I = 0; I <= FieldIdx;) {
+    llvm::Type *ElementTy = LayoutTy->getElementType(I + Skipped);
+    if (CGF.CGM.getTargetCodeGenInfo().isHLSLPadding(ElementTy))
+      ++Skipped;
+    else
+      ++I;
+  }
+  FieldIdx += Skipped;
+  assert(FieldIdx < LayoutTy->getNumElements() && "Access out of bounds");
+
   // Now index into the struct, making sure that the type we return is the
   // buffer layout type rather than the original type in the AST.
   QualType FieldType = Field->getType();
diff --git a/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl b/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl
index 7a0b45875faf9..657694aa90fd7 100644
--- a/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl
+++ b/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl
@@ -73,7 +73,7 @@ void cbstructs() {
   use(s1.a1);
   // CHECK: load <3 x i16>, ptr addrspace(2) getelementptr inbounds nuw (%B, ptr addrspace(2) @s2, i32 0, i32 1), align 2
   use(s2.b1);
-  // CHECK: load <2 x float>, ptr addrspace(2) getelementptr inbounds nuw (%C, ptr addrspace(2) @s3, i32 0, i32 1), align 8
+  // CHECK: load <2 x float>, ptr addrspace(2) getelementptr inbounds nuw (%C, ptr addrspace(2) @s3, i32 0, i32 2), align 8
   use(s3.c2.a1);
   // CHECK: load <2 x float>, ptr addrspace(2) getelementptr (<{ %A, target("dx.Padding", 8) }>, ptr addrspace(2) @s4, i32 2, i32 0), align 8
   use(s4[2].a1);
@@ -108,9 +108,9 @@ void cbmix() {
   use(m3.y);
   // CHECK: load <2 x float>, ptr addrspace(2) getelementptr (<{ <2 x float>, target("dx.Padding", 8) }>, ptr addrspace(2) getelementptr (<{ <{ [3 x <{ <2 x float>, target("dx.Padding", 8) }>], <2 x float> }>, target("dx.Padding", 8) }>, ptr addrspace(2) @m4, i32 2, i32 0), i32 3, i32 0), align 16
   use(m4[2][3]);
-  // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_1]], ptr addrspace(2) @m5, i32 0, i32 1), align 16
+  // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_1]], ptr addrspace(2) @m5, i32 0, i32 2), align 16
   use(m5.d);
-  // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds ([5 x <4 x i32>], ptr addrspace(2) getelementptr (<{ %ArrayAndScalar, target("dx.Padding", 12) }>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_2]], ptr addrspace(2) @m6, i32 0, i32 1), i32 2, i32 0), i32 0, i32 2), align 16
+  // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds ([5 x <4 x i32>], ptr addrspace(2) getelementptr (<{ %ArrayAndScalar, target("dx.Padding", 12) }>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_2]], ptr addrspace(2) @m6, i32 0, i32 2), i32 2, i32 0), i32 0, i32 2), align 16
   use(m6.j[2].x[2]);
   // CHECK: load <1 x double>, ptr addrspace(2) @m7, align 8
   use(m7);

``````````

</details>


https://github.com/llvm/llvm-project/pull/179768