[MS][clang] Make sure vector deleting dtor calls correct operator delete (#133950)

During additional testing I spotted that vector deleting dtor calls
operator delete, not operator delete[] when performing array deletion.
This patch fixes that.

Author: Fznamznon

clang/include/clang/AST/DeclCXX.h

@@ -2852,6 +2852,7 @@ class CXXDestructorDecl : public CXXMethodDecl {
   // FIXME: Don't allocate storage for these except in the first declaration
   // of a virtual destructor.
   FunctionDecl *OperatorDelete = nullptr;
+  FunctionDecl *OperatorArrayDelete = nullptr;
   Expr *OperatorDeleteThisArg = nullptr;
 
   CXXDestructorDecl(ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc,
@@ -2877,11 +2878,16 @@ class CXXDestructorDecl : public CXXMethodDecl {
   static CXXDestructorDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
 
   void setOperatorDelete(FunctionDecl *OD, Expr *ThisArg);
+  void setOperatorArrayDelete(FunctionDecl *OD, Expr *ThisArg);
 
   const FunctionDecl *getOperatorDelete() const {
     return getCanonicalDecl()->OperatorDelete;
   }
 
+  const FunctionDecl *getArrayOperatorDelete() const {
+    return getCanonicalDecl()->OperatorArrayDelete;
+  }
+
   Expr *getOperatorDeleteThisArg() const {
     return getCanonicalDecl()->OperatorDeleteThisArg;
   }

clang/include/clang/Sema/Sema.h

@@ -8335,7 +8335,8 @@ class Sema final : public SemaBase {
                                               bool Overaligned,
                                               DeclarationName Name);
   FunctionDecl *FindDeallocationFunctionForDestructor(SourceLocation StartLoc,
-                                                      CXXRecordDecl *RD);
+                                                      CXXRecordDecl *RD,
+                                                      DeclarationName Name);
 
   /// ActOnCXXDelete - Parsed a C++ 'delete' expression (C++ 5.3.5), as in:
   /// @code ::delete ptr; @endcode

clang/lib/AST/DeclCXX.cpp

@@ -3026,6 +3026,13 @@ void CXXDestructorDecl::setOperatorDelete(FunctionDecl *OD, Expr *ThisArg) {
   }
 }
 
+void CXXDestructorDecl::setOperatorArrayDelete(FunctionDecl *OD,
+                                               Expr *ThisArg) {
+  auto *First = cast<CXXDestructorDecl>(getFirstDecl());
+  if (OD && !First->OperatorArrayDelete)
+    First->OperatorArrayDelete = OD;
+}
+
 bool CXXDestructorDecl::isCalledByDelete(const FunctionDecl *OpDel) const {
   // C++20 [expr.delete]p6: If the value of the operand of the delete-
   // expression is not a null pointer value and the selected deallocation

clang/lib/CodeGen/CGClass.cpp

@@ -1489,7 +1489,7 @@ static void EmitConditionalArrayDtorCall(const CXXDestructorDecl *DD,
   CGF.EmitBlock(callDeleteBB);
   const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CGF.CurCodeDecl);
   const CXXRecordDecl *ClassDecl = Dtor->getParent();
-  CGF.EmitDeleteCall(Dtor->getOperatorDelete(), allocatedPtr,
+  CGF.EmitDeleteCall(Dtor->getArrayOperatorDelete(), allocatedPtr,
                      CGF.getContext().getTagDeclType(ClassDecl));
 
   CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);

clang/lib/Sema/SemaDeclCXX.cpp

@@ -11042,9 +11042,11 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
     else
       Loc = RD->getLocation();
 
+    DeclarationName Name =
+        Context.DeclarationNames.getCXXOperatorName(OO_Delete);
     // If we have a virtual destructor, look up the deallocation function
     if (FunctionDecl *OperatorDelete =
-            FindDeallocationFunctionForDestructor(Loc, RD)) {
+            FindDeallocationFunctionForDestructor(Loc, RD, Name)) {
       Expr *ThisArg = nullptr;
 
       // If the notional 'delete this' expression requires a non-trivial
@@ -11075,6 +11077,15 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
       DiagnoseUseOfDecl(OperatorDelete, Loc);
       MarkFunctionReferenced(Loc, OperatorDelete);
       Destructor->setOperatorDelete(OperatorDelete, ThisArg);
+      // Lookup delete[] too in case we have to emit a vector deleting dtor;
+      DeclarationName VDeleteName =
+          Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete);
+      FunctionDecl *ArrOperatorDelete =
+          FindDeallocationFunctionForDestructor(Loc, RD, VDeleteName);
+      // delete[] in the TU will make sure the operator is referenced and its
+      // uses diagnosed, otherwise vector deleting dtor won't be called anyway,
+      // so just record it in the destructor.
+      Destructor->setOperatorArrayDelete(ArrOperatorDelete, ThisArg);
     }
   }
 

clang/lib/Sema/SemaExprCXX.cpp

@@ -3265,18 +3265,16 @@ FunctionDecl *Sema::FindUsualDeallocationFunction(SourceLocation StartLoc,
   return Result.FD;
 }
 
-FunctionDecl *Sema::FindDeallocationFunctionForDestructor(SourceLocation Loc,
-                                                          CXXRecordDecl *RD) {
-  DeclarationName Name = Context.DeclarationNames.getCXXOperatorName(OO_Delete);
+FunctionDecl *Sema::FindDeallocationFunctionForDestructor(
+    SourceLocation Loc, CXXRecordDecl *RD, DeclarationName Name) {
 
   FunctionDecl *OperatorDelete = nullptr;
   if (FindDeallocationFunction(Loc, RD, Name, OperatorDelete))
     return nullptr;
   if (OperatorDelete)
     return OperatorDelete;
 
-  // If there's no class-specific operator delete, look up the global
-  // non-array delete.
+  // If there's no class-specific operator delete, look up the global delete.
   return FindUsualDeallocationFunction(
       Loc, true, hasNewExtendedAlignment(*this, Context.getRecordType(RD)),
       Name);

clang/test/CodeGenCXX/microsoft-vector-deleting-dtors.cpp

@@ -28,6 +28,13 @@ struct JustAWeirdBird {
   }
 };
 
+int i = 0;
+struct HasOperatorDelete : public Bird{
+~HasOperatorDelete() { }
+void operator delete(void *p) { i-=2; }
+void operator delete[](void *p) { i--; }
+};
+
 // Vector deleting dtor for Bird is an alias because no new Bird[] expressions
 // in the TU.
 // X64: @"??_EBird@@UEAAPEAXI@Z" = weak dso_local unnamed_addr alias ptr (ptr, i32), ptr @"??_GBird@@UEAAPEAXI@Z"
@@ -53,6 +60,9 @@ void bar() {
 
   JustAWeirdBird B;
   B.doSmth(38);
+
+  Bird *p = new HasOperatorDelete[2];
+  dealloc(p);
 }
 
 // CHECK-LABEL: define dso_local void @{{.*}}dealloc{{.*}}(
@@ -129,8 +139,8 @@ void bar() {
 // CHECK-NEXT:   %[[ISFIRSTBITZERO:.*]] = icmp eq i32 %[[FIRSTBIT]], 0
 // CHECK-NEXT:   br i1 %[[ISFIRSTBITZERO]], label %dtor.continue, label %dtor.call_delete_after_array_destroy
 // CHECK: dtor.call_delete_after_array_destroy:
-// X64-NEXT:     call void @"??3@YAXPEAX_K@Z"(ptr noundef %[[COOKIEGEP]], i64 noundef 8)
-// X86-NEXT:     call void @"??3@YAXPAXI@Z"(ptr noundef %[[COOKIEGEP]], i32 noundef 4)
+// X64-NEXT:     call void @"??_V@YAXPEAX_K@Z"(ptr noundef %[[COOKIEGEP]], i64 noundef 8)
+// X86-NEXT:     call void @"??_V@YAXPAXI@Z"(ptr noundef %[[COOKIEGEP]], i32 noundef 4)
 // CHECK-NEXT:   br label %dtor.continue
 // CHECK: dtor.scalar:
 // X64-NEXT:   call void @"??1Parrot@@UEAA@XZ"(ptr noundef nonnull align 8 dereferenceable(8) %[[LTHIS]])
@@ -150,3 +160,12 @@ void bar() {
 // X64-SAME: ptr noundef nonnull align 8 dereferenceable(8) %this, i32 noundef %should_call_delete)
 // X86: define weak dso_local x86_thiscallcc noundef ptr @"??_EJustAWeirdBird@@UAEPAXI@Z"(
 // X86-SAME: ptr noundef nonnull align 4 dereferenceable(4) %this, i32 noundef %should_call_delete) unnamed_addr
+
+// X64-LABEL: define weak dso_local noundef ptr @"??_EHasOperatorDelete@@UEAAPEAXI@Z"
+// X86-LABEL: define weak dso_local x86_thiscallcc noundef ptr @"??_EHasOperatorDelete@@UAEPAXI@Z"
+// CHECK: dtor.call_delete_after_array_destroy:
+// X64-NEXT: call void @"??_VHasOperatorDelete@@SAXPEAX@Z"
+// X86-NEXT: call void @"??_VHasOperatorDelete@@SAXPAX@Z"
+// CHECK: dtor.call_delete:
+// X64-NEXT: call void @"??3HasOperatorDelete@@SAXPEAX@Z"
+// X86-NEXT: call void @"??3HasOperatorDelete@@SAXPAX@Z"