[CodeGen] Fix a crash when updating a zeroinitialize designated initializer

[erik.pilkington]

Summary

LLVM IR, in it's infinite wisdom, doesn't relate ConstantAggregate and ConstantAggregateZero through inheritance, so make sure we handle both cases here. Fixes a crash-on-valid.

rdar://45691981

Thanks for taking a look!
Erik

[rjmccall]

clang/lib/CodeGen/CGExprConstant.cpp

1142 ↗

(On Diff #172266)

Is getAggregateElement not good enough here?

[erik.pilkington]

Use getAggregateElement. Thanks!

clang/lib/CodeGen/CGExprConstant.cpp

1142 ↗

(On Diff #172266)

No, getAggregateElement is perfect, I just didn't know it existed!

[rjmccall]

LGTM.

lib/CodeGen/CGExprConstant.cpp

class ConstStructBuilder {
 
   bool Packed;
   CharUnits NextFieldOffsetInChars;
   CharUnits LLVMStructAlignment;
   SmallVector<llvm::Constant *, 32> Elements;
 public:
   static llvm::Constant *BuildStruct(ConstantEmitter &Emitter,
                                      ConstExprEmitter *ExprEmitter,
-                                     llvm::ConstantStruct *Base,
+                                     llvm::Constant *Base,
                                      InitListExpr *Updater,
                                      QualType ValTy);
   static llvm::Constant *BuildStruct(ConstantEmitter &Emitter,
                                      InitListExpr *ILE, QualType StructTy);
   static llvm::Constant *BuildStruct(ConstantEmitter &Emitter,
                                      const APValue &Value, QualType ValTy);
 
 private:
private:
 
   void AppendPadding(CharUnits PadSize);
 
   void AppendTailPadding(CharUnits RecordSize);
 
   void ConvertStructToPacked();
 
   bool Build(InitListExpr *ILE);
-  bool Build(ConstExprEmitter *Emitter, llvm::ConstantStruct *Base,
+  bool Build(ConstExprEmitter *Emitter, llvm::Constant *Base,
              InitListExpr *Updater);
   bool Build(const APValue &Val, const RecordDecl *RD, bool IsPrimaryBase,
              const CXXRecordDecl *VTableClass, CharUnits BaseOffset);
   llvm::Constant *Finalize(QualType Ty);
 
   CharUnits getAlignment(const llvm::Constant *C) const {
     if (Packed)  return CharUnits::One();
     return CharUnits::fromQuantity(
assert(NextFieldOffsetInChars.alignTo(getAlignment(Result)) ==
              getSizeInChars(Result) &&
          "Size mismatch!");
 
   return Result;
 }
 
 llvm::Constant *ConstStructBuilder::BuildStruct(ConstantEmitter &Emitter,
                                                 ConstExprEmitter *ExprEmitter,
-                                                llvm::ConstantStruct *Base,
+                                                llvm::Constant *Base,
                                                 InitListExpr *Updater,
                                                 QualType ValTy) {
   ConstStructBuilder Builder(Emitter);
   if (!Builder.Build(ExprEmitter, Base, Updater))
     return nullptr;
   return Builder.Finalize(ValTy);
 }
 
if (auto destAT = CGM.getContext().getAsArrayType(destType)) {
                                                         Types, true);
         return llvm::ConstantStruct::get(SType, Elts);
       }
 
       return llvm::ConstantArray::get(AType, Elts);
     }
 
     if (destType->isRecordType())
-      return ConstStructBuilder::BuildStruct(Emitter, this,
-                 dyn_cast<llvm::ConstantStruct>(Base), Updater, destType);
+      return ConstStructBuilder::BuildStruct(Emitter, this, Base, Updater,
+                                             destType);
 
     return nullptr;
   }
 
   llvm::Constant *VisitDesignatedInitUpdateExpr(DesignatedInitUpdateExpr *E,
                                                 QualType destType) {
     auto C = Visit(E->getBase(), destType);
     if (!C) return nullptr;
public:
   llvm::Type *ConvertType(QualType T) {
     return CGM.getTypes().ConvertType(T);
   }
 };
 
 }  // end anonymous namespace.
 
 bool ConstStructBuilder::Build(ConstExprEmitter *ExprEmitter,
-                               llvm::ConstantStruct *Base,
+                               llvm::Constant *Base,
                                InitListExpr *Updater) {
   assert(Base && "base expression should not be empty");
 
   QualType ExprType = Updater->getType();
   RecordDecl *RD = ExprType->getAs<RecordType>()->getDecl();
   const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(RD);
   const llvm::StructLayout *BaseLayout = CGM.getDataLayout().getStructLayout(
-                                           Base->getType());
+      cast<llvm::StructType>(Base->getType()));
   unsigned FieldNo = -1;
   unsigned ElementNo = 0;
 
   // Bail out if we have base classes. We could support these, but they only
   // arise in C++1z where we will have already constant folded most interesting
   // cases. FIXME: There are still a few more cases we can handle this way.
   if (auto *CXXRD = dyn_cast<CXXRecordDecl>(RD))
     if (CXXRD->getNumBases())
       return false;
 
   for (FieldDecl *Field : RD->fields()) {
     ++FieldNo;
 
     if (RD->isUnion() && Updater->getInitializedFieldInUnion() != Field)
       continue;
 
     // Skip anonymous bitfields.
     if (Field->isUnnamedBitfield())
       continue;
 
-    llvm::Constant *EltInit = Base->getOperand(ElementNo);
+    llvm::Constant *EltInit = Base->getAggregateElement(ElementNo);
 
     // Bail out if the type of the ConstantStruct does not have the same layout
     // as the type of the InitListExpr.
     if (CGM.getTypes().ConvertType(Field->getType()) != EltInit->getType() ||
         Layout.getFieldOffset(ElementNo) !=
           BaseLayout->getElementOffsetInBits(ElementNo))
       return false;
 

test/CodeGen/designated-initializers.c

union_16644_t union_16644_instance_4[2] =
   [1].a.zero = 3,
   [0].a.zero = 5,
   [1].b[1] = 4
 };
 
 // CHECK: @lab = global { [4 x i8], i32 } { [4 x i8] undef, i32 123 }
 struct leading_anon_bitfield { int : 32; int n; } lab = { .n = 123 };
 
+// rdar://45691981
+struct Base {
+  struct {
+    int A;
+  };
+};
+struct Derived {
+  struct Base B;
+};
+struct Derived D = {{}, .B.A = 42};
+// CHECK: @D = global %struct.Derived { %struct.Base { %struct.anon.4 { i32 42 } } }, align 4
+
 void test1(int argc, char **argv)
 {
   // CHECK: internal global %struct.foo { i8* null, i32 1024 }
   static struct foo foo = {
     .b = 1024,
   };
 
-  // CHECK: bitcast %union.anon.4* %u2
+  // CHECK: bitcast %union.anon.5* %u2
   // CHECK: call void @llvm.memset
    union { int i; float f; } u2 = { };
 
   // CHECK-NOT: call void @llvm.memset
   union { int i; float f; } u3;
 
   // CHECK: ret void
 }