Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions clang/include/clang/AST/DeclCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/AST/DeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/CGClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 12 additions & 1 deletion clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down
8 changes: 3 additions & 5 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3266,18 +3266,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);
Expand Down
23 changes: 21 additions & 2 deletions clang/test/CodeGenCXX/microsoft-vector-deleting-dtors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{{.*}}(
Expand Down Expand Up @@ -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]])
Expand All @@ -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"
Loading