Revert "[clang][CodeGen] Zero init unspecified fields in initializers in C" (#109898)

Reverts llvm/llvm-project#97121

Causing failures on LNT bots; log shows a crash in
ConstStructBuilder::BuildStruct.

3 files changed, 13 insertions, 171 deletions

diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 43f3bcc..bbfc667 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -1698,17 +1698,6 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
   // Prepare a 'this' for CXXDefaultInitExprs.
   CodeGenFunction::FieldConstructionScope FCS(CGF, Dest.getAddress());
 
-  const bool ZeroInitPadding =
-      CGF.CGM.shouldZeroInitPadding() && !Dest.isZeroed();
-  const Address BaseLoc = Dest.getAddress().withElementType(CGF.Int8Ty);
-  auto DoZeroInitPadding = [&](CharUnits Offset, CharUnits Size) {
-    if (Size.isPositive()) {
-      Address Loc = CGF.Builder.CreateConstGEP(BaseLoc, Offset.getQuantity());
-      llvm::Constant *SizeVal = CGF.Builder.getInt64(Size.getQuantity());
-      CGF.Builder.CreateMemSet(Loc, CGF.Builder.getInt8(0), SizeVal, false);
-    }
-  };
-
   if (record->isUnion()) {
     // Only initialize one field of a union. The field itself is
     // specified by the initializer list.
@@ -1733,37 +1722,17 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
     if (NumInitElements) {
       // Store the initializer into the field
       EmitInitializationToLValue(InitExprs[0], FieldLoc);
-      if (ZeroInitPadding) {
-        CharUnits TotalSize =
-            Dest.getPreferredSize(CGF.getContext(), DestLV.getType());
-        CharUnits FieldSize =
-            CGF.getContext().getTypeSizeInChars(FieldLoc.getType());
-        DoZeroInitPadding(FieldSize, TotalSize - FieldSize);
-      }
     } else {
       // Default-initialize to null.
-      if (ZeroInitPadding)
-        EmitNullInitializationToLValue(DestLV);
-      else
-        EmitNullInitializationToLValue(FieldLoc);
+      EmitNullInitializationToLValue(FieldLoc);
     }
+
     return;
   }
 
   // Here we iterate over the fields; this makes it simpler to both
   // default-initialize fields and skip over unnamed fields.
-  const ASTRecordLayout &Layout = CGF.getContext().getASTRecordLayout(record);
-  CharUnits SizeSoFar = CharUnits::Zero();
   for (const auto *field : record->fields()) {
-    if (ZeroInitPadding) {
-      unsigned FieldNo = field->getFieldIndex();
-      CharUnits Offset =
-          CGF.getContext().toCharUnitsFromBits(Layout.getFieldOffset(FieldNo));
-      DoZeroInitPadding(SizeSoFar, Offset - SizeSoFar);
-      CharUnits FieldSize =
-          CGF.getContext().getTypeSizeInChars(field->getType());
-      SizeSoFar = Offset + FieldSize;
-    }
     // We're done once we hit the flexible array member.
     if (field->getType()->isIncompleteArrayType())
       break;
@@ -1805,11 +1774,6 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
       }
     }
   }
-  if (ZeroInitPadding) {
-    CharUnits TotalSize =
-        Dest.getPreferredSize(CGF.getContext(), DestLV.getType());
-    DoZeroInitPadding(SizeSoFar, TotalSize - SizeSoFar);
-  }
 }
 
 void AggExprEmitter::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E,
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 66bc064..dd65080 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -42,16 +42,6 @@ using namespace CodeGen;
 namespace {
 class ConstExprEmitter;
 
-llvm::Constant *getPadding(const CodeGenModule &CGM, CharUnits PadSize) {
-  llvm::Type *Ty = CGM.CharTy;
-  if (PadSize > CharUnits::One())
-    Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
-  if (CGM.shouldZeroInitPadding()) {
-    return llvm::Constant::getNullValue(Ty);
-  }
-  return llvm::UndefValue::get(Ty);
-}
-
 struct ConstantAggregateBuilderUtils {
   CodeGenModule &CGM;
 
@@ -71,7 +61,10 @@ struct ConstantAggregateBuilderUtils {
   }
 
   llvm::Constant *getPadding(CharUnits PadSize) const {
-    return ::getPadding(CGM, PadSize);
+    llvm::Type *Ty = CGM.CharTy;
+    if (PadSize > CharUnits::One())
+      Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+    return llvm::UndefValue::get(Ty);
   }
 
   llvm::Constant *getZeroes(CharUnits ZeroSize) const {
@@ -598,11 +591,6 @@ private:
   bool Build(const InitListExpr *ILE, bool AllowOverwrite);
   bool Build(const APValue &Val, const RecordDecl *RD, bool IsPrimaryBase,
              const CXXRecordDecl *VTableClass, CharUnits BaseOffset);
-  bool DoZeroInitPadding(const ASTRecordLayout &Layout, unsigned FieldNo,
-                         const FieldDecl &Field, bool AllowOverwrite,
-                         CharUnits &FieldSize, CharUnits &SizeSoFar);
-  bool DoZeroInitPadding(const ASTRecordLayout &Layout, bool AllowOverwrite,
-                         CharUnits &SizeSoFar);
   llvm::Constant *Finalize(QualType Ty);
 };
 
@@ -727,10 +715,6 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) {
     if (CXXRD->getNumBases())
       return false;
 
-  const bool ZeroInitPadding = CGM.shouldZeroInitPadding();
-  CharUnits FieldSize = CharUnits::Zero();
-  CharUnits SizeSoFar = CharUnits::Zero();
-
   for (FieldDecl *Field : RD->fields()) {
     ++FieldNo;
 
@@ -748,13 +732,8 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) {
     const Expr *Init = nullptr;
     if (ElementNo < ILE->getNumInits())
       Init = ILE->getInit(ElementNo++);
-    if (isa_and_nonnull<NoInitExpr>(Init)) {
-      if (ZeroInitPadding &&
-          !DoZeroInitPadding(Layout, FieldNo, *Field, AllowOverwrite, FieldSize,
-                             SizeSoFar))
-        return false;
+    if (isa_and_nonnull<NoInitExpr>(Init))
       continue;
-    }
 
     // Zero-sized fields are not emitted, but their initializers may still
     // prevent emission of this struct as a constant.
@@ -764,11 +743,6 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) {
       continue;
     }
 
-    if (ZeroInitPadding &&
-        !DoZeroInitPadding(Layout, FieldNo, *Field, AllowOverwrite, FieldSize,
-                           SizeSoFar))
-      return false;
-
     // When emitting a DesignatedInitUpdateExpr, a nested InitListExpr
     // represents additional overwriting of our current constant value, and not
     // a new constant to emit independently.
@@ -794,10 +768,6 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) {
     if (!EltInit)
       return false;
 
-    if (ZeroInitPadding && FieldSize.isZero())
-      SizeSoFar += CharUnits::fromQuantity(
-          CGM.getDataLayout().getTypeAllocSize(EltInit->getType()));
-
     if (!Field->isBitField()) {
       // Handle non-bitfield members.
       if (!AppendField(Field, Layout.getFieldOffset(FieldNo), EltInit,
@@ -815,9 +785,6 @@ bool ConstStructBuilder::Build(const InitListExpr *ILE, bool AllowOverwrite) {
     }
   }
 
-  if (ZeroInitPadding && !DoZeroInitPadding(Layout, AllowOverwrite, SizeSoFar))
-    return false;
-
   return true;
 }
 
@@ -882,9 +849,6 @@ bool ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD,
 
   unsigned FieldNo = 0;
   uint64_t OffsetBits = CGM.getContext().toBits(Offset);
-  const bool ZeroInitPadding = CGM.shouldZeroInitPadding();
-  CharUnits FieldSize = CharUnits::Zero();
-  CharUnits SizeSoFar = CharUnits::Zero();
 
   bool AllowOverwrite = false;
   for (RecordDecl::field_iterator Field = RD->field_begin(),
@@ -906,15 +870,6 @@ bool ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD,
     if (!EltInit)
       return false;
 
-    if (ZeroInitPadding) {
-      if (!DoZeroInitPadding(Layout, FieldNo, **Field, AllowOverwrite,
-                             FieldSize, SizeSoFar))
-        return false;
-      if (FieldSize.isZero())
-        SizeSoFar += CharUnits::fromQuantity(
-            CGM.getDataLayout().getTypeAllocSize(EltInit->getType()));
-    }
-
     if (!Field->isBitField()) {
       // Handle non-bitfield members.
       if (!AppendField(*Field, Layout.getFieldOffset(FieldNo) + OffsetBits,
@@ -931,35 +886,7 @@ bool ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD,
         return false;
     }
   }
-  if (ZeroInitPadding && !DoZeroInitPadding(Layout, AllowOverwrite, SizeSoFar))
-    return false;
-
-  return true;
-}
 
-bool ConstStructBuilder::DoZeroInitPadding(
-    const ASTRecordLayout &Layout, unsigned FieldNo, const FieldDecl &Field,
-    bool AllowOverwrite, CharUnits &FieldSize, CharUnits &SizeSoFar) {
-  CharUnits Offset =
-      CGM.getContext().toCharUnitsFromBits(Layout.getFieldOffset(FieldNo));
-  if (SizeSoFar < Offset)
-    if (!AppendBytes(SizeSoFar, getPadding(CGM, Offset - SizeSoFar),
-                     AllowOverwrite))
-      return false;
-  FieldSize = CGM.getContext().getTypeSizeInChars(Field.getType());
-  SizeSoFar = Offset + FieldSize;
-  return true;
-}
-
-bool ConstStructBuilder::DoZeroInitPadding(const ASTRecordLayout &Layout,
-                                           bool AllowOverwrite,
-                                           CharUnits &SizeSoFar) {
-  CharUnits TotalSize = Layout.getSize();
-  if (SizeSoFar < TotalSize)
-    if (!AppendBytes(SizeSoFar, getPadding(CGM, TotalSize - SizeSoFar),
-                     AllowOverwrite))
-      return false;
-  SizeSoFar = TotalSize;
   return true;
 }
 
@@ -1200,10 +1127,12 @@ public:
 
       assert(CurSize <= TotalSize && "Union size mismatch!");
       if (unsigned NumPadBytes = TotalSize - CurSize) {
-        llvm::Constant *Padding =
-            getPadding(CGM, CharUnits::fromQuantity(NumPadBytes));
-        Elts.push_back(Padding);
-        Types.push_back(Padding->getType());
+        llvm::Type *Ty = CGM.CharTy;
+        if (NumPadBytes > 1)
+          Ty = llvm::ArrayType::get(Ty, NumPadBytes);
+
+        Elts.push_back(llvm::UndefValue::get(Ty));
+        Types.push_back(Ty);
       }
 
       llvm::StructType *STy = llvm::StructType::get(VMContext, Types, false);
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index fcdfef0..c58bb88 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1676,57 +1676,6 @@ public:
     MustTailCallUndefinedGlobals.insert(Global);
   }
 
-  bool shouldZeroInitPadding() const {
-    // In C23 (N3096) $6.7.10:
-    // """
-    // If any object is initialized with an empty iniitializer, then it is
-    // subject to default initialization:
-    //  - if it is an aggregate, every member is initialized (recursively)
-    //  according to these rules, and any padding is initialized to zero bits;
-    //  - if it is a union, the first named member is initialized (recursively)
-    //  according to these rules, and any padding is initialized to zero bits.
-    //
-    // If the aggregate or union contains elements or members that are
-    // aggregates or unions, these rules apply recursively to the subaggregates
-    // or contained unions.
-    //
-    // If there are fewer initializers in a brace-enclosed list than there are
-    // elements or members of an aggregate, or fewer characters in a string
-    // literal used to initialize an array of known size than there are elements
-    // in the array, the remainder of the aggregate is subject to default
-    // initialization.
-    // """
-    //
-    // From my understanding, the standard is ambiguous in the following two
-    // areas:
-    // 1. For a union type with empty initializer, if the first named member is
-    // not the largest member, then the bytes comes after the first named member
-    // but before padding are left unspecified. An example is:
-    //    union U { int a; long long b;};
-    //    union U u = {};  // The first 4 bytes are 0, but 4-8 bytes are left
-    //    unspecified.
-    //
-    // 2. It only mentions padding for empty initializer, but doesn't mention
-    // padding for a non empty initialization list. And if the aggregation or
-    // union contains elements or members that are aggregates or unions, and
-    // some are non empty initializers, while others are empty initiailizers,
-    // the padding initialization is unclear. An example is:
-    //    struct S1 { int a; long long b; };
-    //    struct S2 { char c; struct S1 s1; };
-    //    // The values for paddings between s2.c and s2.s1.a, between s2.s1.a
-    //    and s2.s1.b are unclear.
-    //    struct S2 s2 = { 'c' };
-    //
-    // Here we choose to zero initiailize left bytes of a union type. Because
-    // projects like the Linux kernel are relying on this behavior. If we don't
-    // explicitly zero initialize them, the undef values can be optimized to
-    // return gabage data. We also choose to zero initialize paddings for
-    // aggregates and unions, no matter they are initialized by empty
-    // initializers or non empty initializers. This can provide a consistent
-    // behavior. So projects like the Linux kernel can rely on it.
-    return !getLangOpts().CPlusPlus;
-  }
-
 private:
   bool shouldDropDLLAttribute(const Decl *D, const llvm::GlobalValue *GV) const;