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@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" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
