Skip to content

Commit

Permalink
[clang] Fix parenthesized list initialization of arrays not working w…
Browse files Browse the repository at this point in the history
…ith `new` (#76976)

This bug is caused by parenthesized list initialization not being
implemented in `CodeGenFunction::EmitNewArrayInitializer(...)`.

Parenthesized list initialization of `struct`s with `operator new`
already works in Clang and is not affected by this bug.

Additionally, fix the test new-delete.cpp as it incorrectly assumes that
using parentheses with operator new to initialize arrays is illegal for
C++ versions >= C++17.

Fixes #68198
  • Loading branch information
alanzhao1 committed Jan 18, 2024
1 parent 5de1d00 commit 2c9f04c
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 33 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,9 @@ Bug Fixes to C++ Support
``std::source_location::current()`` was used in a function template.
Fixes (`#78128 <https://github.com/llvm/llvm-project/issues/78128>`_)

- Clang now allows parenthesized initialization of arrays in `operator new[]`.
Fixes: (`#68198 <https://github.com/llvm/llvm-project/issues/68198>`_)

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
- Fixed an import failure of recursive friend class template.
Expand Down
60 changes: 42 additions & 18 deletions clang/lib/CodeGen/CGExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1038,11 +1038,25 @@ void CodeGenFunction::EmitNewArrayInitializer(
return true;
};

const InitListExpr *ILE = dyn_cast<InitListExpr>(Init);
const CXXParenListInitExpr *CPLIE = nullptr;
const StringLiteral *SL = nullptr;
const ObjCEncodeExpr *OCEE = nullptr;
const Expr *IgnoreParen = nullptr;
if (!ILE) {
IgnoreParen = Init->IgnoreParenImpCasts();
CPLIE = dyn_cast<CXXParenListInitExpr>(IgnoreParen);
SL = dyn_cast<StringLiteral>(IgnoreParen);
OCEE = dyn_cast<ObjCEncodeExpr>(IgnoreParen);
}

// If the initializer is an initializer list, first do the explicit elements.
if (const InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
if (ILE || CPLIE || SL || OCEE) {
// Initializing from a (braced) string literal is a special case; the init
// list element does not initialize a (single) array element.
if (ILE->isStringLiteralInit()) {
if ((ILE && ILE->isStringLiteralInit()) || SL || OCEE) {
if (!ILE)
Init = IgnoreParen;
// Initialize the initial portion of length equal to that of the string
// literal. The allocation must be for at least this much; we emitted a
// check for that earlier.
Expand All @@ -1054,12 +1068,13 @@ void CodeGenFunction::EmitNewArrayInitializer(
AggValueSlot::DoesNotOverlap,
AggValueSlot::IsNotZeroed,
AggValueSlot::IsSanitizerChecked);
EmitAggExpr(ILE->getInit(0), Slot);
EmitAggExpr(ILE ? ILE->getInit(0) : Init, Slot);

// Move past these elements.
InitListElements =
cast<ConstantArrayType>(ILE->getType()->getAsArrayTypeUnsafe())
->getSize().getZExtValue();
cast<ConstantArrayType>(Init->getType()->getAsArrayTypeUnsafe())
->getSize()
.getZExtValue();
CurPtr = Builder.CreateConstInBoundsGEP(
CurPtr, InitListElements, "string.init.end");

Expand All @@ -1073,7 +1088,9 @@ void CodeGenFunction::EmitNewArrayInitializer(
return;
}

InitListElements = ILE->getNumInits();
ArrayRef<const Expr *> InitExprs =
ILE ? ILE->inits() : CPLIE->getInitExprs();
InitListElements = InitExprs.size();

// If this is a multi-dimensional array new, we will initialize multiple
// elements with each init list element.
Expand Down Expand Up @@ -1101,7 +1118,8 @@ void CodeGenFunction::EmitNewArrayInitializer(
}

CharUnits StartAlign = CurPtr.getAlignment();
for (unsigned i = 0, e = ILE->getNumInits(); i != e; ++i) {
unsigned i = 0;
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
// observed to be unnecessary.
Expand All @@ -1111,18 +1129,17 @@ void CodeGenFunction::EmitNewArrayInitializer(
// 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, ILE->getInit(i),
ILE->getInit(i)->getType(), CurPtr,
StoreAnyExprIntoOneUnit(*this, IE, IE->getType(), CurPtr,
AggValueSlot::DoesNotOverlap);
CurPtr = Address(Builder.CreateInBoundsGEP(
CurPtr.getElementType(), CurPtr.getPointer(),
Builder.getSize(1), "array.exp.next"),
CurPtr.getElementType(),
StartAlign.alignmentAtOffset((i + 1) * ElementSize));
StartAlign.alignmentAtOffset((++i) * ElementSize));
}

// The remaining elements are filled with the array filler expression.
Init = ILE->getArrayFiller();
Init = ILE ? ILE->getArrayFiller() : CPLIE->getArrayFiller();

// Extract the initializer for the individual array elements by pulling
// out the array filler from all the nested initializer lists. This avoids
Expand Down Expand Up @@ -1561,16 +1578,23 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
// 1. Build a call to the allocation function.
FunctionDecl *allocator = E->getOperatorNew();

// If there is a brace-initializer, cannot allocate fewer elements than inits.
// If there is a brace-initializer or C++20 parenthesized initializer, cannot
// allocate fewer elements than inits.
unsigned minElements = 0;
if (E->isArray() && E->hasInitializer()) {
const InitListExpr *ILE = dyn_cast<InitListExpr>(E->getInitializer());
if (ILE && ILE->isStringLiteralInit())
const Expr *Init = E->getInitializer();
const InitListExpr *ILE = dyn_cast<InitListExpr>(Init);
const CXXParenListInitExpr *CPLIE = dyn_cast<CXXParenListInitExpr>(Init);
const Expr *IgnoreParen = Init->IgnoreParenImpCasts();
if ((ILE && ILE->isStringLiteralInit()) ||
isa<StringLiteral>(IgnoreParen) || isa<ObjCEncodeExpr>(IgnoreParen)) {
minElements =
cast<ConstantArrayType>(ILE->getType()->getAsArrayTypeUnsafe())
->getSize().getZExtValue();
else if (ILE)
minElements = ILE->getNumInits();
cast<ConstantArrayType>(Init->getType()->getAsArrayTypeUnsafe())
->getSize()
.getZExtValue();
} else if (ILE || CPLIE) {
minElements = ILE ? ILE->getNumInits() : CPLIE->getInitExprs().size();
}
}

llvm::Value *numElements = nullptr;
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1947,11 +1947,11 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
}

static bool isLegalArrayNewInitializer(CXXNewInitializationStyle Style,
Expr *Init) {
Expr *Init, bool IsCPlusPlus20) {
if (!Init)
return true;
if (ParenListExpr *PLE = dyn_cast<ParenListExpr>(Init))
return PLE->getNumExprs() == 0;
return IsCPlusPlus20 || PLE->getNumExprs() == 0;
if (isa<ImplicitValueInitExpr>(Init))
return true;
else if (CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(Init))
Expand Down Expand Up @@ -2406,7 +2406,8 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
// Array 'new' can't have any initializers except empty parentheses.
// Initializer lists are also allowed, in C++11. Rely on the parser for the
// dialect distinction.
if (ArraySize && !isLegalArrayNewInitializer(InitStyle, Initializer)) {
if (ArraySize && !isLegalArrayNewInitializer(InitStyle, Initializer,
getLangOpts().CPlusPlus20)) {
SourceRange InitRange(Exprs.front()->getBeginLoc(),
Exprs.back()->getEndLoc());
Diag(StartLoc, diag::err_new_array_init_args) << InitRange;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5496,7 +5496,7 @@ static void TryOrBuildParenListInitialization(
return;
}
// ...and value-initialized for each k < i <= n;
if (ArrayLength > Args.size()) {
if (ArrayLength > Args.size() || Entity.isVariableLengthArrayNew()) {
InitializedEntity SubEntity = InitializedEntity::InitializeElement(
S.getASTContext(), Args.size(), Entity);
InitializationKind SubKind = InitializationKind::CreateValue(
Expand Down
57 changes: 57 additions & 0 deletions clang/test/CodeGen/paren-list-agg-init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,3 +513,60 @@ namespace gh61567 {
I(0);
}
}

namespace gh68198 {
// CHECK: define {{.*}} void @{{.*foo25.*}} {
// CHECK-NEXT: entry
// CHECK-NEXT: [[ARR_8:%.*arr8.*]] = alloca ptr, align 8
// CHECK-NEXT: [[CALL_PTR:%.*]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 8)
// CHECK-NEXT: store i32 1, ptr [[CALL_PTR]], align 4
// CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds i32, ptr [[CALL_PTR]], i64 1
// CHECK-NEXT: store i32 2, ptr [[ARRAY_EXP_NEXT]], align 4
// CHECK-NEXT: [[ARRAY_EXP_NEXT1:%.*]] = getelementptr inbounds i32, ptr [[ARRAY_EXP_NEXT]], i64 1
// CHECK-NEXT: store ptr [[CALL_PTR]], ptr %arr8, align 8
// CHECK-NEXT: ret void
void foo25() {
int* arr8 = new int[](1, 2);
}

// CHECK: define {{.*}} void @{{.*foo26.*}} {
// CHECK-NEXT: entry
// CHECK-NEXT: [[ARR_10:%.*arr9.*]] = alloca ptr, align 8
// CHECK-NEXT: [[CALL_PTR]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 16)
// CHECK-NEXT: [[ARRAYINIT_BEGIN:%.*]] = getelementptr inbounds [2 x i32], ptr [[CALL]], i64 0, i64 0
// CHECK-NEXT: store i32 1, ptr [[ARRAYINIT_BEGIN]], align 4
// CHECK-NEXT: [[ARRAYINIT_ELEMENT:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN]], i64 1
// CHECK-NEXT: store i32 2, ptr [[ARRAYINIT_ELEMENT]], align 4
// CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds [2 x i32], ptr %call, i64 1
// CHECK-NEXT: [[ARRAYINIT_BEGIN1:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 0, i64 0
// CHECK-NEXT: store i32 3, ptr [[ARRAYINIT_BEGIN1]], align 4
// CHECK-NEXT: [[ARRAYINIT_ELEMENT2:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN1]], i64 1
// CHECK-NEXT: store i32 4, ptr [[ARRAYINIT_ELEMENT2]], align 4
// CHECK-NEXT: [[ARRAY_EXP_NEXT3:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 1
// CHECK-NEXT: store ptr [[CALL_PTR]], ptr [[ARR_10]], align 8
// CHECK-NEXT: ret void
void foo26() {
void* arr9 = new int[][2]({1, 2}, {3, 4});
}

// CHECK: define {{.*}} void @{{.*foo27.*}} {
// CHECK-NEXT: entry
// CHECK-NEXT: [[ARR_10:%.*arr10.*]] = alloca ptr, align 8
// CHECK-NEXT: [[CALL_PTR]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 32)
// CHECK-NEXT: [[ARRAYINIT_BEGIN:%.*]] = getelementptr inbounds [2 x i32], ptr [[CALL]], i64 0, i64 0
// CHECK-NEXT: store i32 5, ptr [[ARRAYINIT_BEGIN]], align 4
// CHECK-NEXT: [[ARRAYINIT_ELEMENT:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN]], i64 1
// CHECK-NEXT: store i32 6, ptr [[ARRAYINIT_ELEMENT]], align 4
// CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds [2 x i32], ptr %call, i64 1
// CHECK-NEXT: [[ARRAYINIT_BEGIN1:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 0, i64 0
// CHECK-NEXT: store i32 7, ptr [[ARRAYINIT_BEGIN1]], align 4
// CHECK-NEXT: [[ARRAYINIT_ELEMENT2:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN1]], i64 1
// CHECK-NEXT: store i32 8, ptr [[ARRAYINIT_ELEMENT2]], align 4
// CHECK-NEXT: [[ARRAY_EXP_NEXT3:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 1
// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[ARRAY_EXP_NEXT3]], i8 0, i64 16, i1 false)
// CHECK-NEXT: store ptr [[CALL_PTR]], ptr [[ARR_10]], align 8
// CHECK-NEXT: ret void
void foo27() {
void* arr10 = new int[4][2]({5, 6}, {7, 8});
}
}
94 changes: 93 additions & 1 deletion clang/test/CodeGenCXX/new-array-init.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -std=c++11 -triple i386-unknown-unknown %s -emit-llvm -o - | FileCheck %s
// RUN: %clang_cc1 -std=c++20 -triple i386-unknown-unknown %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKCXX20
// RUN: %clang_cc1 -std=c++11 -triple i386-unknown-unknown %s -emit-llvm -fsanitize=signed-integer-overflow -o - | FileCheck --check-prefix=SIO %s

// CHECK: @[[ABC4:.*]] = {{.*}} constant [4 x i8] c"abc\00"
Expand All @@ -15,20 +16,51 @@ void fn(int n) {
new int[n] { 1, 2, 3 };
}

#if __cplusplus >= 202002L
// CHECKCXX20-LABEL: define{{.*}} void @_Z8fn_pareni
void fn_paren(int n) {
// CHECKCXX20: icmp ult i{{32|64}} %{{[^ ]+}}, 3
// CHECKCXX20: store i32 1
// CHECKCXX20: store i32 2
// CHECKCXX20: store i32 3
// CHECKCXX20: sub {{.*}}, 12
// CHECKCXX20: call void @llvm.memset
new int[n](1, 2, 3);
}
#endif

// CHECK-LABEL: define{{.*}} void @_Z11const_exactv
void const_exact() {
// CHECK-NOT: icmp ult i{{32|64}} %{{[^ ]+}}, 3
// CHECK-NOT: icmp eq ptr
new int[3] { 1, 2, 3 };
}

#if __cplusplus >= 202002L
// CHECKCXX20-LABEL: define{{.*}} void @_Z17const_exact_parenv
void const_exact_paren() {
// CHECKCXX20-NOT: icmp ult i{{32|64}} %{{[^ ]+}}, 3
// CHECKCXX20-NOT: icmp eq ptr
new int[3](1, 2, 3);
}
#endif

// CHECK-LABEL: define{{.*}} void @_Z16const_sufficientv
void const_sufficient() {
// CHECK-NOT: icmp ult i{{32|64}} %{{[^ ]+}}, 3
new int[4] { 1, 2, 3 };
// CHECK: ret void
}

#if __cplusplus >= 202002L
// CHECKCXX20-LABEL: define{{.*}} void @_Z22const_sufficient_parenv
void const_sufficient_paren() {
// CHECKCXX20-NOT: icmp ult i{{32|64}} %{{[^ ]+}}, 3
new int[4](1, 2, 3);
// CHECKCXX20: ret void
}
#endif

// CHECK-LABEL: define{{.*}} void @_Z22check_array_value_initv
void check_array_value_init() {
struct S;
Expand All @@ -46,7 +78,7 @@ void check_array_value_init() {

// CHECK-LABEL: define{{.*}} void @_Z15string_nonconsti
void string_nonconst(int n) {
// CHECK: icmp slt i{{32|64}} %{{[^ ]+}}, 4
// CHECK: icmp {{s|u}}lt i{{32|64}} %{{[^ ]+}}, 4
// FIXME: Conditionally throw an exception rather than passing -1 to alloc function
// CHECK: select
// CHECK: %[[PTR:.*]] = call noalias noundef nonnull ptr @_Zna{{.}}(i{{32|64}}
Expand All @@ -57,6 +89,34 @@ void string_nonconst(int n) {
new char[n] { "abc" };
}

#if __cplusplus >= 202002L
// CHECKCXX20-LABEL: define{{.*}} void @_Z21string_nonconst_pareni
void string_nonconst_paren(int n) {
// CHECKCXX20: icmp {{s|u}}lt i{{32|64}} %{{[^ ]+}}, 4
// FIXME: Conditionally throw an exception rather than passing -1 to alloc function
// CHECKCXX20: select
// CHECKCXX20: %[[PTR:.*]] = call noalias noundef nonnull ptr @_Zna{{.}}(i{{32|64}}
// CHECKCXX20: call void @llvm.memcpy{{.*}}(ptr align {{[0-9]+}} %[[PTR]], ptr align {{[0-9]+}} @[[ABC4]], i32 4,
// CHECKCXX20: %[[REST:.*]] = getelementptr inbounds i8, ptr %[[PTR]], i32 4
// CHECKCXX20: %[[RESTSIZE:.*]] = sub {{.*}}, 4
// CHECKCXX20: call void @llvm.memset{{.*}}(ptr align {{[0-9]+}} %[[REST]], i8 0, i{{32|64}} %[[RESTSIZE]],
new char[n]("abc");
}

// CHECKCXX20-LABEL: define{{.*}} void @_Z33string_nonconst_paren_extra_pareni
void string_nonconst_paren_extra_paren(int n) {
// CHECKCXX20: icmp {{s|u}}lt i{{32|64}} %{{[^ ]+}}, 4
// FIXME: Conditionally throw an exception rather than passing -1 to alloc function
// CHECKCXX20: select
// CHECKCXX20: %[[PTR:.*]] = call noalias noundef nonnull ptr @_Zna{{.}}(i{{32|64}}
// CHECKCXX20: call void @llvm.memcpy{{.*}}(ptr align {{[0-9]+}} %[[PTR]], ptr align {{[0-9]+}} @[[ABC4]], i32 4,
// CHECKCXX20: %[[REST:.*]] = getelementptr inbounds i8, ptr %[[PTR]], i32 4
// CHECKCXX20: %[[RESTSIZE:.*]] = sub {{.*}}, 4
// CHECKCXX20: call void @llvm.memset{{.*}}(ptr align {{[0-9]+}} %[[REST]], i8 0, i{{32|64}} %[[RESTSIZE]],
new char[n](("abc"));
}
#endif

// CHECK-LABEL: define{{.*}} void @_Z12string_exactv
void string_exact() {
// CHECK-NOT: icmp
Expand All @@ -66,6 +126,26 @@ void string_exact() {
new char[4] { "abc" };
}

#if __cplusplus >= 202002L
// CHECKCXX20-LABEL: define{{.*}} void @_Z18string_exact_parenv
void string_exact_paren() {
// CHECKCXX20-NOT: icmp
// CHECKCXX20: %[[PTR:.*]] = call noalias noundef nonnull ptr @_Zna{{.}}(i{{32|64}} noundef 4)
// CHECKCXX20: call void @llvm.memcpy{{.*}}(ptr align {{[0-9]+}} %[[PTR]], ptr align {{[0-9]+}} @[[ABC4]], i32 4,
// CHECKCXX20-NOT: memset
new char[4]("abc");
}

// CHECKCXX20-LABEL: define{{.*}} void @_Z28string_exact_paren_extensionv
void string_exact_paren_extension() {
// CHECKCXX20-NOT: icmp
// CHECKCXX20: %[[PTR:.*]] = call noalias noundef nonnull ptr @_Zna{{.}}(i{{32|64}} noundef 4)
// CHECKCXX20: call void @llvm.memcpy{{.*}}(ptr align {{[0-9]+}} %[[PTR]], ptr align {{[0-9]+}} @[[ABC4]], i32 4,
// CHECKCXX20-NOT: memset
new char[4](__extension__ "abc");
}
#endif

// CHECK-LABEL: define{{.*}} void @_Z17string_sufficientv
void string_sufficient() {
// CHECK-NOT: icmp
Expand All @@ -76,6 +156,18 @@ void string_sufficient() {
new char[15] { "abc" };
}

#if __cplusplus >= 202002L
// CHECKCXX20-LABEL: define{{.*}} void @_Z23string_sufficient_parenv
void string_sufficient_paren() {
// CHECKCXX20-NOT: icmp
// CHECKCXX20: %[[PTR:.*]] = call noalias noundef nonnull ptr @_Zna{{.}}(i{{32|64}} noundef 15)
// FIXME: For very large arrays, it would be preferable to emit a small copy and a memset.
// CHECKCXX20: call void @llvm.memcpy{{.*}}(ptr align {{[0-9]+}} %[[PTR]], ptr align {{[0-9]+}} @[[ABC15]], i32 15,
// CHECKCXX20-NOT: memset
new char[15] { "abc" };
}
#endif

// CHECK-LABEL: define{{.*}} void @_Z10aggr_exactv
void aggr_exact() {
// CHECK-NOT: icmp
Expand Down

0 comments on commit 2c9f04c

Please sign in to comment.