[clang] 8a691cc - [MS][clang] Make sure vector deleting dtor calls correct operator delete (#133950)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 2 00:25:46 PDT 2025
Author: Mariya Podchishchaeva
Date: 2025-04-02T09:25:43+02:00
New Revision: 8a691cc6157b2c3bc91af767eb1154d7a715562a
URL: https://github.com/llvm/llvm-project/commit/8a691cc6157b2c3bc91af767eb1154d7a715562a
DIFF: https://github.com/llvm/llvm-project/commit/8a691cc6157b2c3bc91af767eb1154d7a715562a.diff
LOG: [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.
Added:
Modified:
clang/include/clang/AST/DeclCXX.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclCXX.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/CodeGenCXX/microsoft-vector-deleting-dtors.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index dbd02ef7f8011..7dbefeea4b1a3 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/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;
}
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 822cae99ddae7..6e504b7211567 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/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
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 7eff776882629..3c447a905a83c 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/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
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index f508930cc9f2b..c683dbb0af825 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/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);
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 43bf9b7cd0f95..cbd37aa0dd6a4 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/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);
}
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index fa492bc124abd..78eba8e262771 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -3265,9 +3265,8 @@ 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))
@@ -3275,8 +3274,7 @@ FunctionDecl *Sema::FindDeallocationFunctionForDestructor(SourceLocation Loc,
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);
diff --git a/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors.cpp b/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors.cpp
index ebff4f6a851b0..439ff84456033 100644
--- a/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors.cpp
+++ b/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 at Z" = weak dso_local unnamed_addr alias ptr (ptr, i32), ptr @"??_GBird@@UEAAPEAXI at 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 at YAXPEAX_K@Z"(ptr noundef %[[COOKIEGEP]], i64 noundef 8)
-// X86-NEXT: call void @"??3 at YAXPAXI@Z"(ptr noundef %[[COOKIEGEP]], i32 noundef 4)
+// X64-NEXT: call void @"??_V at YAXPEAX_K@Z"(ptr noundef %[[COOKIEGEP]], i64 noundef 8)
+// X86-NEXT: call void @"??_V at YAXPAXI@Z"(ptr noundef %[[COOKIEGEP]], i32 noundef 4)
// CHECK-NEXT: br label %dtor.continue
// CHECK: dtor.scalar:
// X64-NEXT: call void @"??1Parrot@@UEAA at 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 at 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 at Z"
+// X86-LABEL: define weak dso_local x86_thiscallcc noundef ptr @"??_EHasOperatorDelete@@UAEPAXI at Z"
+// CHECK: dtor.call_delete_after_array_destroy:
+// X64-NEXT: call void @"??_VHasOperatorDelete@@SAXPEAX at Z"
+// X86-NEXT: call void @"??_VHasOperatorDelete@@SAXPAX at Z"
+// CHECK: dtor.call_delete:
+// X64-NEXT: call void @"??3HasOperatorDelete@@SAXPEAX at Z"
+// X86-NEXT: call void @"??3HasOperatorDelete@@SAXPAX at Z"
More information about the cfe-commits
mailing list