Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

efriedma-quic
Copy link
Collaborator

Reverts #97121

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

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen labels Sep 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2024

@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)

Changes

Reverts llvm/llvm-project#97121

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


Patch is 56.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109898.diff

23 Files Affected:

  • (modified) clang/docs/LanguageExtensions.rst (-23)
  • (modified) clang/lib/CodeGen/CGExprAgg.cpp (+2-38)
  • (modified) clang/lib/CodeGen/CGExprConstant.cpp (+11-82)
  • (modified) clang/lib/CodeGen/CodeGenModule.h (-51)
  • (modified) clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c (+1-1)
  • (modified) clang/test/CodeGen/2008-08-07-AlignPadding1.c (+2-2)
  • (modified) clang/test/CodeGen/2009-06-14-anonymous-union-init.c (+2-2)
  • (modified) clang/test/CodeGen/64bit-swiftcall.c (+7-5)
  • (modified) clang/test/CodeGen/arm-swiftcall.c (+2-2)
  • (modified) clang/test/CodeGen/const-init.c (+2-2)
  • (modified) clang/test/CodeGen/decl.c (+2-2)
  • (modified) clang/test/CodeGen/designated-initializers.c (+6-6)
  • (modified) clang/test/CodeGen/ext-int.c (+9-9)
  • (modified) clang/test/CodeGen/flexible-array-init.c (+12-12)
  • (modified) clang/test/CodeGen/global-init.c (+1-1)
  • (modified) clang/test/CodeGen/init.c (+19)
  • (removed) clang/test/CodeGen/linux-kernel-struct-union-initializer.c (-267)
  • (removed) clang/test/CodeGen/linux-kernel-struct-union-initializer2.c (-140)
  • (modified) clang/test/CodeGen/mingw-long-double.c (+6-3)
  • (modified) clang/test/CodeGen/mms-bitfields.c (+2-2)
  • (modified) clang/test/CodeGen/union-init2.c (+2-2)
  • (modified) clang/test/CodeGen/windows-swiftcall.c (+7-5)
  • (modified) clang/test/CodeGenObjC/designated-initializers.m (+1-1)
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index f4be97047422fa..0c6b9b1b8f9ce4 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -5860,26 +5860,3 @@ specify the starting offset to begin embedding from. The resources is treated
 as being empty if the specified offset is larger than the number of bytes in
 the resource. The offset will be applied *before* any ``limit`` parameters are
 applied.
-
-Union and aggregate initialization in C
-=======================================
-
-In C23 (N2900), when an object is initialized from initializer ``= {}``, all
-elements of arrays, all members of structs, and the first members of unions are
-empty-initialized recursively. In addition, all padding bits are initialized to
-zero.
-
-Clang guarantees the following behaviors:
-
-* ``1:`` Clang supports initializer ``= {}`` mentioned above in all C
-  standards.
-
-* ``2:`` When unions are initialized from initializer ``= {}``, bytes outside
-  of the first members of unions are also initialized to zero.
-
-* ``3:`` When unions, structures and arrays are initialized from initializer
-  ``= { initializer-list }``, all members not explicitly initialized in
-  the initializer list are empty-initialized recursively. In addition, all
-  padding bits are initialized to zero.
-
-Currently, the above extension only applies to C source code, not C++.
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 43f3bcc95fe767..bbfc6672ecc25a 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 66bc0640b632aa..dd65080a840446 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 @@ class ConstStructBuilder {
   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 @@ class ConstExprEmitter
 
       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 fcdfef03088da5..c58bb88035ca8a 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1676,57 +1676,6 @@ class CodeGenModule : public CodeGenTypeCache {
     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;
 
diff --git a/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c b/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c
index b639734ef5d4b7..b72d689659e602 100644
--- a/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c
+++ b/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c
@@ -8,4 +8,4 @@ struct et7 {
   52, 
 };
 
-// CHECK: @yv7 ={{.*}} global { [0 x float], i8, [3 x i8] } { [0 x float] zeroinitializer, i8 52, [3 x i8] zeroinitializer }
+// CHECK: @yv7 ={{.*}} global %struct.et7 { [0 x float] zeroinitializer, i8 52 }
diff --git a/clang/test/CodeGen/2008-08-07-AlignPadding1.c b/clang/test/CodeGen/2008-08-07-AlignPadding1.c
index d69cbc22cc1dfb..17e88ce02659f0 100644
--- a/clang/test/CodeGen/2008-08-07-AlignPadding1.c
+++ b/clang/test/CodeGen/2008-08-07-AlignPadding1.c
@@ -20,9 +20,9 @@ struct gc_generation {
 
 #define GEN_HEAD(n) (&generations[n].head)
 
-// The idea is that there are 6 zeroinitializers in this structure initializer to cover
+// The idea is that there are 6 undefs in this structure initializer to cover
 // the padding between elements.
-// CHECK: @generations ={{.*}} global [3 x %struct.gc_generation] [%struct.gc_generation { %union._gc_head { %struct.anon { ptr @generations, ptr @generations, i64 0 }, [8 x i8] zeroinitializer }, i32 700, i32 0, [8 x i8] zeroinitializer }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 48), ptr getelementptr (i8, ptr @generations, i64 48), i64 0 }, [8 x i8] zeroinitializer }, i32 10, i32 0, [8 x i8] zeroinitializer }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 96), ptr getelementptr (i8, ptr @generations, i64 96), i64 0 }, [8 x i8] zeroinitializer }, i32 10, i32 0, [8 x i8] zeroinitializer }]
+// CHECK: @generations ={{.*}} global [3 x %struct.gc_generation] [%struct.gc_generation { %union._gc_head { %struct.anon { ptr @generations, ptr @generations, i64 0 }, [8 x i8] undef }, i32 700, i32 0, [8 x i8] undef }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 48), ptr getelementptr (i8, ptr @generations, i64 48), i64 0 }, [8 x i8] undef }, i32 10, i32 0, [8 x i8] undef }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 96), ptr getelementptr (i8, ptr @generations, i64 96), i64 0 }, [8 x i8] undef }, i32 10, i32 0, [8 x i8] undef }]
 /* linked lists of container objects */
 struct gc_generation generations[3] = {
         /* PyGC_Head,                           threshold,      count */
diff --git a/clang/test/CodeGen/2009-06-14-anonymous-union-init.c b/clang/test/CodeGen/2009-06-14-anonymous-union-init.c
index a4375d7868f01d..13f6357f7966d9 100644
--- a/clang/test/CodeGen/2009-06-14-anonymous-union-init.c
+++ b/clang/test/CodeGen/2009-06-14-anonymous-union-init.c
@@ -7,7 +7,7 @@ struct sysfs_dirent {
 };
 struct sysfs_dirent sysfs_root = { {}, 16877 };
 
-// CHECK: @sysfs_root = {{.*}}global { %union.anon, i16, [2 x i8] } { %union.anon zeroinitializer, i16 16877, [2 x i8] zeroinitializer }
+// CHECK: @sysfs_root = {{.*}}global %struct.sysfs_dirent { %union.anon zeroinitializer, i16 16877 }
 
 struct Foo {
  union { struct empty {} x; };
@@ -16,4 +16,4 @@ struct Foo {
 struct Foo foo = { {}, 16877 };
 
 // EMPTY:      @foo = {{.*}}global %struct.Foo { i16 16877 }
-// EMPTY-MSVC: @foo = {{.*}}global %struct.Foo { [4 x i8] zeroinitializer, i16 16877 }
+// EMPTY-MSVC: @foo = {{.*}}global %struct.Foo { [4 x i8] undef, i16 16877 }
diff --git a/clang/test/CodeGen/64bit-swiftcall.c b/clang/test/CodeGen/64bit-swiftcall.c
index 7f8aa02d97ce1f..7af65ccf556a81 100644
--- a/clang/test/CodeGen/64bit-swiftcall.c
+++ b/clang/test/CodeGen/64bit-swiftcall.c
@@ -14,6 +14,8 @@
 
 // CHECK-DAG: %struct.atomic_padded = type { { %struct.packed, [7 x i8] } }
 // CHECK-DAG: %struct.packed = type <{ i64, i8 }>
+//
+// CHECK: [[STRUCT2_RESULT:@.*]] = private {{.*}} constant [[STRUCT2_TYPE:%.*]] { i32 0, i8 0, i8 undef, i8 0, i32 0, i32 0 }
 
 /*****************************************************************************/
 /****************************** PARAMETER ABIS *******************************/
@@ -160,8 +162,8 @@ typedef struct {
 } struct_2;
 TEST(struct_2);
 // CHECK-LABEL: define{{.*}} swiftcc { i64, i64 } @return_struct_2() {{.*}}{
-// CHECK:   [[RET:%.*]] = alloca [[STRUCT2:%.*]], align 4
-// CHECK:   call void @llvm.memset
+// CHECK:   [[RET:%.*]] = alloca [[STRUCT2_TYPE]], align 4
+// CHECK:   call void @llvm.memcpy{{.*}}({{.*}}[[RET]], {{.*}}[[STRUCT2_RESULT]]
 // CHECK:   [[GEP0:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[RET]], i32 0, i32 0
 // CHECK:   [[T0:%.*]] = load i64, ptr [[GEP0]], align 4
 // CHECK:   [[GEP1:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[RET]], i32 0, i32 1
@@ -171,7 +173,7 @@ TEST(struct_2);
 // CHECK:   ret { i64, i64 } [[R1]]
 // CHECK: }
 // CHECK-LABEL: define{{.*}} swiftcc void @take_struct_2(i64 %0, i64 %1) {{.*}}{
-// CHECK:   [[V:%.*]] = alloca [[STRUCT2]], align 4
+// CHECK:   [[V:%.*]] = alloca [[STRUCT:%.*]], align 4
 // CHECK:   [[GEP0:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[V]], i32 0, i32 0
 // CHECK:   store i64 %0, ptr [[GEP0]], align 4
 // CHECK:   [[GEP1:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[V]], i32 0, i32 1
@@ -179,7 +181,7 @@ TEST(struct_2);
 // CHECK:   ret void
 // CHECK: }
 // CHECK-LABEL: define{{.*}} void @test_struct_2() {{.*}} {
-// CHECK:   [[TMP:%.*]] = alloca [[STRUCT2]], align 4
+// CHECK:   [[TMP:%.*]] = alloca [[STRUCT2_TYPE]], align 4
 // CHECK:   [[CALL:%.*]] = call swiftcc { i64, i64 } @return_struct_2()
 // CHECK:   [[GEP:%.*]] = getelementptr inbounds nuw {{.*}} [[TMP]], i32 0, i32 0
 // CHECK:   [[T0:%.*]] = extractvalue { i64, i64 } [[CALL]], 0
@@ -252,7 +254,7 @@ typedef union {
 TEST(union_het_fp)...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2024

@llvm/pr-subscribers-clang-codegen

Author: Eli Friedman (efriedma-quic)

Changes

Reverts llvm/llvm-project#97121

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


Patch is 56.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109898.diff

23 Files Affected:

  • (modified) clang/docs/LanguageExtensions.rst (-23)
  • (modified) clang/lib/CodeGen/CGExprAgg.cpp (+2-38)
  • (modified) clang/lib/CodeGen/CGExprConstant.cpp (+11-82)
  • (modified) clang/lib/CodeGen/CodeGenModule.h (-51)
  • (modified) clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c (+1-1)
  • (modified) clang/test/CodeGen/2008-08-07-AlignPadding1.c (+2-2)
  • (modified) clang/test/CodeGen/2009-06-14-anonymous-union-init.c (+2-2)
  • (modified) clang/test/CodeGen/64bit-swiftcall.c (+7-5)
  • (modified) clang/test/CodeGen/arm-swiftcall.c (+2-2)
  • (modified) clang/test/CodeGen/const-init.c (+2-2)
  • (modified) clang/test/CodeGen/decl.c (+2-2)
  • (modified) clang/test/CodeGen/designated-initializers.c (+6-6)
  • (modified) clang/test/CodeGen/ext-int.c (+9-9)
  • (modified) clang/test/CodeGen/flexible-array-init.c (+12-12)
  • (modified) clang/test/CodeGen/global-init.c (+1-1)
  • (modified) clang/test/CodeGen/init.c (+19)
  • (removed) clang/test/CodeGen/linux-kernel-struct-union-initializer.c (-267)
  • (removed) clang/test/CodeGen/linux-kernel-struct-union-initializer2.c (-140)
  • (modified) clang/test/CodeGen/mingw-long-double.c (+6-3)
  • (modified) clang/test/CodeGen/mms-bitfields.c (+2-2)
  • (modified) clang/test/CodeGen/union-init2.c (+2-2)
  • (modified) clang/test/CodeGen/windows-swiftcall.c (+7-5)
  • (modified) clang/test/CodeGenObjC/designated-initializers.m (+1-1)
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index f4be97047422fa..0c6b9b1b8f9ce4 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -5860,26 +5860,3 @@ specify the starting offset to begin embedding from. The resources is treated
 as being empty if the specified offset is larger than the number of bytes in
 the resource. The offset will be applied *before* any ``limit`` parameters are
 applied.
-
-Union and aggregate initialization in C
-=======================================
-
-In C23 (N2900), when an object is initialized from initializer ``= {}``, all
-elements of arrays, all members of structs, and the first members of unions are
-empty-initialized recursively. In addition, all padding bits are initialized to
-zero.
-
-Clang guarantees the following behaviors:
-
-* ``1:`` Clang supports initializer ``= {}`` mentioned above in all C
-  standards.
-
-* ``2:`` When unions are initialized from initializer ``= {}``, bytes outside
-  of the first members of unions are also initialized to zero.
-
-* ``3:`` When unions, structures and arrays are initialized from initializer
-  ``= { initializer-list }``, all members not explicitly initialized in
-  the initializer list are empty-initialized recursively. In addition, all
-  padding bits are initialized to zero.
-
-Currently, the above extension only applies to C source code, not C++.
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 43f3bcc95fe767..bbfc6672ecc25a 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 66bc0640b632aa..dd65080a840446 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 @@ class ConstStructBuilder {
   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 @@ class ConstExprEmitter
 
       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 fcdfef03088da5..c58bb88035ca8a 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1676,57 +1676,6 @@ class CodeGenModule : public CodeGenTypeCache {
     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;
 
diff --git a/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c b/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c
index b639734ef5d4b7..b72d689659e602 100644
--- a/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c
+++ b/clang/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c
@@ -8,4 +8,4 @@ struct et7 {
   52, 
 };
 
-// CHECK: @yv7 ={{.*}} global { [0 x float], i8, [3 x i8] } { [0 x float] zeroinitializer, i8 52, [3 x i8] zeroinitializer }
+// CHECK: @yv7 ={{.*}} global %struct.et7 { [0 x float] zeroinitializer, i8 52 }
diff --git a/clang/test/CodeGen/2008-08-07-AlignPadding1.c b/clang/test/CodeGen/2008-08-07-AlignPadding1.c
index d69cbc22cc1dfb..17e88ce02659f0 100644
--- a/clang/test/CodeGen/2008-08-07-AlignPadding1.c
+++ b/clang/test/CodeGen/2008-08-07-AlignPadding1.c
@@ -20,9 +20,9 @@ struct gc_generation {
 
 #define GEN_HEAD(n) (&generations[n].head)
 
-// The idea is that there are 6 zeroinitializers in this structure initializer to cover
+// The idea is that there are 6 undefs in this structure initializer to cover
 // the padding between elements.
-// CHECK: @generations ={{.*}} global [3 x %struct.gc_generation] [%struct.gc_generation { %union._gc_head { %struct.anon { ptr @generations, ptr @generations, i64 0 }, [8 x i8] zeroinitializer }, i32 700, i32 0, [8 x i8] zeroinitializer }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 48), ptr getelementptr (i8, ptr @generations, i64 48), i64 0 }, [8 x i8] zeroinitializer }, i32 10, i32 0, [8 x i8] zeroinitializer }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 96), ptr getelementptr (i8, ptr @generations, i64 96), i64 0 }, [8 x i8] zeroinitializer }, i32 10, i32 0, [8 x i8] zeroinitializer }]
+// CHECK: @generations ={{.*}} global [3 x %struct.gc_generation] [%struct.gc_generation { %union._gc_head { %struct.anon { ptr @generations, ptr @generations, i64 0 }, [8 x i8] undef }, i32 700, i32 0, [8 x i8] undef }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 48), ptr getelementptr (i8, ptr @generations, i64 48), i64 0 }, [8 x i8] undef }, i32 10, i32 0, [8 x i8] undef }, %struct.gc_generation { %union._gc_head { %struct.anon { ptr getelementptr (i8, ptr @generations, i64 96), ptr getelementptr (i8, ptr @generations, i64 96), i64 0 }, [8 x i8] undef }, i32 10, i32 0, [8 x i8] undef }]
 /* linked lists of container objects */
 struct gc_generation generations[3] = {
         /* PyGC_Head,                           threshold,      count */
diff --git a/clang/test/CodeGen/2009-06-14-anonymous-union-init.c b/clang/test/CodeGen/2009-06-14-anonymous-union-init.c
index a4375d7868f01d..13f6357f7966d9 100644
--- a/clang/test/CodeGen/2009-06-14-anonymous-union-init.c
+++ b/clang/test/CodeGen/2009-06-14-anonymous-union-init.c
@@ -7,7 +7,7 @@ struct sysfs_dirent {
 };
 struct sysfs_dirent sysfs_root = { {}, 16877 };
 
-// CHECK: @sysfs_root = {{.*}}global { %union.anon, i16, [2 x i8] } { %union.anon zeroinitializer, i16 16877, [2 x i8] zeroinitializer }
+// CHECK: @sysfs_root = {{.*}}global %struct.sysfs_dirent { %union.anon zeroinitializer, i16 16877 }
 
 struct Foo {
  union { struct empty {} x; };
@@ -16,4 +16,4 @@ struct Foo {
 struct Foo foo = { {}, 16877 };
 
 // EMPTY:      @foo = {{.*}}global %struct.Foo { i16 16877 }
-// EMPTY-MSVC: @foo = {{.*}}global %struct.Foo { [4 x i8] zeroinitializer, i16 16877 }
+// EMPTY-MSVC: @foo = {{.*}}global %struct.Foo { [4 x i8] undef, i16 16877 }
diff --git a/clang/test/CodeGen/64bit-swiftcall.c b/clang/test/CodeGen/64bit-swiftcall.c
index 7f8aa02d97ce1f..7af65ccf556a81 100644
--- a/clang/test/CodeGen/64bit-swiftcall.c
+++ b/clang/test/CodeGen/64bit-swiftcall.c
@@ -14,6 +14,8 @@
 
 // CHECK-DAG: %struct.atomic_padded = type { { %struct.packed, [7 x i8] } }
 // CHECK-DAG: %struct.packed = type <{ i64, i8 }>
+//
+// CHECK: [[STRUCT2_RESULT:@.*]] = private {{.*}} constant [[STRUCT2_TYPE:%.*]] { i32 0, i8 0, i8 undef, i8 0, i32 0, i32 0 }
 
 /*****************************************************************************/
 /****************************** PARAMETER ABIS *******************************/
@@ -160,8 +162,8 @@ typedef struct {
 } struct_2;
 TEST(struct_2);
 // CHECK-LABEL: define{{.*}} swiftcc { i64, i64 } @return_struct_2() {{.*}}{
-// CHECK:   [[RET:%.*]] = alloca [[STRUCT2:%.*]], align 4
-// CHECK:   call void @llvm.memset
+// CHECK:   [[RET:%.*]] = alloca [[STRUCT2_TYPE]], align 4
+// CHECK:   call void @llvm.memcpy{{.*}}({{.*}}[[RET]], {{.*}}[[STRUCT2_RESULT]]
 // CHECK:   [[GEP0:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[RET]], i32 0, i32 0
 // CHECK:   [[T0:%.*]] = load i64, ptr [[GEP0]], align 4
 // CHECK:   [[GEP1:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[RET]], i32 0, i32 1
@@ -171,7 +173,7 @@ TEST(struct_2);
 // CHECK:   ret { i64, i64 } [[R1]]
 // CHECK: }
 // CHECK-LABEL: define{{.*}} swiftcc void @take_struct_2(i64 %0, i64 %1) {{.*}}{
-// CHECK:   [[V:%.*]] = alloca [[STRUCT2]], align 4
+// CHECK:   [[V:%.*]] = alloca [[STRUCT:%.*]], align 4
 // CHECK:   [[GEP0:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[V]], i32 0, i32 0
 // CHECK:   store i64 %0, ptr [[GEP0]], align 4
 // CHECK:   [[GEP1:%.*]] = getelementptr inbounds nuw { i64, i64 }, ptr [[V]], i32 0, i32 1
@@ -179,7 +181,7 @@ TEST(struct_2);
 // CHECK:   ret void
 // CHECK: }
 // CHECK-LABEL: define{{.*}} void @test_struct_2() {{.*}} {
-// CHECK:   [[TMP:%.*]] = alloca [[STRUCT2]], align 4
+// CHECK:   [[TMP:%.*]] = alloca [[STRUCT2_TYPE]], align 4
 // CHECK:   [[CALL:%.*]] = call swiftcc { i64, i64 } @return_struct_2()
 // CHECK:   [[GEP:%.*]] = getelementptr inbounds nuw {{.*}} [[TMP]], i32 0, i32 0
 // CHECK:   [[T0:%.*]] = extractvalue { i64, i64 } [[CALL]], 0
@@ -252,7 +254,7 @@ typedef union {
 TEST(union_het_fp)...
[truncated]

@efriedma-quic efriedma-quic merged commit d50eaac into main Sep 25, 2024
9 of 11 checks passed
@efriedma-quic efriedma-quic deleted the revert-97121-zero-init-unspecified-fields-in-initializers-in-c branch September 25, 2024 03:31
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 25, 2024

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building clang at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/5542

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentDelaySignalWatch.py (610 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentDelayedCrashWithBreakpointSignal.py (611 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentDelayedCrashWithBreakpointWatchpoint.py (612 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py (613 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentSignalBreak.py (614 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py (615 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentSignalDelayBreak.py (616 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentManyCrash.py (617 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentManySignals.py (618 of 2031)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py (619 of 2031)
FAIL: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py (620 of 2031)
******************** TEST 'lldb-api :: functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/local/bin/llvm-ar --env OBJCOPY=/usr/bin/llvm-objcopy --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/thread/concurrent_events -p TestConcurrentSignalWatch.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision d50eaac12f0cdfe27e942290942b06889ab12a8c)
  clang revision d50eaac12f0cdfe27e942290942b06889ab12a8c
  llvm revision d50eaac12f0cdfe27e942290942b06889ab12a8c
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

Watchpoint 1 hit:
old value: 0
new value: 1

--
Command Output (stderr):
--
FAIL: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test (TestConcurrentSignalWatch.ConcurrentSignalWatch)
======================================================================
FAIL: test (TestConcurrentSignalWatch.ConcurrentSignalWatch)
   Test a watchpoint and a signal in multiple threads.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 148, in wrapper
    return func(*args, **kwargs)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py", line 14, in test
    self.do_thread_actions(num_signal_threads=1, num_watchpoint_threads=1)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/concurrent_base.py", line 333, in do_thread_actions
    self.assertEqual(
AssertionError: 1 != 2 : Expected 1 stops due to signal delivery, but got 2
Config=aarch64-/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang
----------------------------------------------------------------------
Ran 1 test in 0.615s


@yabinc
Copy link
Contributor

yabinc commented Sep 25, 2024

LGTM

One problem I found is for bitfields:
struct {
char x;
unsigned char y : 4;
unsigned char z : 7;
} attribute((packed)) t5 = { 101, 15, 123 };

The last field takes byte 2-3, while the code thinks it only takes byte 2. The code didn't consider well for fields starting from (or ending with) a (% 8 != 0) bit offset.

yabinc added a commit to yabinc/llvm-project that referenced this pull request Sep 25, 2024
…s in C" (llvm#109898)

This reverts commit d50eaac.
Also fixes a bug calculating offsets for bit fields in the original patch.
augusto2112 pushed a commit to augusto2112/llvm-project that referenced this pull request Sep 26, 2024
… in C" (llvm#109898)

Reverts llvm#97121

Causing failures on LNT bots; log shows a crash in
ConstStructBuilder::BuildStruct.
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
… in C" (llvm#109898)

Reverts llvm#97121

Causing failures on LNT bots; log shows a crash in
ConstStructBuilder::BuildStruct.
yabinc added a commit that referenced this pull request Oct 14, 2024
…s in C" (#109898) (#110051)

This reverts commit d50eaac. Also fixes
a bug calculating offsets for bit fields in the original patch.
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…s in C" (llvm#109898) (llvm#110051)

This reverts commit d50eaac. Also fixes
a bug calculating offsets for bit fields in the original patch.
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
…s in C" (llvm#109898) (llvm#110051)

This reverts commit d50eaac. Also fixes
a bug calculating offsets for bit fields in the original patch.
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
…s in C" (llvm#109898) (llvm#110051)

This reverts commit d50eaac. Also fixes
a bug calculating offsets for bit fields in the original patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants