Abstract IR generation of array cookies into the C++ ABI class and

implement ARM array cookies.  Also fix a few unfortunate bugs:
  - throwing dtors in deletes prevented the allocation from being deleted
  - adding the cookie to the new[] size was not being considered for
    overflow (and, more seriously, was screwing up the earlier checks)
  - deleting an array via a pointer to array of class type was not
    causing any destructors to be run and was passing the unadjusted
    pointer to the deallocator
  - lots of address-space problems, in case anyone wants to support
    free store in a variant address space :)



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@112814 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 859 insertions, 235 deletions

diff --git a/lib/CodeGen/CGCXX.cpp b/lib/CodeGen/CGCXX.cpp
index f8d81fa796..179716f631 100644
--- a/lib/CodeGen/CGCXX.cpp
+++ b/lib/CodeGen/CGCXX.cpp
@@ -327,7 +327,7 @@ static void ErrorUnsupportedABI(CodeGenFunction &CGF,
                                 llvm::StringRef S) {
   Diagnostic &Diags = CGF.CGM.getDiags();
   unsigned DiagID = Diags.getCustomDiagID(Diagnostic::Error,
-                                          "cannot yet compile %s in this ABI");
+                                          "cannot yet compile %1 in this ABI");
   Diags.Report(CGF.getContext().getFullLoc(CGF.CurCodeDecl->getLocation()),
                DiagID)
     << S;
@@ -445,3 +445,27 @@ void CGCXXABI::EmitReturnFromThunk(CodeGenFunction &CGF,
                                    RValue RV, QualType ResultType) {
   CGF.EmitReturnOfRValue(RV, ResultType);
 }
+
+CharUnits CGCXXABI::GetArrayCookieSize(QualType ElementType) {
+  return CharUnits::Zero();
+}
+
+llvm::Value *CGCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
+                                             llvm::Value *NewPtr,
+                                             llvm::Value *NumElements,
+                                             QualType ElementType) {
+  // Should never be called.
+  ErrorUnsupportedABI(CGF, "array cookie initialization");
+  return 0;
+}
+
+void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
+                               QualType ElementType, llvm::Value *&NumElements,
+                               llvm::Value *&AllocPtr, CharUnits &CookieSize) {
+  ErrorUnsupportedABI(CGF, "array cookie reading");
+
+  // This should be enough to avoid assertions.
+  NumElements = 0;
+  AllocPtr = llvm::Constant::getNullValue(CGF.Builder.getInt8PtrTy());
+  CookieSize = CharUnits::Zero();
+}
diff --git a/lib/CodeGen/CGCXXABI.h b/lib/CodeGen/CGCXXABI.h
index 8d3e70f0b3..367e345918 100644
--- a/lib/CodeGen/CGCXXABI.h
+++ b/lib/CodeGen/CGCXXABI.h
@@ -67,6 +67,8 @@ protected:
   /// for 'this' emitted by BuildThisParam.
   void EmitThisParam(CodeGenFunction &CGF);
 
+  ASTContext &getContext() const { return CGM.getContext(); }
+
 public:
 
   virtual ~CGCXXABI();
@@ -170,6 +172,51 @@ public:
 
   virtual void EmitReturnFromThunk(CodeGenFunction &CGF,
                                    RValue RV, QualType ResultType);
+
+  /**************************** Array cookies ******************************/
+
+  /// Returns the extra size required in order to store the array
+  /// cookie for the given type.  May return 0 to indicate that no
+  /// array cookie is required.
+  ///
+  /// Several cases are filtered out before this method is called:
+  ///   - non-array allocations never need a cookie
+  ///   - calls to ::operator new(size_t, void*) never need a cookie
+  ///
+  /// \param ElementType - the allocated type of the expression,
+  ///   i.e. the pointee type of the expression result type
+  virtual CharUnits GetArrayCookieSize(QualType ElementType);
+
+  /// Initialize the array cookie for the given allocation.
+  ///
+  /// \param NewPtr - a char* which is the presumed-non-null
+  ///   return value of the allocation function
+  /// \param NumElements - the computed number of elements,
+  ///   potentially collapsed from the multidimensional array case
+  /// \param ElementType - the base element allocated type,
+  ///   i.e. the allocated type after stripping all array types
+  virtual llvm::Value *InitializeArrayCookie(CodeGenFunction &CGF,
+                                             llvm::Value *NewPtr,
+                                             llvm::Value *NumElements,
+                                             QualType ElementType);
+
+  /// Reads the array cookie associated with the given pointer,
+  /// if it has one.
+  ///
+  /// \param Ptr - a pointer to the first element in the array
+  /// \param ElementType - the base element type of elements of the array
+  /// \param NumElements - an out parameter which will be initialized
+  ///   with the number of elements allocated, or zero if there is no
+  ///   cookie
+  /// \param AllocPtr - an out parameter which will be initialized
+  ///   with a char* pointing to the address returned by the allocation
+  ///   function
+  /// \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,
+                               QualType ElementType, llvm::Value *&NumElements,
+                               llvm::Value *&AllocPtr, CharUnits &CookieSize);
+
 };
 
 /// Creates an instance of a C++ ABI class.
diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index 1a95d4bb09..5e417605b1 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -305,52 +305,6 @@ CodeGenFunction::EmitCXXConstructExpr(llvm::Value *Dest,
   }
 }
 
-static CharUnits CalculateCookiePadding(ASTContext &Ctx, QualType ElementType) {
-  ElementType = Ctx.getBaseElementType(ElementType);
-  const RecordType *RT = ElementType->getAs<RecordType>();
-  if (!RT)
-    return CharUnits::Zero();
-  
-  const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
-  if (!RD)
-    return CharUnits::Zero();
-  
-  // Check if the class has a trivial destructor.
-  if (RD->hasTrivialDestructor()) {
-    // Check if the usual deallocation function takes two arguments.
-    const CXXMethodDecl *UsualDeallocationFunction = 0;
-    
-    DeclarationName OpName =
-      Ctx.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);
-
-      if (Delete->isUsualDeallocationFunction()) {
-        UsualDeallocationFunction = Delete;
-        break;
-      }
-    }
-    
-    // No usual deallocation function, we don't need a cookie.
-    if (!UsualDeallocationFunction)
-      return CharUnits::Zero();
-    
-    // The usual deallocation function doesn't take a size_t argument, so we
-    // don't need a cookie.
-    if (UsualDeallocationFunction->getNumParams() == 1)
-      return CharUnits::Zero();
-        
-    assert(UsualDeallocationFunction->getNumParams() == 2 && 
-           "Unexpected deallocation function type!");
-  }  
-  
-  // Padding is the maximum of sizeof(size_t) and alignof(ElementType)
-  return std::max(Ctx.getTypeSizeInChars(Ctx.getSizeType()),
-                  Ctx.getTypeAlignInChars(ElementType));
-}
-
 /// Check whether the given operator new[] is the global placement
 /// operator new[].
 static bool IsPlacementOperatorNewArray(ASTContext &Ctx,
@@ -369,17 +323,18 @@ static bool IsPlacementOperatorNewArray(ASTContext &Ctx,
   return (ParamType == Ctx.VoidPtrTy);
 }
 
-static CharUnits CalculateCookiePadding(ASTContext &Ctx, const CXXNewExpr *E) {
+static CharUnits CalculateCookiePadding(CodeGenFunction &CGF,
+                                        const CXXNewExpr *E) {
   if (!E->isArray())
     return CharUnits::Zero();
 
   // No cookie is required if the new operator being used is 
   // ::operator new[](size_t, void*).
   const FunctionDecl *OperatorNew = E->getOperatorNew();
-  if (IsPlacementOperatorNewArray(Ctx, OperatorNew))
+  if (IsPlacementOperatorNewArray(CGF.getContext(), OperatorNew))
     return CharUnits::Zero();
 
-  return CalculateCookiePadding(Ctx, E->getAllocatedType());
+  return CGF.CGM.getCXXABI().GetArrayCookieSize(E->getAllocatedType());
 }
 
 static llvm::Value *EmitCXXNewAllocSize(ASTContext &Context,
@@ -388,90 +343,166 @@ static llvm::Value *EmitCXXNewAllocSize(ASTContext &Context,
                                         llvm::Value *&NumElements,
                                         llvm::Value *&SizeWithoutCookie) {
   QualType ElemType = E->getAllocatedType();
+
+  const llvm::IntegerType *SizeTy =
+    cast<llvm::IntegerType>(CGF.ConvertType(CGF.getContext().getSizeType()));
   
+  CharUnits TypeSize = CGF.getContext().getTypeSizeInChars(ElemType);
+
   if (!E->isArray()) {
-    CharUnits TypeSize = CGF.getContext().getTypeSizeInChars(ElemType);
-    const llvm::Type *SizeTy = CGF.ConvertType(CGF.getContext().getSizeType());
     SizeWithoutCookie = llvm::ConstantInt::get(SizeTy, TypeSize.getQuantity());
     return SizeWithoutCookie;
   }
 
+  // Figure out the cookie size.
+  CharUnits CookieSize = CalculateCookiePadding(CGF, E);
+
   // Emit the array size expression.
   // We multiply the size of all dimensions for NumElements.
   // e.g for 'int[2][3]', ElemType is 'int' and NumElements is 6.
   NumElements = CGF.EmitScalarExpr(E->getArraySize());
+  assert(NumElements->getType() == SizeTy && "element count not a size_t");
+
+  uint64_t ArraySizeMultiplier = 1;
   while (const ConstantArrayType *CAT
              = CGF.getContext().getAsConstantArrayType(ElemType)) {
     ElemType = CAT->getElementType();
-    llvm::Value *ArraySize
-        = llvm::ConstantInt::get(CGF.CGM.getLLVMContext(), CAT->getSize());
-    NumElements = CGF.Builder.CreateMul(NumElements, ArraySize);
+    ArraySizeMultiplier *= CAT->getSize().getZExtValue();
   }
 
-  CharUnits TypeSize = CGF.getContext().getTypeSizeInChars(ElemType);
-  const llvm::Type *SizeTy = CGF.ConvertType(CGF.getContext().getSizeType());
-  llvm::Value *Size = llvm::ConstantInt::get(SizeTy, TypeSize.getQuantity());
+  llvm::Value *Size;
   
   // If someone is doing 'new int[42]' there is no need to do a dynamic check.
   // Don't bloat the -O0 code.
   if (llvm::ConstantInt *NumElementsC =
         dyn_cast<llvm::ConstantInt>(NumElements)) {
-    // Determine if there is an overflow here by doing an extended multiply.
     llvm::APInt NEC = NumElementsC->getValue();
-    NEC.zext(NEC.getBitWidth()*2);
-    
-    llvm::APInt SC = cast<llvm::ConstantInt>(Size)->getValue();
-    SC.zext(SC.getBitWidth()*2);
+    unsigned SizeWidth = NEC.getBitWidth();
+
+    // Determine if there is an overflow here by doing an extended multiply.
+    NEC.zext(SizeWidth*2);
+    llvm::APInt SC(SizeWidth*2, TypeSize.getQuantity());
     SC *= NEC;
+
+    if (!CookieSize.isZero()) {
+      // Save the current size without a cookie.  We don't care if an
+      // overflow's already happened because SizeWithoutCookie isn't
+      // used if the allocator returns null or throws, as it should
+      // always do on an overflow.
+      llvm::APInt SWC = SC;
+      SWC.trunc(SizeWidth);
+      SizeWithoutCookie = llvm::ConstantInt::get(SizeTy, SWC);
+
+      // Add the cookie size.
+      SC += llvm::APInt(SizeWidth*2, CookieSize.getQuantity());
+    }
     
-    if (SC.countLeadingZeros() >= NumElementsC->getValue().getBitWidth()) {
-      SC.trunc(NumElementsC->getValue().getBitWidth());
-      Size = llvm::ConstantInt::get(Size->getContext(), SC);
+    if (SC.countLeadingZeros() >= SizeWidth) {
+      SC.trunc(SizeWidth);
+      Size = llvm::ConstantInt::get(SizeTy, SC);
     } else {
       // On overflow, produce a -1 so operator new throws.
-      Size = llvm::Constant::getAllOnesValue(Size->getType());
+      Size = llvm::Constant::getAllOnesValue(SizeTy);
     }
-    
+
+    // Scale NumElements while we're at it.
+    uint64_t N = NEC.getZExtValue() * ArraySizeMultiplier;
+    NumElements = llvm::ConstantInt::get(SizeTy, N);
+
+  // Otherwise, we don't need to do an overflow-checked multiplication if
+  // we're multiplying by one.
+  } else if (TypeSize.isOne()) {
+    assert(ArraySizeMultiplier == 1);
+
+    Size = NumElements;
+
+    // If we need a cookie, add its size in with an overflow check.
+    // This is maybe a little paranoid.
+    if (!CookieSize.isZero()) {
+      SizeWithoutCookie = Size;
+
+      llvm::Value *CookieSizeV
+        = llvm::ConstantInt::get(SizeTy, CookieSize.getQuantity());
+
+      const llvm::Type *Types[] = { SizeTy };
+      llvm::Value *UAddF
+        = CGF.CGM.getIntrinsic(llvm::Intrinsic::uadd_with_overflow, Types, 1);
+      llvm::Value *AddRes
+        = CGF.Builder.CreateCall2(UAddF, Size, CookieSizeV);
+
+      Size = CGF.Builder.CreateExtractValue(AddRes, 0);
+      llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(AddRes, 1);
+      Size = CGF.Builder.CreateSelect(DidOverflow,
+                                      llvm::ConstantInt::get(SizeTy, -1),
+                                      Size);
+    }
+
+  // Otherwise use the int.umul.with.overflow intrinsic.
   } else {
-    // Multiply with the type size.  This multiply can overflow, e.g. in:
-    //   new double[n]
-    // where n is 2^30 on a 32-bit machine or 2^62 on a 64-bit machine.  Because
-    // of this, we need to detect the overflow and ensure that an exception is
-    // called by forcing the size to -1 on overflow.
-    llvm::Value *UMulF =
-      CGF.CGM.getIntrinsic(llvm::Intrinsic::umul_with_overflow, &SizeTy, 1);
-    llvm::Value *MulRes = CGF.Builder.CreateCall2(UMulF, NumElements, Size);
-    // Branch on the overflow bit to the overflow block, which is lazily
-    // created.
+    llvm::Value *OutermostElementSize
+      = llvm::ConstantInt::get(SizeTy, TypeSize.getQuantity());
+
+    llvm::Value *NumOutermostElements = NumElements;
+
+    // Scale NumElements by the array size multiplier.  This might
+    // overflow, but only if the multiplication below also overflows,
+    // in which case this multiplication isn't used.
+    if (ArraySizeMultiplier != 1)
+      NumElements = CGF.Builder.CreateMul(NumElements,
+                         llvm::ConstantInt::get(SizeTy, ArraySizeMultiplier));
+
+    // The requested size of the outermost array is non-constant.
+    // Multiply that by the static size of the elements of that array;
+    // on unsigned overflow, set the size to -1 to trigger an
+    // exception from the allocation routine.  This is sufficient to
+    // prevent buffer overruns from the allocator returning a
+    // seemingly valid pointer to insufficient space.  This idea comes
+    // originally from MSVC, and GCC has an open bug requesting
+    // similar behavior:
+    //   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19351
+    //
+    // This will not be sufficient for C++0x, which requires a
+    // specific exception class (std::bad_array_new_length).
+    // That will require ABI support that has not yet been specified.
+    const llvm::Type *Types[] = { SizeTy };
+    llvm::Value *UMulF
+      = CGF.CGM.getIntrinsic(llvm::Intrinsic::umul_with_overflow, Types, 1);
+    llvm::Value *MulRes = CGF.Builder.CreateCall2(UMulF, NumOutermostElements,
+                                                  OutermostElementSize);
+
+    // The overflow bit.
     llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(MulRes, 1);
-    // Get the normal result of the multiplication.
-    llvm::Value *V = CGF.Builder.CreateExtractValue(MulRes, 0);
-    
-    llvm::BasicBlock *NormalBB = CGF.createBasicBlock("no_overflow");
-    llvm::BasicBlock *OverflowBB = CGF.createBasicBlock("overflow");
-    
-    CGF.Builder.CreateCondBr(DidOverflow, OverflowBB, NormalBB);
 
-    llvm::BasicBlock *PrevBB = CGF.Builder.GetInsertBlock();
-    
-    // We just need the overflow block to build a PHI node.
-    CGF.EmitBlock(OverflowBB);
-    CGF.EmitBlock(NormalBB);
-    
-    llvm::PHINode *PN = CGF.Builder.CreatePHI(V->getType());
-    
-    PN->addIncoming(V, PrevBB);
-    PN->addIncoming(llvm::Constant::getAllOnesValue(V->getType()), OverflowBB);
-    Size = PN;
+    // The result of the multiplication.
+    Size = CGF.Builder.CreateExtractValue(MulRes, 0);
+
+    // If we have a cookie, we need to add that size in, too.
+    if (!CookieSize.isZero()) {
+      SizeWithoutCookie = Size;
+
+      llvm::Value *CookieSizeV
+        = llvm::ConstantInt::get(SizeTy, CookieSize.getQuantity());
+      llvm::Value *UAddF
+        = CGF.CGM.getIntrinsic(llvm::Intrinsic::uadd_with_overflow, Types, 1);
+      llvm::Value *AddRes
+        = CGF.Builder.CreateCall2(UAddF, SizeWithoutCookie, CookieSizeV);
+
+      Size = CGF.Builder.CreateExtractValue(AddRes, 0);
+
+      llvm::Value *AddDidOverflow = CGF.Builder.CreateExtractValue(AddRes, 1);
+      DidOverflow = CGF.Builder.CreateAnd(DidOverflow, AddDidOverflow);
+    }
+
+    Size = CGF.Builder.CreateSelect(DidOverflow,
+                                    llvm::ConstantInt::get(SizeTy, -1),
+                                    Size);
   }
-  SizeWithoutCookie = Size;
-  
-  // Add the cookie padding if necessary.
-  CharUnits CookiePadding = CalculateCookiePadding(CGF.getContext(), E);
-  if (!CookiePadding.isZero())
-    Size = CGF.Builder.CreateAdd(Size, 
-        llvm::ConstantInt::get(SizeTy, CookiePadding.getQuantity()));
-  
+
+  if (CookieSize.isZero())
+    SizeWithoutCookie = Size;
+  else
+    assert(SizeWithoutCookie && "didn't set SizeWithoutCookie?");
+
   return Size;
 }
 
@@ -633,6 +664,10 @@ static void EmitNewInitializer(CodeGenFunction &CGF, const CXXNewExpr *E,
 
 llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
   QualType AllocType = E->getAllocatedType();
+  if (AllocType->isArrayType())
+    while (const ArrayType *AType = getContext().getAsArrayType(AllocType))
+      AllocType = AType->getElementType();
+
   FunctionDecl *NewFD = E->getOperatorNew();
   const FunctionProtoType *NewFTy = NewFD->getType()->getAs<FunctionProtoType>();
 
@@ -694,112 +729,68 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
   bool NullCheckResult = NewFTy->hasEmptyExceptionSpec() &&
     !(AllocType->isPODType() && !E->hasInitializer());
 
-  llvm::BasicBlock *NewNull = 0;
+  llvm::BasicBlock *NullCheckSource = 0;
   llvm::BasicBlock *NewNotNull = 0;
   llvm::BasicBlock *NewEnd = 0;
 
   llvm::Value *NewPtr = RV.getScalarVal();
+  unsigned AS = cast<llvm::PointerType>(NewPtr->getType())->getAddressSpace();
 
   if (NullCheckResult) {
-    NewNull = createBasicBlock("new.null");
+    NullCheckSource = Builder.GetInsertBlock();
     NewNotNull = createBasicBlock("new.notnull");
     NewEnd = createBasicBlock("new.end");
 
-    llvm::Value *IsNull =
-      Builder.CreateICmpEQ(NewPtr,
-                           llvm::Constant::getNullValue(NewPtr->getType()),
-                           "isnull");
-
-    Builder.CreateCondBr(IsNull, NewNull, NewNotNull);
+    llvm::Value *IsNull = Builder.CreateIsNull(NewPtr, "new.isnull");
+    Builder.CreateCondBr(IsNull, NewEnd, NewNotNull);
     EmitBlock(NewNotNull);
   }
   
-  CharUnits CookiePadding = CalculateCookiePadding(getContext(), E);
-  if (!CookiePadding.isZero()) {
-    CharUnits CookieOffset = 
-      CookiePadding - getContext().getTypeSizeInChars(SizeTy);
-    
-    llvm::Value *NumElementsPtr = 
-      Builder.CreateConstInBoundsGEP1_64(NewPtr, CookieOffset.getQuantity());
-    
-    NumElementsPtr = Builder.CreateBitCast(NumElementsPtr, 
-                                           ConvertType(SizeTy)->getPointerTo());
-    Builder.CreateStore(NumElements, NumElementsPtr);
-
-    // Now add the padding to the new ptr.
-    NewPtr = Builder.CreateConstInBoundsGEP1_64(NewPtr, 
-                                                CookiePadding.getQuantity());
+  assert((AllocSize == AllocSizeWithoutCookie) ==
+         CalculateCookiePadding(*this, E).isZero());
+  if (AllocSize != AllocSizeWithoutCookie) {
+    assert(E->isArray());
+    NewPtr = CGM.getCXXABI().InitializeArrayCookie(CGF, NewPtr, NumElements,
+                                                   AllocType);
   }
-  
-  if (AllocType->isArrayType()) {
-    while (const ArrayType *AType = getContext().getAsArrayType(AllocType))
-      AllocType = AType->getElementType();
-    NewPtr = 
-      Builder.CreateBitCast(NewPtr, 
-                          ConvertType(getContext().getPointerType(AllocType)));
+
+  const llvm::Type *ElementPtrTy = ConvertType(AllocType)->getPointerTo(AS);
+  NewPtr = Builder.CreateBitCast(NewPtr, ElementPtrTy);
+  if (E->isArray()) {
     EmitNewInitializer(*this, E, NewPtr, NumElements, AllocSizeWithoutCookie);
-    NewPtr = Builder.CreateBitCast(NewPtr, ConvertType(E->getType()));
-  }
-  else {
-    NewPtr = Builder.CreateBitCast(NewPtr, ConvertType(E->getType()));
+
+    // NewPtr is a pointer to the base element type.  If we're
+    // allocating an array of arrays, we'll need to cast back to the
+    // array pointer type.
+    const llvm::Type *ResultTy = ConvertType(E->getType());
+    if (NewPtr->getType() != ResultTy)
+      NewPtr = Builder.CreateBitCast(NewPtr, ResultTy);
+  } else {
     EmitNewInitializer(*this, E, NewPtr, NumElements, AllocSizeWithoutCookie);
   }
   
   if (NullCheckResult) {
     Builder.CreateBr(NewEnd);
-    NewNotNull = Builder.GetInsertBlock();
-    EmitBlock(NewNull);
-    Builder.CreateBr(NewEnd);
+    llvm::BasicBlock *NotNullSource = Builder.GetInsertBlock();
     EmitBlock(NewEnd);
 
     llvm::PHINode *PHI = Builder.CreatePHI(NewPtr->getType());
     PHI->reserveOperandSpace(2);
-    PHI->addIncoming(NewPtr, NewNotNull);
-    PHI->addIncoming(llvm::Constant::getNullValue(NewPtr->getType()), NewNull);
+    PHI->addIncoming(NewPtr, NotNullSource);
+    PHI->addIncoming(llvm::Constant::getNullValue(NewPtr->getType()),
+                     NullCheckSource);
 
     NewPtr = PHI;
   }
-
-  return NewPtr;
-}
-
-static std::pair<llvm::Value *, llvm::Value *>
-GetAllocatedObjectPtrAndNumElements(CodeGenFunction &CGF,
-                                    llvm::Value *Ptr, QualType DeleteTy) {
-  QualType SizeTy = CGF.getContext().getSizeType();
-  const llvm::Type *SizeLTy = CGF.ConvertType(SizeTy);
   
-  CharUnits DeleteTypeAlign = CGF.getContext().getTypeAlignInChars(DeleteTy);
-  CharUnits CookiePadding = 
-    std::max(CGF.getContext().getTypeSizeInChars(SizeTy),
-             DeleteTypeAlign);
-  assert(!CookiePadding.isZero() && "CookiePadding should not be 0.");
-
-  const llvm::Type *Int8PtrTy = llvm::Type::getInt8PtrTy(CGF.getLLVMContext());
-  CharUnits CookieOffset = 
-    CookiePadding - CGF.getContext().getTypeSizeInChars(SizeTy);
-
-  llvm::Value *AllocatedObjectPtr = CGF.Builder.CreateBitCast(Ptr, Int8PtrTy);
-  AllocatedObjectPtr = 
-    CGF.Builder.CreateConstInBoundsGEP1_64(AllocatedObjectPtr,
-                                           -CookiePadding.getQuantity());
-
-  llvm::Value *NumElementsPtr =
-    CGF.Builder.CreateConstInBoundsGEP1_64(AllocatedObjectPtr, 
-                                           CookieOffset.getQuantity());
-  NumElementsPtr = 
-    CGF.Builder.CreateBitCast(NumElementsPtr, SizeLTy->getPointerTo());
-  
-  llvm::Value *NumElements = CGF.Builder.CreateLoad(NumElementsPtr);
-  NumElements = 
-    CGF.Builder.CreateIntCast(NumElements, SizeLTy, /*isSigned=*/false);
-  
-  return std::make_pair(AllocatedObjectPtr, NumElements);
+  return NewPtr;
 }
 
 void CodeGenFunction::EmitDeleteCall(const FunctionDecl *DeleteFD,
                                      llvm::Value *Ptr,
                                      QualType DeleteTy) {
+  assert(DeleteFD->getOverloadedOperator() == OO_Delete);
+
   const FunctionProtoType *DeleteFTy =
     DeleteFD->getType()->getAs<FunctionProtoType>();
 
@@ -815,21 +806,6 @@ void CodeGenFunction::EmitDeleteCall(const FunctionDecl *DeleteFD,
                                   DeleteTypeSize.getQuantity());
   }
   
-  if (DeleteFD->getOverloadedOperator() == OO_Array_Delete &&
-      !CalculateCookiePadding(getContext(), DeleteTy).isZero()) {
-    // We need to get the number of elements in the array from the cookie.
-    llvm::Value *AllocatedObjectPtr;
-    llvm::Value *NumElements;
-    llvm::tie(AllocatedObjectPtr, NumElements) =
-      GetAllocatedObjectPtrAndNumElements(*this, Ptr, DeleteTy);
-    
-    // Multiply the size with the number of elements.
-    if (Size)
-      Size = Builder.CreateMul(NumElements, Size);
-    
-    Ptr = AllocatedObjectPtr;
-  }
-  
   QualType ArgTy = DeleteFTy->getArgType(0);
   llvm::Value *DeletePtr = Builder.CreateBitCast(Ptr, ConvertType(ArgTy));
   DeleteArgs.push_back(std::make_pair(RValue::get(DeletePtr), ArgTy));
@@ -843,6 +819,156 @@ void CodeGenFunction::EmitDeleteCall(const FunctionDecl *DeleteFD,
            DeleteArgs, DeleteFD);
 }
 
+namespace {
+  /// Calls the given 'operator delete' on a single object.
+  struct CallObjectDelete : EHScopeStack::Cleanup {
+    llvm::Value *Ptr;
+    const FunctionDecl *OperatorDelete;
+    QualType ElementType;
+
+    CallObjectDelete(llvm::Value *Ptr,
+                     const FunctionDecl *OperatorDelete,
+                     QualType ElementType)
+      : Ptr(Ptr), OperatorDelete(OperatorDelete), ElementType(ElementType) {}
+
+    void Emit(CodeGenFunction &CGF, bool IsForEH) {
+      CGF.EmitDeleteCall(OperatorDelete, Ptr, ElementType);
+    }
+  };
+}
+
+/// Emit the code for deleting a single object.
+static void EmitObjectDelete(CodeGenFunction &CGF,
+                             const FunctionDecl *OperatorDelete,
+                             llvm::Value *Ptr,
+                             QualType ElementType) {
+  // Find the destructor for the type, if applicable.  If the
+  // destructor is virtual, we'll just emit the vcall and return.
+  const CXXDestructorDecl *Dtor = 0;
+  if (const RecordType *RT = ElementType->getAs<RecordType>()) {
+    CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+    if (!RD->hasTrivialDestructor()) {
+      Dtor = RD->getDestructor();
+
+      if (Dtor->isVirtual()) {
+        const llvm::Type *Ty =
+          CGF.getTypes().GetFunctionType(CGF.getTypes().getFunctionInfo(Dtor),
+                                         /*isVariadic=*/false);
+          
+        llvm::Value *Callee
+          = CGF.BuildVirtualCall(Dtor, Dtor_Deleting, Ptr, Ty);
+        CGF.EmitCXXMemberCall(Dtor, Callee, ReturnValueSlot(), Ptr, /*VTT=*/0,
+                              0, 0);
+
+        // The dtor took care of deleting the object.
+        return;
+      }
+    }
+  }
+
+  // Make sure that we call delete even if the dtor throws.
+  CGF.EHStack.pushCleanup<CallObjectDelete>(NormalAndEHCleanup,
+                                            Ptr, OperatorDelete, ElementType);
+
+  if (Dtor)
+    CGF.EmitCXXDestructorCall(Dtor, Dtor_Complete,
+                              /*ForVirtualBase=*/false, Ptr);
+
+  CGF.PopCleanupBlock();
+}
+
+namespace {
+  /// Calls the given 'operator delete' on an array of objects.
+  struct CallArrayDelete : EHScopeStack::Cleanup {
+    llvm::Value *Ptr;
+    const FunctionDecl *OperatorDelete;
+    llvm::Value *NumElements;
+    QualType ElementType;
+    CharUnits CookieSize;
+
+    CallArrayDelete(llvm::Value *Ptr,
+                    const FunctionDecl *OperatorDelete,
+                    llvm::Value *NumElements,
+                    QualType ElementType,
+                    CharUnits CookieSize)
+      : Ptr(Ptr), OperatorDelete(OperatorDelete), NumElements(NumElements),
+        ElementType(ElementType), CookieSize(CookieSize) {}
+
+    void Emit(CodeGenFunction &CGF, bool IsForEH) {
+      const FunctionProtoType *DeleteFTy =
+        OperatorDelete->getType()->getAs<FunctionProtoType>();
+      assert(DeleteFTy->getNumArgs() == 1 || DeleteFTy->getNumArgs() == 2);
+
+      CallArgList Args;
+      
+      // Pass the pointer as the first argument.
+      QualType VoidPtrTy = DeleteFTy->getArgType(0);
+      llvm::Value *DeletePtr
+        = CGF.Builder.CreateBitCast(Ptr, CGF.ConvertType(VoidPtrTy));
+      Args.push_back(std::make_pair(RValue::get(DeletePtr), VoidPtrTy));
+
+      // Pass the original requested size as the second argument.
+      if (DeleteFTy->getNumArgs() == 2) {
+        QualType size_t = DeleteFTy->getArgType(1);
+        const llvm::IntegerType *SizeTy
+          = cast<llvm::IntegerType>(CGF.ConvertType(size_t));
+        
+        CharUnits ElementTypeSize =
+          CGF.CGM.getContext().getTypeSizeInChars(ElementType);
+
+        // The size of an element, multiplied by the number of elements.
+        llvm::Value *Size
+          = llvm::ConstantInt::get(SizeTy, ElementTypeSize.getQuantity());
+        Size = CGF.Builder.CreateMul(Size, NumElements);
+
+        // Plus the size of the cookie if applicable.
+        if (!CookieSize.isZero()) {
+          llvm::Value *CookieSizeV
+            = llvm::ConstantInt::get(SizeTy, CookieSize.getQuantity());
+          Size = CGF.Builder.CreateAdd(Size, CookieSizeV);
+        }
+
+        Args.push_back(std::make_pair(RValue::get(Size), size_t));
+      }
+
+      // Emit the call to delete.
+      CGF.EmitCall(CGF.getTypes().getFunctionInfo(Args, DeleteFTy),
+                   CGF.CGM.GetAddrOfFunction(OperatorDelete),
+                   ReturnValueSlot(), Args, OperatorDelete);
+    }
+  };
+}
+
+/// Emit the code for deleting an array of objects.
+static void EmitArrayDelete(CodeGenFunction &CGF,
+                            const FunctionDecl *OperatorDelete,
+                            llvm::Value *Ptr,
+                            QualType ElementType) {
+  llvm::Value *NumElements = 0;
+  llvm::Value *AllocatedPtr = 0;
+  CharUnits CookieSize;
+  CGF.CGM.getCXXABI().ReadArrayCookie(CGF, Ptr, ElementType,
+                                      NumElements, AllocatedPtr, CookieSize);
+
+  assert(AllocatedPtr && "ReadArrayCookie didn't set AllocatedPtr");
+
+  // Make sure that we call delete even if one of the dtors throws.
+  CGF.EHStack.pushCleanup<CallArrayDelete>(NormalAndEHCleanup,
+                                           AllocatedPtr, OperatorDelete,
+                                           NumElements, ElementType,
+                                           CookieSize);
+
+  if (const CXXRecordDecl *RD = ElementType->getAsCXXRecordDecl()) {
+    if (!RD->hasTrivialDestructor()) {
+      assert(NumElements && "ReadArrayCookie didn't find element count"
+                            " for a class with destructor");
+      CGF.EmitCXXAggrDestructorCall(RD->getDestructor(), NumElements, Ptr);
+    }
+  }
+
+  CGF.PopCleanupBlock();
+}
+
 void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) {
   
   // Get at the argument before we performed the implicit conversion
@@ -855,8 +981,6 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) {
     else
       break;
   }
-  
-  QualType DeleteTy = Arg->getType()->getAs<PointerType>()->getPointeeType();
 
   llvm::Value *Ptr = EmitScalarExpr(Arg);
 
@@ -870,41 +994,38 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) {
 
   Builder.CreateCondBr(IsNull, DeleteEnd, DeleteNotNull);
   EmitBlock(DeleteNotNull);
-  
-  bool ShouldCallDelete = true;
-  
-  // Call the destructor if necessary.
-  if (const RecordType *RT = DeleteTy->getAs<RecordType>()) {
-    if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl())) {
-      if (!RD->hasTrivialDestructor()) {
-        const CXXDestructorDecl *Dtor = RD->getDestructor();
-        if (E->isArrayForm()) {
-          llvm::Value *AllocatedObjectPtr;
-          llvm::Value *NumElements;
-          llvm::tie(AllocatedObjectPtr, NumElements) =
-            GetAllocatedObjectPtrAndNumElements(*this, Ptr, DeleteTy);
-          
-          EmitCXXAggrDestructorCall(Dtor, NumElements, Ptr);
-        } else if (Dtor->isVirtual()) {
-          const llvm::Type *Ty =
-            CGM.getTypes().GetFunctionType(CGM.getTypes().getFunctionInfo(Dtor),
-                                           /*isVariadic=*/false);
-          
-          llvm::Value *Callee = BuildVirtualCall(Dtor, Dtor_Deleting, Ptr, Ty);
-          EmitCXXMemberCall(Dtor, Callee, ReturnValueSlot(), Ptr, /*VTT=*/0,
-                            0, 0);
-
-          // The dtor took care of deleting the object.
-          ShouldCallDelete = false;
-        } else 
-          EmitCXXDestructorCall(Dtor, Dtor_Complete, /*ForVirtualBase=*/false,
-                                Ptr);
-      }
+
+  // We might be deleting a pointer to array.  If so, GEP down to the
+  // first non-array element.
+  // (this assumes that A(*)[3][7] is converted to [3 x [7 x %A]]*)
+  QualType DeleteTy = Arg->getType()->getAs<PointerType>()->getPointeeType();
+  if (DeleteTy->isConstantArrayType()) {
+    llvm::Value *Zero = Builder.getInt32(0);
+    llvm::SmallVector<llvm::Value*,8> GEP;
+
+    GEP.push_back(Zero); // point at the outermost array
+
+    // For each layer of array type we're pointing at:
+    while (const ConstantArrayType *Arr
+             = getContext().getAsConstantArrayType(DeleteTy)) {
+      // 1. Unpeel the array type.
+      DeleteTy = Arr->getElementType();
+
+      // 2. GEP to the first element of the array.
+      GEP.push_back(Zero);
     }
+
+    Ptr = Builder.CreateInBoundsGEP(Ptr, GEP.begin(), GEP.end(), "del.first");
   }
 
-  if (ShouldCallDelete)
-    EmitDeleteCall(E->getOperatorDelete(), Ptr, DeleteTy);
+  assert(ConvertType(DeleteTy) ==
+         cast<llvm::PointerType>(Ptr->getType())->getElementType());
+
+  if (E->isArrayForm()) {
+    EmitArrayDelete(*this, E->getOperatorDelete(), Ptr, DeleteTy);
+  } else {
+    EmitObjectDelete(*this, E->getOperatorDelete(), Ptr, DeleteTy);
+  }
 
   EmitBlock(DeleteEnd);
 }
diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index fc741f8ef0..4f0420536a 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -731,6 +731,7 @@ private:
 public:
   CodeGenFunction(CodeGenModule &cgm);
 
+  CodeGenTypes &getTypes() const { return CGM.getTypes(); }
   ASTContext &getContext() const;
   CGDebugInfo *getDebugInfo() { return DebugInfo; }
 
diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index 13768006f4..85a67a437d 100644
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -48,6 +48,8 @@ protected:
     return PtrDiffTy;
   }
 
+  bool NeedsArrayCookie(QualType ElementType);
+
 public:
   ItaniumCXXABI(CodeGen::CodeGenModule &CGM, bool IsARM = false) :
     CGCXXABI(CGM), PtrDiffTy(0), MangleCtx(CGM.getContext(), CGM.getDiags()),
@@ -108,6 +110,15 @@ public:
                                    FunctionArgList &Params);
 
   void EmitInstanceFunctionProlog(CodeGenFunction &CGF);
+
+  CharUnits GetArrayCookieSize(QualType ElementType);
+  llvm::Value *InitializeArrayCookie(CodeGenFunction &CGF,
+                                     llvm::Value *NewPtr,
+                                     llvm::Value *NumElements,
+                                     QualType ElementType);
+  void ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
+                       QualType ElementType, llvm::Value *&NumElements,
+                       llvm::Value *&AllocPtr, CharUnits &CookieSize);
 };
 
 class ARMCXXABI : public ItaniumCXXABI {
@@ -132,6 +143,14 @@ public:
 
   void EmitReturnFromThunk(CodeGenFunction &CGF, RValue RV, QualType ResTy);
 
+  CharUnits GetArrayCookieSize(QualType ElementType);
+  llvm::Value *InitializeArrayCookie(CodeGenFunction &am