Author: Akash Manna Date: 2026-09-09T16:48:54Z New Revision: eff57ef78633c2eff5ae2cb7e4fa8e11e14d4b46
URL: https://github.com/llvm/llvm-project/commit/eff57ef78633c2eff5ae2cb7e4fa8e11e14d4b46 DIFF: https://github.com/llvm/llvm-project/commit/eff57ef78633c2eff5ae2cb7e4fa8e11e14d4b46.diff LOG: [clang][CodeGen] Fix assertion failure with #embed in array new-expression initializers (#218262) Fixes #128985 An `EmbedExpr` in a semantic initializer list can represent many array elements at once, and every consumer of such lists has to expand it. The array `new` emitter, `EmitNewArrayInitializer`, never did: it counted the `EmbedExpr` as one element and emitted it through the scalar path, hitting `assert(E->getDataElementCount() == 1)`. The undercount also made the runtime minimum allocation check too lax and miscomputed the trailing zero-fill size. `Codegen` now emits one store per embed data element, converted to the element type, and counts initializers with `getNumInitsWithEmbedExpanded()` in both places. That helper also learned to look through implicit casts, since Sema wraps a multi-element `EmbedExpr` in a conversion when the element type isn't `int` — the other embed consumers already strip casts before checking. LLM tools were used for this contribution. I've reviewed, built, and tested the change myself before pushing to GitHub. Added: clang/test/CodeGenCXX/GH128985.cpp clang/test/CodeGenCXX/Inputs/embed-data.txt clang/test/SemaCXX/GH128985.cpp Modified: clang/docs/ReleaseNotes.md clang/include/clang/AST/Expr.h clang/lib/CodeGen/CGExprCXX.cpp clang/lib/Sema/SemaInit.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 679a603d58ac0..4eb356997b8c1 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -553,6 +553,10 @@ features cannot lower the translation-unit ABI level; - Fixed an issue where `__typeof__` incorrectly rejected cv-qualified function types. +- Fixed an assertion failure when `#embed` was used in the braced initializer + of an array new-expression, or of an array whose elements are of class type. + (#GH128985) + - Fixed a bug where top-level CV qualifiers (such as ``const``) were dropped from pointers modified by Microsoft pointer attributes (like ``__ptr32`` and ``__ptr64``) and WebAssembly's ``__funcref``. - Fixed a bug where we accepted ``__super`` being qualified by a scope specifier, causing codegen to assertion fail elsewhere. (#GH212988) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 535086a6c2aa3..c03c88232e13d 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -5175,7 +5175,7 @@ struct EmbedDataStorage { /// { {EE(9th and 10th element), { zeroinitializer }}} /// /// EmbedExpr inside of a semantic initializer list and referencing more than -/// one element can only appear for arrays of scalars. +/// one element can only appear for arrays of integer or floating-point type. class EmbedExpr final : public Expr { SourceLocation EmbedKeywordLoc; IntegerLiteral *FakeChildNode = nullptr; @@ -5389,7 +5389,7 @@ class InitListExpr : public Expr { unsigned getNumInitsWithEmbedExpanded() const { unsigned Sum = InitExprs.size(); for (auto *IE : InitExprs) - if (auto *EE = dyn_cast<EmbedExpr>(IE)) + if (auto *EE = dyn_cast<EmbedExpr>(cast<Expr>(IE)->IgnoreParenImpCasts())) Sum += EE->getDataElementCount() - 1; return Sum; } diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index e400a5c5a49c5..97bfcd7bda4e8 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1103,7 +1103,8 @@ void CodeGenFunction::EmitNewArrayInitializer( ArrayRef<const Expr *> InitExprs = ILE ? ILE->inits() : CPLIE->getInitExprs(); - InitListElements = InitExprs.size(); + InitListElements = + ILE ? ILE->getNumInitsWithEmbedExpanded() : InitExprs.size(); // If this is a multi-dimensional array new, we will initialize multiple // elements with each init list element. @@ -1138,6 +1139,14 @@ void CodeGenFunction::EmitNewArrayInitializer( CharUnits StartAlign = CurPtr.getAlignment(); unsigned i = 0; + auto AdvanceToNextElement = [&]() { + CurPtr = Address(Builder.CreateInBoundsGEP(CurPtr.getElementType(), + CurPtr.emitRawPointer(*this), + Builder.getSize(1), + "array.exp.next"), + CurPtr.getElementType(), + StartAlign.alignmentAtOffset((++i) * ElementSize)); + }; for (const Expr *IE : InitExprs) { // Tell the cleanup that it needs to destroy up to this // element. TODO: some of these stores can be trivially @@ -1145,17 +1154,32 @@ void CodeGenFunction::EmitNewArrayInitializer( if (EndOfInit.isValid()) { Builder.CreateStore(CurPtr.emitRawPointer(*this), EndOfInit); } + // A multi-element EmbedExpr initializes several array elements at once. + // A single-element embed can be wrapped in a conversion to a non-scalar + // element type (e.g. _Complex) and is emitted like any other + // initializer. + const auto *EmbedS = dyn_cast<EmbedExpr>(IE->IgnoreParenImpCasts()); + if (EmbedS && EmbedS->getDataElementCount() > 1) { + const StringLiteral *SL = EmbedS->getDataStringLiteral(); + llvm::Type *DataTy = ConvertType(EmbedS->getType()); + for (unsigned I = EmbedS->getStartingElementPos(), + End = I + EmbedS->getDataElementCount(); + I != End; ++I) { + llvm::Value *Val = EmitScalarConversion( + llvm::ConstantInt::get(DataTy, SL->getCodeUnit(I)), + EmbedS->getType(), ElementType, EmbedS->getLocation()); + EmitStoreOfScalar(Val, MakeAddrLValue(CurPtr, ElementType), + /*isInit=*/true); + AdvanceToNextElement(); + } + continue; + } // FIXME: If the last initializer is an incomplete initializer list for // an array, and we have an array filler, we can fold together the two // initialization loops. StoreAnyExprIntoOneUnit(*this, IE, IE->getType(), CurPtr, AggValueSlot::DoesNotOverlap); - CurPtr = Address(Builder.CreateInBoundsGEP(CurPtr.getElementType(), - CurPtr.emitRawPointer(*this), - Builder.getSize(1), - "array.exp.next"), - CurPtr.getElementType(), - StartAlign.alignmentAtOffset((++i) * ElementSize)); + AdvanceToNextElement(); } // The remaining elements are filled with the array filler expression. @@ -1591,7 +1615,8 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { cast<ConstantArrayType>(Init->getType()->getAsArrayTypeUnsafe()) ->getZExtSize(); } else if (ILE || CPLIE) { - minElements = ILE ? ILE->getNumInits() : CPLIE->getInitExprs().size(); + minElements = ILE ? ILE->getNumInitsWithEmbedExpanded() + : CPLIE->getInitExprs().size(); } } diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 48ce51863c2c0..1ff66e0d927df 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -562,7 +562,9 @@ class InitListChecker { // Reference just one if we're initializing a single scalar. uint64_t ElsCount = 1; // Otherwise try to fill whole array with embed data. - if (Entity.getKind() == InitializedEntity::EK_ArrayElement) { + if (Entity.getKind() == InitializedEntity::EK_ArrayElement && + (Entity.getType()->isIntegerType() || + Entity.getType()->isRealFloatingType())) { unsigned ArrIndex = Entity.getElementIndex(); auto *AType = SemaRef.Context.getAsArrayType(Entity.getParent()->getType()); diff --git a/clang/test/CodeGenCXX/GH128985.cpp b/clang/test/CodeGenCXX/GH128985.cpp new file mode 100644 index 0000000000000..a1d813cc43dfd --- /dev/null +++ b/clang/test/CodeGenCXX/GH128985.cpp @@ -0,0 +1,234 @@ +// RUN: %clang_cc1 %s -triple x86_64 --embed-dir=%S/Inputs -emit-llvm -o - | FileCheck %s + +// embed-data.txt contains "0123456789" (48 ... 57) without a trailing newline. + +struct S { + int a, b; +}; + +struct A { + A(char); +}; + +// CHECK-LABEL: define {{.*}}void @_Z2f1i( +// CHECK: icmp ult i64 %{{.*}}, 4 +// CHECK: %[[A1:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 48, ptr %[[A1]] +// CHECK: %[[F1E1:.*]] = getelementptr inbounds i32, ptr %[[A1]], i64 1 +// CHECK: store i32 49, ptr %[[F1E1]] +// CHECK: %[[F1E2:.*]] = getelementptr inbounds i32, ptr %[[F1E1]], i64 1 +// CHECK: store i32 50, ptr %[[F1E2]] +// CHECK: %[[F1E3:.*]] = getelementptr inbounds i32, ptr %[[F1E2]], i64 1 +// CHECK: store i32 51, ptr %[[F1E3]] +// CHECK: %[[F1REST:.*]] = sub i64 %{{.*}}, 16 +// CHECK: call void @llvm.memset.p0.i64(ptr align 4 %{{.*}}, i8 0, i64 %[[F1REST]], i1 false) +void f1(int x) { + int *p = new int[x]{ +#embed <embed-data.txt> limit(4) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z2f2i( +// CHECK: icmp ult i64 %{{.*}}, 4 +// CHECK: %[[A2:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 500, ptr %[[A2]] +// CHECK: %[[F2E1:.*]] = getelementptr inbounds i32, ptr %[[A2]], i64 1 +// CHECK: store i32 48, ptr %[[F2E1]] +// CHECK: %[[F2E2:.*]] = getelementptr inbounds i32, ptr %[[F2E1]], i64 1 +// CHECK: store i32 49, ptr %[[F2E2]] +// CHECK: %[[F2E3:.*]] = getelementptr inbounds i32, ptr %[[F2E2]], i64 1 +// CHECK: store i32 600, ptr %[[F2E3]] +// CHECK: %[[F2REST:.*]] = sub i64 %{{.*}}, 16 +// CHECK: call void @llvm.memset.p0.i64(ptr align 4 %{{.*}}, i8 0, i64 %[[F2REST]], i1 false) +void f2(int x) { + int *p = new int[x]{ + 500, +#embed <embed-data.txt> limit(2) suffix(, 600) + }; +} + +// char arrays go through the string literal initialization path. +// CHECK-LABEL: define {{.*}}void @_Z2f3i( +// CHECK: icmp ult i64 %{{.*}}, 4 +// CHECK: %[[A3:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 1 %[[A3]], ptr align 1 @{{.*}}, i64 4, i1 false) +// CHECK: %[[F3END:.*]] = getelementptr inbounds i8, ptr %[[A3]], i64 4 +// CHECK: %[[F3REST:.*]] = sub i64 %{{.*}}, 4 +// CHECK: call void @llvm.memset.p0.i64(ptr align 1 %[[F3END]], i8 0, i64 %[[F3REST]], i1 false) +void f3(int x) { + char *p = new char[x]{ +#embed <embed-data.txt> limit(4) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z2f4i( +// CHECK: icmp ult i64 %{{.*}}, 2 +// CHECK: %[[A4:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 900, ptr %[[A4]] +// CHECK: %[[F4E1:.*]] = getelementptr inbounds i32, ptr %[[A4]], i64 1 +// CHECK: store i32 48, ptr %[[F4E1]] +// CHECK: %[[F4REST:.*]] = sub i64 %{{.*}}, 8 +// CHECK: call void @llvm.memset.p0.i64(ptr align 4 %{{.*}}, i8 0, i64 %[[F4REST]], i1 false) +void f4(int x) { + int *p = new int[x]{ +#embed <embed-data.txt> limit(1) prefix(900, ) + }; +} + +// Constant size fully covered by the embed data: no trailing fill. +// CHECK-LABEL: define {{.*}}void @_Z2f5v( +// CHECK: %[[A5:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 48, ptr %[[A5]] +// CHECK: %[[F5E1:.*]] = getelementptr inbounds i32, ptr %[[A5]], i64 1 +// CHECK: store i32 49, ptr %[[F5E1]] +// CHECK: %[[F5E2:.*]] = getelementptr inbounds i32, ptr %[[F5E1]], i64 1 +// CHECK: store i32 50, ptr %[[F5E2]] +// CHECK: %[[F5E3:.*]] = getelementptr inbounds i32, ptr %[[F5E2]], i64 1 +// CHECK: store i32 51, ptr %[[F5E3]] +// CHECK-NOT: call void @llvm.memset +// CHECK: ret void +void f5() { + int *p = new int[4]{ +#embed <embed-data.txt> limit(4) + }; +} + +// Sema wraps the EmbedExpr in an implicit conversion to the element type. +// CHECK-LABEL: define {{.*}}void @_Z2f6i( +// CHECK: icmp ult i64 %{{.*}}, 4 +// CHECK: %[[A6:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i64 48, ptr %[[A6]] +// CHECK: %[[F6E1:.*]] = getelementptr inbounds i64, ptr %[[A6]], i64 1 +// CHECK: store i64 49, ptr %[[F6E1]] +// CHECK: %[[F6E2:.*]] = getelementptr inbounds i64, ptr %[[F6E1]], i64 1 +// CHECK: store i64 50, ptr %[[F6E2]] +// CHECK: %[[F6E3:.*]] = getelementptr inbounds i64, ptr %[[F6E2]], i64 1 +// CHECK: store i64 51, ptr %[[F6E3]] +// CHECK: %[[F6REST:.*]] = sub i64 %{{.*}}, 32 +// CHECK: call void @llvm.memset.p0.i64(ptr align 8 %{{.*}}, i8 0, i64 %[[F6REST]], i1 false) +void f6(int x) { + long long *p = new long long[x]{ +#embed <embed-data.txt> limit(4) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z2f7i( +// CHECK: icmp ult i64 %{{.*}}, 2 +// CHECK: call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 48, ptr +// CHECK: store i32 49, ptr +// CHECK: store i32 50, ptr +// CHECK: store i32 51, ptr +// CHECK: %[[F7REST:.*]] = sub i64 %{{.*}}, 16 +// CHECK: call void @llvm.memset.p0.i64(ptr align {{[0-9]+}} %{{.*}}, i8 0, i64 %[[F7REST]], i1 false) +void f7(int x) { + int (*p)[2] = new int[x][2]{ +#embed <embed-data.txt> limit(4) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z2f8i( +// CHECK: icmp ult i64 %{{.*}}, 2 +// CHECK: call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 48, ptr +// CHECK: store i32 49, ptr +// CHECK: store i32 50, ptr +// CHECK: store i32 51, ptr +// CHECK: %[[F8REST:.*]] = sub i64 %{{.*}}, 16 +// CHECK: call void @llvm.memset.p0.i64(ptr align {{[0-9]+}} %{{.*}}, i8 0, i64 %[[F8REST]], i1 false) +void f8(int x) { + S *p = new S[x]{ +#embed <embed-data.txt> limit(4) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z2f9i( +// CHECK: icmp ult i64 %{{.*}}, 2 +// CHECK: call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store i32 48, ptr +// CHECK: store i32 49, ptr +// CHECK: store i32 50, ptr +// CHECK: store i32 51, ptr +// CHECK: store i32 52, ptr +// CHECK: store i32 53, ptr +// CHECK: store i32 54, ptr +// CHECK: store i32 55, ptr +// CHECK: %[[F9REST:.*]] = sub i64 %{{.*}}, 32 +// CHECK: call void @llvm.memset.p0.i64(ptr align {{[0-9]+}} %{{.*}}, i8 0, i64 %[[F9REST]], i1 false) +void f9(int x) { + S (*p)[2] = new S[x][2]{ +#embed <embed-data.txt> limit(8) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z3f10v( +// CHECK: %[[A10:.*]] = call {{.*}}ptr @_Znam(i64 noundef 16) +// CHECK: store i32 48, ptr %[[A10]] +// CHECK: %[[F10E1:.*]] = getelementptr inbounds i32, ptr %[[A10]], i64 1 +// CHECK: store i32 49, ptr %[[F10E1]] +// CHECK: %[[F10E2:.*]] = getelementptr inbounds i32, ptr %[[F10E1]], i64 1 +// CHECK: store i32 50, ptr %[[F10E2]] +// CHECK: %[[F10E3:.*]] = getelementptr inbounds i32, ptr %[[F10E2]], i64 1 +// CHECK: store i32 51, ptr %[[F10E3]] +// CHECK-NOT: call void @llvm.memset +// CHECK: ret void +void f10() { + int *p = new int[]{ +#embed <embed-data.txt> limit(4) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z3f11i( +// CHECK: icmp ult i64 %{{.*}}, 4 +// CHECK: %[[A11:.*]] = call {{.*}}ptr @_Znam(i64 {{.*}}) +// CHECK: store float 4.800000e+01, ptr %[[A11]] +// CHECK: %[[F11E1:.*]] = getelementptr inbounds float, ptr %[[A11]], i64 1 +// CHECK: store float 4.900000e+01, ptr %[[F11E1]] +// CHECK: %[[F11E2:.*]] = getelementptr inbounds float, ptr %[[F11E1]], i64 1 +// CHECK: store float 5.000000e+01, ptr %[[F11E2]] +// CHECK: %[[F11E3:.*]] = getelementptr inbounds float, ptr %[[F11E2]], i64 1 +// CHECK: store float 5.100000e+01, ptr %[[F11E3]] +// CHECK: %[[F11REST:.*]] = sub i64 %{{.*}}, 16 +// CHECK: call void @llvm.memset.p0.i64(ptr align 4 %{{.*}}, i8 0, i64 %[[F11REST]], i1 false) +void f11(int x) { + float *p = new float[x]{ +#embed <embed-data.txt> limit(4) + }; +} + +// Class elements are constructed from one data element each. +// CHECK-LABEL: define {{.*}}void @_Z3f12v( +// CHECK: %[[A12:.*]] = call {{.*}}ptr @_Znam(i64 noundef 2) +// CHECK: call void @_ZN1AC1Ec(ptr {{.*}}%[[A12]], i8 noundef signext 48) +// CHECK: %[[F12E1:.*]] = getelementptr inbounds %struct.A, ptr %[[A12]], i64 1 +// CHECK: call void @_ZN1AC1Ec(ptr {{.*}}%[[F12E1]], i8 noundef signext 49) +void f12() { + A *p = new A[]{ +#embed <embed-data.txt> limit(2) + }; +} + +// Complex elements are not integer or floating-point type, so Sema slices +// the embed into single-element EmbedExprs wrapped in an int-to-complex +// conversion, one per element. +// CHECK-LABEL: define {{.*}}void @_Z3f13v( +// CHECK: call {{.*}}ptr @_Znam(i64 noundef 32) +// CHECK: store double 4.800000e+01, ptr +// CHECK: store double 0.000000e+00, ptr +// CHECK: store double 4.900000e+01, ptr +// CHECK: store double 0.000000e+00, ptr +void f13() { + _Complex double *p = new _Complex double[]{ +#embed <embed-data.txt> limit(2) + }; +} + +// CHECK-LABEL: define {{.*}}void @_Z3f14v( +// CHECK: %[[A14:.*]] = call {{.*}}ptr @_Znam(i64 noundef 16) +// CHECK: store double 4.800000e+01, ptr +// CHECK: store double 0.000000e+00, ptr +void f14() { + _Complex double *p = new _Complex double[]{ +#embed <embed-data.txt> limit(1) + }; +} diff --git a/clang/test/CodeGenCXX/Inputs/embed-data.txt b/clang/test/CodeGenCXX/Inputs/embed-data.txt new file mode 100644 index 0000000000000..ad471007bd7f5 --- /dev/null +++ b/clang/test/CodeGenCXX/Inputs/embed-data.txt @@ -0,0 +1 @@ +0123456789 \ No newline at end of file diff --git a/clang/test/SemaCXX/GH128985.cpp b/clang/test/SemaCXX/GH128985.cpp new file mode 100644 index 0000000000000..0d51e3e53582f --- /dev/null +++ b/clang/test/SemaCXX/GH128985.cpp @@ -0,0 +1,54 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c23-extensions %s + +struct S { + int a, b; +}; + +struct A { + A(char); +}; + +void f(int x) { + int *a = new int[2]{ +#embed __FILE__ limit(4) + // expected-error@-1 {{excess elements in array initializer}} + }; + + int *b = new int[4]{ +#embed __FILE__ limit(4) + }; + + int *c = new int[8]{ +#embed __FILE__ limit(4) + }; + + int *d = new int[x]{ +#embed __FILE__ limit(4) + }; + + int (*e)[2] = new int[2][2]{ +#embed __FILE__ limit(5) + // expected-error@-1 {{excess elements in array initializer}} + }; + + S *s = new S[1]{ +#embed __FILE__ limit(3) + // expected-error@-1 {{excess elements in array initializer}} + }; + + S *t = new S[x]{ +#embed __FILE__ limit(3) + }; + + A *u = new A[]{1, 2, 3, +#embed __FILE__ limit(10) + }; + + A v[] = {1, 2, 3, +#embed __FILE__ limit(10) + }; + + _Complex double *w = new _Complex double[]{ +#embed __FILE__ limit(10) + }; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
