Do a proper recursive lookup when deciding whether a class's usual
deallocation function has a two-argument form. Store the result of this
check in new[] and delete[] nodes.
Fixes rdar://problem/8913519
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@124373 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index b4e8a4e..7cd714b 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -111,7 +111,8 @@
SourceRange TypeIdParens, Expr *arraySize,
CXXConstructorDecl *constructor, bool initializer,
Expr **constructorArgs, unsigned numConsArgs,
- FunctionDecl *operatorDelete, QualType ty,
+ FunctionDecl *operatorDelete,
+ bool usualArrayDeleteWantsSize, QualType ty,
TypeSourceInfo *AllocatedTypeInfo,
SourceLocation startLoc, SourceLocation endLoc,
SourceLocation constructorLParen,
@@ -119,8 +120,9 @@
: Expr(CXXNewExprClass, ty, VK_RValue, OK_Ordinary,
ty->isDependentType(), ty->isDependentType(),
ty->containsUnexpandedParameterPack()),
- GlobalNew(globalNew),
- Initializer(initializer), SubExprs(0), OperatorNew(operatorNew),
+ GlobalNew(globalNew), Initializer(initializer),
+ UsualArrayDeleteWantsSize(usualArrayDeleteWantsSize),
+ SubExprs(0), OperatorNew(operatorNew),
OperatorDelete(operatorDelete), Constructor(constructor),
AllocatedTypeInfo(AllocatedTypeInfo), TypeIdParens(TypeIdParens),
StartLoc(startLoc), EndLoc(endLoc), ConstructorLParen(constructorLParen),
diff --git a/lib/CodeGen/CGCXXABI.cpp b/lib/CodeGen/CGCXXABI.cpp
index 627df43..7fb1ffd 100644
--- a/lib/CodeGen/CGCXXABI.cpp
+++ b/lib/CodeGen/CGCXXABI.cpp
@@ -142,13 +142,14 @@
CGF.EmitReturnOfRValue(RV, ResultType);
}
-CharUnits CGCXXABI::GetArrayCookieSize(QualType ElementType) {
+CharUnits CGCXXABI::GetArrayCookieSize(const CXXNewExpr *expr) {
return CharUnits::Zero();
}
llvm::Value *CGCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
llvm::Value *NewPtr,
llvm::Value *NumElements,
+ const CXXNewExpr *expr,
QualType ElementType) {
// Should never be called.
ErrorUnsupportedABI(CGF, "array cookie initialization");
@@ -156,7 +157,8 @@
}
void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
- QualType ElementType, llvm::Value *&NumElements,
+ const CXXDeleteExpr *expr, QualType ElementType,
+ llvm::Value *&NumElements,
llvm::Value *&AllocPtr, CharUnits &CookieSize) {
ErrorUnsupportedABI(CGF, "array cookie reading");
diff --git a/lib/CodeGen/CGCXXABI.h b/lib/CodeGen/CGCXXABI.h
index 8bc1385..547f98a 100644
--- a/lib/CodeGen/CGCXXABI.h
+++ b/lib/CodeGen/CGCXXABI.h
@@ -189,7 +189,7 @@
///
/// \param ElementType - the allocated type of the expression,
/// i.e. the pointee type of the expression result type
- virtual CharUnits GetArrayCookieSize(QualType ElementType);
+ virtual CharUnits GetArrayCookieSize(const CXXNewExpr *expr);
/// Initialize the array cookie for the given allocation.
///
@@ -202,6 +202,7 @@
virtual llvm::Value *InitializeArrayCookie(CodeGenFunction &CGF,
llvm::Value *NewPtr,
llvm::Value *NumElements,
+ const CXXNewExpr *expr,
QualType ElementType);
/// Reads the array cookie associated with the given pointer,
@@ -218,6 +219,7 @@
/// \param CookieSize - an out parameter which will be initialized
/// with the size of the cookie, or zero if there is no cookie
virtual void ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
+ const CXXDeleteExpr *expr,
QualType ElementType, llvm::Value *&NumElements,
llvm::Value *&AllocPtr, CharUnits &CookieSize);
diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index 7177a64..4f1d176 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -395,7 +395,7 @@
if (IsPlacementOperatorNewArray(CGF.getContext(), OperatorNew))
return CharUnits::Zero();
- return CGF.CGM.getCXXABI().GetArrayCookieSize(E->getAllocatedType());
+ return CGF.CGM.getCXXABI().GetArrayCookieSize(E);
}
static llvm::Value *EmitCXXNewAllocSize(ASTContext &Context,
@@ -1065,7 +1065,7 @@
if (AllocSize != AllocSizeWithoutCookie) {
assert(E->isArray());
NewPtr = CGM.getCXXABI().InitializeArrayCookie(CGF, NewPtr, NumElements,
- AllocType);
+ E, AllocType);
}
// If there's an operator delete, enter a cleanup to call it if an
@@ -1271,18 +1271,19 @@
/// Emit the code for deleting an array of objects.
static void EmitArrayDelete(CodeGenFunction &CGF,
- const FunctionDecl *OperatorDelete,
+ const CXXDeleteExpr *E,
llvm::Value *Ptr,
QualType ElementType) {
llvm::Value *NumElements = 0;
llvm::Value *AllocatedPtr = 0;
CharUnits CookieSize;
- CGF.CGM.getCXXABI().ReadArrayCookie(CGF, Ptr, ElementType,
+ CGF.CGM.getCXXABI().ReadArrayCookie(CGF, Ptr, E, ElementType,
NumElements, AllocatedPtr, CookieSize);
assert(AllocatedPtr && "ReadArrayCookie didn't set AllocatedPtr");
// Make sure that we call delete even if one of the dtors throws.
+ const FunctionDecl *OperatorDelete = E->getOperatorDelete();
CGF.EHStack.pushCleanup<CallArrayDelete>(NormalAndEHCleanup,
AllocatedPtr, OperatorDelete,
NumElements, ElementType,
@@ -1352,7 +1353,7 @@
cast<llvm::PointerType>(Ptr->getType())->getElementType());
if (E->isArrayForm()) {
- EmitArrayDelete(*this, E->getOperatorDelete(), Ptr, DeleteTy);
+ EmitArrayDelete(*this, E, Ptr, DeleteTy);
} else {
EmitObjectDelete(*this, E->getOperatorDelete(), Ptr, DeleteTy);
}
diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index edc03b1..ef5455c 100644
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -47,7 +47,9 @@
return PtrDiffTy;
}
- bool NeedsArrayCookie(QualType ElementType);
+ bool NeedsArrayCookie(const CXXNewExpr *expr);
+ bool NeedsArrayCookie(const CXXDeleteExpr *expr,
+ QualType elementType);
public:
ItaniumCXXABI(CodeGen::CodeGenModule &CGM, bool IsARM = false) :
@@ -105,12 +107,14 @@
void EmitInstanceFunctionProlog(CodeGenFunction &CGF);
- CharUnits GetArrayCookieSize(QualType ElementType);
+ CharUnits GetArrayCookieSize(const CXXNewExpr *expr);
llvm::Value *InitializeArrayCookie(CodeGenFunction &CGF,
llvm::Value *NewPtr,
llvm::Value *NumElements,
+ const CXXNewExpr *expr,
QualType ElementType);
void ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
+ const CXXDeleteExpr *expr,
QualType ElementType, llvm::Value *&NumElements,
llvm::Value *&AllocPtr, CharUnits &CookieSize);
@@ -140,12 +144,14 @@
void EmitReturnFromThunk(CodeGenFunction &CGF, RValue RV, QualType ResTy);
- CharUnits GetArrayCookieSize(QualType ElementType);
+ CharUnits GetArrayCookieSize(const CXXNewExpr *expr);
llvm::Value *InitializeArrayCookie(CodeGenFunction &CGF,
llvm::Value *NewPtr,
llvm::Value *NumElements,
+ const CXXNewExpr *expr,
QualType ElementType);
void ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
+ const CXXDeleteExpr *expr,
QualType ElementType, llvm::Value *&NumElements,
llvm::Value *&AllocPtr, CharUnits &CookieSize);
@@ -794,67 +800,49 @@
/************************** Array allocation cookies **************************/
-bool ItaniumCXXABI::NeedsArrayCookie(QualType ElementType) {
- ElementType = getContext().getBaseElementType(ElementType);
- const RecordType *RT = ElementType->getAs<RecordType>();
- if (!RT) return false;
-
- const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
-
- // If the class has a non-trivial destructor, it always needs a cookie.
- if (!RD->hasTrivialDestructor()) return true;
-
+bool ItaniumCXXABI::NeedsArrayCookie(const CXXNewExpr *expr) {
// If the class's usual deallocation function takes two arguments,
- // it needs a cookie. Otherwise we don't need a cookie.
- const CXXMethodDecl *UsualDeallocationFunction = 0;
+ // it needs a cookie.
+ if (expr->doesUsualArrayDeleteWantSize())
+ return true;
- // Usual deallocation functions of this form are always found on the
- // class.
- //
- // FIXME: what exactly is this code supposed to do if there's an
- // ambiguity? That's possible with using declarations.
- DeclarationName OpName =
- getContext().DeclarationNames.getCXXOperatorName(OO_Array_Delete);
- DeclContext::lookup_const_iterator Op, OpEnd;
- for (llvm::tie(Op, OpEnd) = RD->lookup(OpName); Op != OpEnd; ++Op) {
- const CXXMethodDecl *Delete =
- cast<CXXMethodDecl>((*Op)->getUnderlyingDecl());
-
- if (Delete->isUsualDeallocationFunction()) {
- UsualDeallocationFunction = Delete;
- break;
- }
- }
-
- // No usual deallocation function, we don't need a cookie.
- if (!UsualDeallocationFunction)
- return false;
-
- // The usual deallocation function doesn't take a size_t argument,
- // so we don't need a cookie.
- if (UsualDeallocationFunction->getNumParams() == 1)
- return false;
-
- assert(UsualDeallocationFunction->getNumParams() == 2 &&
- "Unexpected deallocation function type!");
- return true;
+ // Otherwise, if the class has a non-trivial destructor, it always
+ // needs a cookie.
+ const CXXRecordDecl *record =
+ expr->getAllocatedType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
+ return (record && !record->hasTrivialDestructor());
}
-CharUnits ItaniumCXXABI::GetArrayCookieSize(QualType ElementType) {
- if (!NeedsArrayCookie(ElementType))
+bool ItaniumCXXABI::NeedsArrayCookie(const CXXDeleteExpr *expr,
+ QualType elementType) {
+ // If the class's usual deallocation function takes two arguments,
+ // it needs a cookie.
+ if (expr->doesUsualArrayDeleteWantSize())
+ return true;
+
+ // Otherwise, if the class has a non-trivial destructor, it always
+ // needs a cookie.
+ const CXXRecordDecl *record =
+ elementType->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
+ return (record && !record->hasTrivialDestructor());
+}
+
+CharUnits ItaniumCXXABI::GetArrayCookieSize(const CXXNewExpr *expr) {
+ if (!NeedsArrayCookie(expr))
return CharUnits::Zero();
- // Padding is the maximum of sizeof(size_t) and alignof(ElementType)
+ // Padding is the maximum of sizeof(size_t) and alignof(elementType)
ASTContext &Ctx = getContext();
return std::max(Ctx.getTypeSizeInChars(Ctx.getSizeType()),
- Ctx.getTypeAlignInChars(ElementType));
+ Ctx.getTypeAlignInChars(expr->getAllocatedType()));
}
llvm::Value *ItaniumCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
llvm::Value *NewPtr,
llvm::Value *NumElements,
+ const CXXNewExpr *expr,
QualType ElementType) {
- assert(NeedsArrayCookie(ElementType));
+ assert(NeedsArrayCookie(expr));
unsigned AS = cast<llvm::PointerType>(NewPtr->getType())->getAddressSpace();
@@ -887,6 +875,7 @@
void ItaniumCXXABI::ReadArrayCookie(CodeGenFunction &CGF,
llvm::Value *Ptr,
+ const CXXDeleteExpr *expr,
QualType ElementType,
llvm::Value *&NumElements,
llvm::Value *&AllocPtr,
@@ -896,7 +885,7 @@
const llvm::Type *CharPtrTy = CGF.Builder.getInt8Ty()->getPointerTo(AS);
// If we don't need an array cookie, bail out early.
- if (!NeedsArrayCookie(ElementType)) {
+ if (!NeedsArrayCookie(expr, ElementType)) {
AllocPtr = CGF.Builder.CreateBitCast(Ptr, CharPtrTy);
NumElements = 0;
CookieSize = CharUnits::Zero();
@@ -927,8 +916,8 @@
NumElements = CGF.Builder.CreateLoad(NumElementsPtr);
}
-CharUnits ARMCXXABI::GetArrayCookieSize(QualType ElementType) {
- if (!NeedsArrayCookie(ElementType))
+CharUnits ARMCXXABI::GetArrayCookieSize(const CXXNewExpr *expr) {
+ if (!NeedsArrayCookie(expr))
return CharUnits::Zero();
// On ARM, the cookie is always:
@@ -944,8 +933,9 @@
llvm::Value *ARMCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
llvm::Value *NewPtr,
llvm::Value *NumElements,
+ const CXXNewExpr *expr,
QualType ElementType) {
- assert(NeedsArrayCookie(ElementType));
+ assert(NeedsArrayCookie(expr));
// NewPtr is a char*.
@@ -978,6 +968,7 @@
void ARMCXXABI::ReadArrayCookie(CodeGenFunction &CGF,
llvm::Value *Ptr,
+ const CXXDeleteExpr *expr,
QualType ElementType,
llvm::Value *&NumElements,
llvm::Value *&AllocPtr,
@@ -987,7 +978,7 @@
const llvm::Type *CharPtrTy = CGF.Builder.getInt8Ty()->getPointerTo(AS);
// If we don't need an array cookie, bail out early.
- if (!NeedsArrayCookie(ElementType)) {
+ if (!NeedsArrayCookie(expr, ElementType)) {
AllocPtr = CGF.Builder.CreateBitCast(Ptr, CharPtrTy);
NumElements = 0;
CookieSize = CharUnits::Zero();
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 53679d3..8581c6f 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -664,6 +664,61 @@
return move(Result);
}
+/// doesUsualArrayDeleteWantSize - Answers whether the usual
+/// operator delete[] for the given type has a size_t parameter.
+static bool doesUsualArrayDeleteWantSize(Sema &S, SourceLocation loc,
+ QualType allocType) {
+ const RecordType *record =
+ allocType->getBaseElementTypeUnsafe()->getAs<RecordType>();
+ if (!record) return false;
+
+ // Try to find an operator delete[] in class scope.
+
+ DeclarationName deleteName =
+ S.Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete);
+ LookupResult ops(S, deleteName, loc, Sema::LookupOrdinaryName);
+ S.LookupQualifiedName(ops, record->getDecl());
+
+ // We're just doing this for information.
+ ops.suppressDiagnostics();
+
+ // Very likely: there's no operator delete[].
+ if (ops.empty()) return false;
+
+ // If it's ambiguous, it should be illegal to call operator delete[]
+ // on this thing, so it doesn't matter if we allocate extra space or not.
+ if (ops.isAmbiguous()) return false;
+
+ LookupResult::Filter filter = ops.makeFilter();
+ while (filter.hasNext()) {
+ NamedDecl *del = filter.next()->getUnderlyingDecl();
+
+ // C++0x [basic.stc.dynamic.deallocation]p2:
+ // A template instance is never a usual deallocation function,
+ // regardless of its signature.
+ if (isa<FunctionTemplateDecl>(del)) {
+ filter.erase();
+ continue;
+ }
+
+ // C++0x [basic.stc.dynamic.deallocation]p2:
+ // If class T does not declare [an operator delete[] with one
+ // parameter] but does declare a member deallocation function
+ // named operator delete[] with exactly two parameters, the
+ // second of which has type std::size_t, then this function
+ // is a usual deallocation function.
+ if (!cast<CXXMethodDecl>(del)->isUsualDeallocationFunction()) {
+ filter.erase();
+ continue;
+ }
+ }
+ filter.done();
+
+ if (!ops.isSingleResult()) return false;
+
+ const FunctionDecl *del = cast<FunctionDecl>(ops.getFoundDecl());
+ return (del->getNumParams() == 2);
+}
/// ActOnCXXNew - Parsed a C++ 'new' expression (C++ 5.3.4), as in e.g.:
/// @code new (memory) int[size][4] @endcode
@@ -839,6 +894,14 @@
UseGlobal, AllocType, ArraySize, PlaceArgs,
NumPlaceArgs, OperatorNew, OperatorDelete))
return ExprError();
+
+ // If this is an array allocation, compute whether the usual array
+ // deallocation function for the type has a size_t parameter.
+ bool UsualArrayDeleteWantsSize = false;
+ if (ArraySize && !AllocType->isDependentType())
+ UsualArrayDeleteWantsSize
+ = doesUsualArrayDeleteWantSize(*this, StartLoc, AllocType);
+
llvm::SmallVector<Expr *, 8> AllPlaceArgs;
if (OperatorNew) {
// Add default arguments, if any.
@@ -938,6 +1001,7 @@
PlaceArgs, NumPlaceArgs, TypeIdParens,
ArraySize, Constructor, Init,
ConsArgs, NumConsArgs, OperatorDelete,
+ UsualArrayDeleteWantsSize,
ResultType, AllocTypeInfo,
StartLoc,
Init ? ConstructorRParen :
@@ -1492,6 +1556,7 @@
FunctionDecl *OperatorDelete = 0;
bool ArrayFormAsWritten = ArrayForm;
+ bool UsualArrayDeleteWantsSize = false;
if (!Ex->isTypeDependent()) {
QualType Type = Ex->getType();
@@ -1587,6 +1652,21 @@
FindDeallocationFunction(StartLoc, RD, DeleteName, OperatorDelete))
return ExprError();
+ // If we're allocating an array of records, check whether the
+ // usual operator delete[] has a size_t parameter.
+ if (ArrayForm) {
+ // If the user specifically asked to use the global allocator,
+ // we'll need to do the lookup into the class.
+ if (UseGlobal)
+ UsualArrayDeleteWantsSize =
+ doesUsualArrayDeleteWantSize(*this, StartLoc, PointeeElem);
+
+ // Otherwise, the usual operator delete[] should be the
+ // function we just found.
+ else if (isa<CXXMethodDecl>(OperatorDelete))
+ UsualArrayDeleteWantsSize = (OperatorDelete->getNumParams() == 2);
+ }
+
if (!RD->hasTrivialDestructor())
if (CXXDestructorDecl *Dtor = LookupDestructor(RD)) {
MarkDeclarationReferenced(StartLoc,
@@ -1606,13 +1686,14 @@
}
MarkDeclarationReferenced(StartLoc, OperatorDelete);
-
+
// FIXME: Check access and ambiguity of operator delete and destructor.
}
return Owned(new (Context) CXXDeleteExpr(Context.VoidTy, UseGlobal, ArrayForm,
- ArrayFormAsWritten, OperatorDelete,
- Ex, StartLoc));
+ ArrayFormAsWritten,
+ UsualArrayDeleteWantsSize,
+ OperatorDelete, Ex, StartLoc));
}
/// \brief Check the use of the given variable as a C++ condition in an if,
diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index 5fec238..79b68e6 100644
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -1106,8 +1106,9 @@
void ASTStmtReader::VisitCXXNewExpr(CXXNewExpr *E) {
VisitExpr(E);
- E->setGlobalNew(Record[Idx++]);
- E->setHasInitializer(Record[Idx++]);
+ E->GlobalNew = Record[Idx++];
+ E->Initializer = Record[Idx++];
+ E->UsualArrayDeleteWantsSize = Record[Idx++];
bool isArray = Record[Idx++];
unsigned NumPlacementArgs = Record[Idx++];
unsigned NumCtorArgs = Record[Idx++];
@@ -1140,6 +1141,7 @@
E->GlobalDelete = Record[Idx++];
E->ArrayForm = Record[Idx++];
E->ArrayFormAsWritten = Record[Idx++];
+ E->UsualArrayDeleteWantsSize = Record[Idx++];
E->OperatorDelete = cast_or_null<FunctionDecl>(Reader.GetDecl(Record[Idx++]));
E->Argument = Reader.ReadSubExpr();
E->Loc = ReadSourceLocation(Record, Idx);
diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index e03a780..3962439 100644
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -1101,6 +1101,7 @@
VisitExpr(E);
Record.push_back(E->isGlobalNew());
Record.push_back(E->hasInitializer());
+ Record.push_back(E->doesUsualArrayDeleteWantSize());
Record.push_back(E->isArray());
Record.push_back(E->getNumPlacementArgs());
Record.push_back(E->getNumConstructorArgs());
@@ -1125,6 +1126,7 @@
Record.push_back(E->isGlobalDelete());
Record.push_back(E->isArrayForm());
Record.push_back(E->isArrayFormAsWritten());
+ Record.push_back(E->doesUsualArrayDeleteWantSize());
Writer.AddDeclRef(E->getOperatorDelete(), Record);
Writer.AddStmt(E->getArgument());
Writer.AddSourceLocation(E->getSourceRange().getBegin(), Record);