Skip to content

[win][clang] Align scalar deleting destructors with MSABI #139566

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.

Sign up for GitHub

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

[win][clang] Align scalar deleting destructors with MSABI #139566

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1b0b624
[win][clang] Align scalar deleting destructors with MSABI
Fznamznon May 12, 2025
d945d0e
Merge branch 'main' into fix-msvc-scalar-deleting-dtors
Fznamznon Jun 10, 2025
2e07469
Always use getCanonicalDecl() and avoid emitting branch with unreachable
Fznamznon Jun 11, 2025
2c51545
Bring back accidentally removed line
Fznamznon Jun 12, 2025
9493c05
Add release note for the breaking change.
Fznamznon Jun 12, 2025
ef899c2
Merge branch 'main' into fix-msvc-scalar-deleting-dtors
Fznamznon Jun 16, 2025
94f43ae
Enable changes under -fclang-abi-compat
Fznamznon Jun 16, 2025
83a8f06
Change variable's name
Fznamznon Jun 16, 2025
c9efdf8
Add a test for ::delete done on an object of a class with destroying
Fznamznon Jun 17, 2025
f5e7a45
Apply suggestions from code review
Fznamznon Jun 18, 2025
9d3c96c
Format
Fznamznon Jun 18, 2025
d71d3d4
Add comment to EmitConditionalDtorDeleteCall
Fznamznon Jun 18, 2025
c670d74
Assert that setOperatorGlobalDelete only accepts a global delete
Fznamznon Jun 18, 2025
7147603
Merge branch 'main' into fix-msvc-scalar-deleting-dtors
Fznamznon Jun 24, 2025
057ac9f
Update clang/docs/ReleaseNotes.rst
Fznamznon Jun 24, 2025
05c386a
Add serialization support and tests
Fznamznon Jun 25, 2025
d07da86
Remove trailing spaces
Fznamznon Jun 25, 2025
92bbda6
Introduce callGlobalDeleteInDeletingDtor
Fznamznon Jun 26, 2025
83665e2
Change the assertion
Fznamznon Jun 26, 2025
c214b69
Mention -fclang-abi-compat in the release note.
Fznamznon Jul 7, 2025
bc55453
Merge branch 'main' into fix-msvc-scalar-deleting-dtors
Fznamznon Jul 7, 2025
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
10 changes: 10 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ Potentially Breaking Changes
``endbr64`` instruction at the labels named as possible branch
destinations, so it is not safe to use a register-controlled branch
instruction to branch to one. (In line with gcc.)
- Scalar deleting destructors emitted by clang have been aligned with Microsoft
ABI on Windows. Prior to this patch, clang handled ``::delete`` via calling
global operator delete direct after the destructor call and not calling class
operator delete in scalar deleting destructor body by passing "0" as implicit
flag argument value. After this change if library A was compiled with
clang 20, library B compiled with clang 21. class B is implemented in
library A, so its destructor doesn't call operator ``::delete``, then an
object of class B is deleted via ``::delete`` in library B, so the callsite
doesn't call ``::delete`` because of this patch.
So there will be no ``::delete`` call at all.

C/C++ Language Potentially Breaking Changes
-------------------------------------------
Expand Down
11 changes: 11 additions & 0 deletions clang/include/clang/AST/DeclCXX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2856,6 +2856,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 *OperatorGlobalDelete = nullptr;
Expr *OperatorDeleteThisArg = nullptr;

CXXDestructorDecl(ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc,
Expand Down Expand Up @@ -2886,6 +2887,16 @@ class CXXDestructorDecl : public CXXMethodDecl {
return getCanonicalDecl()->OperatorDelete;
}

const FunctionDecl *getOperatorGlobalDelete() const {
return getCanonicalDecl()->OperatorGlobalDelete;
}

void setOperatorGlobalDelete(FunctionDecl *OD) {
auto *First = cast<CXXDestructorDecl>(getCanonicalDecl());
if (OD && !First->OperatorGlobalDelete)
First->OperatorGlobalDelete = OD;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a call to setOperatorGlobalDelete(nullptr) should clear OperatorGlobalDelete.

Hmm, setOperatorDelete() silently ignores a call that passes nullptr. That seems wrong. Maybe there should be a non-null assertion?

setOperatorDelete() also registers the delete operator with ASTMutationListener::ResolvedOperatorDelete() and ASTWriter overrides that function. That seems important for PCH and module support. I think updates are needed to clang/lib/Serialization/ASTReaderDecl.cpp as well to restore OperatorGlobalDelete during AST deserialization; see ASTDeclReader::VisitCXXDestructorDecl() and ASTDeclReader::UpdateDecl() for assignments to OperatorDelete.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what would be the better approach to implement this? Should I invent ResolvedOperatorDelete but for global operator delete or can we tie global operator delete to a regular one like we do with ThisArg in setOperatorDelete() and other serialization parts?
cc @ChuanqiXu9

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Adding a parallel set of functions seems reasonable to me. It might also be reasonable to tie the updates together, but I think that might unnecessarily increase the serialized AST size for Itanium targets. Perhaps that can be avoided in either case; it isn't obvious to me.

While looking at this some more, I noticed that it looks like updates will be needed to clang/lib/AST/ASTImporter.cpp as well; see the call to setOperatorDelete() in ASTNodeImporter::VisitFunctionDecl().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tahonermann , added AST serialization support along with tests.

}

Expr *getOperatorDeleteThisArg() const {
return getCanonicalDecl()->OperatorDeleteThisArg;
}
Expand Down
10 changes: 6 additions & 4 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8528,10 +8528,12 @@ class Sema final : public SemaBase {
bool Diagnose = true);
FunctionDecl *FindUsualDeallocationFunction(SourceLocation StartLoc,
ImplicitDeallocationParameters,
DeclarationName Name);
FunctionDecl *FindDeallocationFunctionForDestructor(SourceLocation StartLoc,
CXXRecordDecl *RD,
bool Diagnose = true);
DeclarationName Name,
bool Diagnose = true);
FunctionDecl *
FindDeallocationFunctionForDestructor(SourceLocation StartLoc,
CXXRecordDecl *RD, bool Diagnose = true,
bool LookForGlobal = false);

/// ActOnCXXDelete - Parsed a C++ 'delete' expression (C++ 5.3.5), as in:
/// @code ::delete ptr; @endcode
Expand Down
73 changes: 59 additions & 14 deletions clang/lib/CodeGen/CGClass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1590,25 +1590,70 @@ namespace {
void EmitConditionalDtorDeleteCall(CodeGenFunction &CGF,
llvm::Value *ShouldDeleteCondition,
bool ReturnAfterDelete) {
const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CGF.CurCodeDecl);
const CXXRecordDecl *ClassDecl = Dtor->getParent();
const FunctionDecl *OD = Dtor->getOperatorDelete();
assert(OD->isDestroyingOperatorDelete() == ReturnAfterDelete &&
"unexpected value for ReturnAfterDelete");
auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
if (OD->isDestroyingOperatorDelete()) {
llvm::BasicBlock *CallDtor = CGF.createBasicBlock("dtor.call_dtor");
llvm::BasicBlock *DontCallDtor = CGF.createBasicBlock("dtor.entry_cont");
// Third bit set signals that global operator delete is called. That means
// despite class having destroying operator delete which is responsible
// for calling dtor, we need to call dtor because global operator delete
// won't do that.
llvm::Value *Check3rdBit = CGF.Builder.CreateAnd(
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 4));
llvm::Value *ShouldCallDtor = CGF.Builder.CreateIsNull(Check3rdBit);
CGF.Builder.CreateCondBr(ShouldCallDtor, DontCallDtor, CallDtor);
CGF.EmitBlock(CallDtor);
QualType ThisTy = Dtor->getFunctionObjectParameterType();
CGF.EmitCXXDestructorCall(Dtor, Dtor_Complete, /*ForVirtualBase=*/false,
/*Delegating=*/false, CGF.LoadCXXThisAddress(),
ThisTy);
CGF.Builder.CreateBr(DontCallDtor);
CGF.EmitBlock(DontCallDtor);
}
llvm::BasicBlock *callDeleteBB = CGF.createBasicBlock("dtor.call_delete");
llvm::BasicBlock *continueBB = CGF.createBasicBlock("dtor.continue");
llvm::Value *ShouldCallDelete
= CGF.Builder.CreateIsNull(ShouldDeleteCondition);
// First bit set signals that operator delete must be called.
llvm::Value *Check1stBit = CGF.Builder.CreateAnd(
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
llvm::Value *ShouldCallDelete = CGF.Builder.CreateIsNull(Check1stBit);
CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);

CGF.EmitBlock(callDeleteBB);
const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CGF.CurCodeDecl);
const CXXRecordDecl *ClassDecl = Dtor->getParent();
CGF.EmitDeleteCall(Dtor->getOperatorDelete(),
LoadThisForDtorDelete(CGF, Dtor),
CGF.getContext().getTagDeclType(ClassDecl));
assert(Dtor->getOperatorDelete()->isDestroyingOperatorDelete() ==
ReturnAfterDelete &&
"unexpected value for ReturnAfterDelete");
if (ReturnAfterDelete)
CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
else
CGF.Builder.CreateBr(continueBB);
auto EmitDeleteAndGoToEnd = [&](const FunctionDecl *DeleteOp) {
CGF.EmitDeleteCall(DeleteOp, LoadThisForDtorDelete(CGF, Dtor),
CGF.getContext().getTagDeclType(ClassDecl));
if (ReturnAfterDelete)
CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
else
CGF.Builder.CreateBr(continueBB);
};
// If Sema only found a global operator delete previously, the dtor can
// always call it. Otherwise we need to check the third bit and call the
// appropriate operator delete, i.e. global or class-specific.
if (const FunctionDecl *GlobOD = Dtor->getOperatorGlobalDelete();
isa<CXXMethodDecl>(OD) && GlobOD) {
// Third bit set signals that global operator delete is called, i.e.
// ::delete appears on the callsite.
llvm::Value *CheckTheBitForGlobDeleteCall = CGF.Builder.CreateAnd(
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 4));
llvm::Value *ShouldCallGlobDelete =
CGF.Builder.CreateIsNull(CheckTheBitForGlobDeleteCall);
llvm::BasicBlock *GlobDelete =
CGF.createBasicBlock("dtor.call_glob_delete");
llvm::BasicBlock *ClassDelete =
CGF.createBasicBlock("dtor.call_class_delete");
CGF.Builder.CreateCondBr(ShouldCallGlobDelete, ClassDelete, GlobDelete);
CGF.EmitBlock(GlobDelete);

EmitDeleteAndGoToEnd(GlobOD);
CGF.EmitBlock(ClassDelete);
}
EmitDeleteAndGoToEnd(OD);

CGF.EmitBlock(continueBB);
}
Expand Down
10 changes: 3 additions & 7 deletions clang/lib/CodeGen/MicrosoftCXXABI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,12 +894,8 @@ void MicrosoftCXXABI::emitVirtualObjectDelete(CodeGenFunction &CGF,
const CXXDestructorDecl *Dtor) {
// FIXME: Provide a source location here even though there's no
// CXXMemberCallExpr for dtor call.
bool UseGlobalDelete = DE->isGlobalDelete();
CXXDtorType DtorType = UseGlobalDelete ? Dtor_Complete : Dtor_Deleting;
llvm::Value *MDThis = EmitVirtualDestructorCall(CGF, Dtor, DtorType, Ptr, DE,
/*CallOrInvoke=*/nullptr);
if (UseGlobalDelete)
CGF.EmitDeleteCall(DE->getOperatorDelete(), MDThis, ElementType);
EmitVirtualDestructorCall(CGF, Dtor, Dtor_Deleting, Ptr, DE,
/*CallOrInvoke=*/nullptr);
}

void MicrosoftCXXABI::emitRethrow(CodeGenFunction &CGF, bool isNoReturn) {
Expand Down Expand Up @@ -2014,7 +2010,7 @@ llvm::Value *MicrosoftCXXABI::EmitVirtualDestructorCall(
ASTContext &Context = getContext();
llvm::Value *ImplicitParam = llvm::ConstantInt::get(
llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
DtorType == Dtor_Deleting);
(DtorType == Dtor_Deleting) | 4 * (D && D->isGlobalDelete()));

QualType ThisTy;
if (CE) {
Expand Down
15 changes: 15 additions & 0 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11139,6 +11139,21 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
DiagnoseUseOfDecl(OperatorDelete, Loc);
MarkFunctionReferenced(Loc, OperatorDelete);
Destructor->setOperatorDelete(OperatorDelete, ThisArg);

if (isa<CXXMethodDecl>(OperatorDelete) &&
Context.getTargetInfo().getCXXABI().isMicrosoft()) {
// In Microsoft ABI whenever a class has a defined operator delete,
// scalar deleting destructors check the 3rd bit of the implicit
// parameter and if it is set, then, global operator delete must be
// called instead of class-specific one. Find and save global operator
// for that case. Do not diagnose because even if we fail to find the
// operator, it won't be possible to compile and execute the code that
// requires it.
FunctionDecl *GlobalOperatorDelete =
FindDeallocationFunctionForDestructor(Loc, RD, /*Diagnose*/ false,
/*LookForGlobal*/ true);
Destructor->setOperatorGlobalDelete(GlobalOperatorDelete);
}
}
}

Expand Down
19 changes: 11 additions & 8 deletions clang/lib/Sema/SemaExprCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3561,7 +3561,7 @@ void Sema::DeclareGlobalAllocationFunction(DeclarationName Name,
FunctionDecl *
Sema::FindUsualDeallocationFunction(SourceLocation StartLoc,
ImplicitDeallocationParameters IDP,
DeclarationName Name) {
DeclarationName Name, bool Diagnose) {
DeclareGlobalNewDelete();

LookupResult FoundDelete(*this, Name, StartLoc, LookupOrdinaryName);
Expand All @@ -3576,7 +3576,7 @@ Sema::FindUsualDeallocationFunction(SourceLocation StartLoc,
if (!Result)
return nullptr;

if (CheckDeleteOperator(*this, StartLoc, StartLoc, /*Diagnose=*/true,
if (CheckDeleteOperator(*this, StartLoc, StartLoc, Diagnose,
FoundDelete.getNamingClass(), Result.Found,
Result.FD))
return nullptr;
Expand All @@ -3587,7 +3587,8 @@ Sema::FindUsualDeallocationFunction(SourceLocation StartLoc,

FunctionDecl *Sema::FindDeallocationFunctionForDestructor(SourceLocation Loc,
CXXRecordDecl *RD,
bool Diagnose) {
bool Diagnose,
bool LookForGlobal) {
DeclarationName Name = Context.DeclarationNames.getCXXOperatorName(OO_Delete);

FunctionDecl *OperatorDelete = nullptr;
Expand All @@ -3596,18 +3597,20 @@ FunctionDecl *Sema::FindDeallocationFunctionForDestructor(SourceLocation Loc,
DeallocType, ShouldUseTypeAwareOperatorNewOrDelete(),
AlignedAllocationMode::No, SizedDeallocationMode::No};

if (FindDeallocationFunction(Loc, RD, Name, OperatorDelete, IDP, Diagnose))
return nullptr;
if (!LookForGlobal) {
if (FindDeallocationFunction(Loc, RD, Name, OperatorDelete, IDP, Diagnose))
return nullptr;

if (OperatorDelete)
return OperatorDelete;
if (OperatorDelete)
return OperatorDelete;
}

// If there's no class-specific operator delete, look up the global
// non-array delete.
IDP.PassAlignment = alignedAllocationModeFromBool(
hasNewExtendedAlignment(*this, DeallocType));
IDP.PassSize = SizedDeallocationMode::Yes;
return FindUsualDeallocationFunction(Loc, IDP, Name);
return FindUsualDeallocationFunction(Loc, IDP, Name, Diagnose);
}

bool Sema::FindDeallocationFunction(SourceLocation StartLoc, CXXRecordDecl *RD,
Expand Down
59 changes: 52 additions & 7 deletions clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,43 @@ H::~H() { call_in_dtor(); }
// CHECK-ITANIUM-NOT: call
// CHECK-ITANIUM: }

// CHECK-MSABI64-LABEL: define {{.*}} @"??_GH@@UEAAPEAXI@Z"(
// CHECK-MSABI64-LABEL: define {{.*}} @"??_GH@@UEAAPEAXI@Z"({{.*}},
// CHECK-MSABI32-LABEL: define {{.*}} @"??_GH@@UAEPAXI@Z"(
// CHECK-MSABI-SAME: i32 noundef %[[IP:.*]])
// CHECK-MSABI-NOT: call{{ }}
// CHECK-MSABI: load i32
// CHECK-MSABI: icmp eq i32 {{.*}}, 0
// CHECK-MSABI: br i1
// CHECK-MSABI: store i32 %[[IP]], ptr %[[IP_ALLOCA:.*]]
// CHECK-MSABI: %[[IMP_PARAM:.*]] = load i32, ptr %[[IP_ALLOCA]]
// CHECK-MSABI-NEXT: %[[THIRDBIT:.*]] = and i32 %[[IMP_PARAM]], 4
// CHECK-MSABI-NEXT: %[[CHCK:.*]] = icmp eq i32 %[[THIRDBIT]], 0
// CHECK-MSABI-NEXT: br i1 %[[CHCK]], label %dtor.entry_cont, label %dtor.call_dtor
//
// CHECK_MSABI-LABEL: dtor.call_dtor:
// CHECK_MSABI-NEXT: call void @"??1H@@UEAA@XZ"({{.*}})
// CHECK_MSABI-NEXT: br label %dtor.entry_cont
//
// CHECK_MSABI-LABEL: dtor.entry_cont:
// CHECK_MSABI-NEXT: %[[FIRSTBIT:.*]] = and i32 %[[IMP_PARAM]], 1
// CHECK_MSABI-NEXT: %[[CHCK1:.*]] = icmp eq i32 %[[FIRSTBIT]], 0
// CHECK_MSABI-NEXT: br i1 %[[CHCK1]], label %dtor.continue, label %dtor.call_delete
//
// CHECK_MSABI-LABEL: dtor.call_delete:
// CHECK-MSABI: %[[THIRDBIT1:.*]] = and i32 %[[IMP_PARAM]], 4
// CHECK-MSABI-NEXT: %[[CHCK2:.*]] = icmp eq i32 %[[THIRDBIT1]], 0
// CHECK-MSABI-NEXT: br i1 %[[CHCK2]], label %dtor.call_class_delete, label %dtor.call_glob_delete
//
// CHECK-MSABI-LABEL: dtor.call_glob_delete:
// CHECK-MSABI64: call void @"??3@YAXPEAX_K@Z"(ptr noundef %{{.*}}, i64 noundef 48)
// CHECK-MSABI32: call void @"??3@YAXPAXIW4align_val_t@std@@@Z"(ptr noundef %{{.*}}, i32 noundef 32, i32 noundef 16)
// CHECK-MSABI-NEXT: br label %[[RETURN:.*]]
//
// CHECK-MSABI: dtor.call_class_delete:
// CHECK-MSABI-NOT: call{{ }}
// CHECK-MSABI64: getelementptr {{.*}}, i64 24
// CHECK-MSABI32: getelementptr {{.*}}, i32 20
// CHECK-MSABI-NOT: call{{ }}
// CHECK-MSABI64: call void @"??3F@@SAXPEAU0@Udestroying_delete_t@std@@_KW4align_val_t@2@@Z"({{.*}}, i64 noundef 48, i64 noundef 16)
// CHECK-MSABI32: call void @"??3F@@SAXPAU0@Udestroying_delete_t@std@@IW4align_val_t@2@@Z"({{.*}}, i32 noundef 32, i32 noundef 16)
// CHECK-MSABI: br label %[[RETURN:.*]]
// CHECK-MSABI: br label %[[RETURN:]]
//
// CHECK-MSABI64: call void @"??1H@@UEAA@XZ"(
// CHECK-MSABI32: call x86_thiscallcc void @"??1H@@UAE@XZ"(
Expand All @@ -194,9 +217,31 @@ I::~I() { call_in_dtor(); }
// CHECK-MSABI32-LABEL: define {{.*}} @"??_GI@@UAEPAXI@Z"(
// CHECK-MSABI-NOT: call{{ }}
// CHECK-MSABI: load i32
// CHECK-MSABI: icmp eq i32 {{.*}}, 0
// CHECK-MSABI: br i1
// CHECK-MSABI-NEXT: and i32 %[[IMP_PARAM:.*]], 4
// CHECK-MSABI-NEXT: icmp eq i32 {{.*}}, 0
// CHECK-MSABI-NEXT: br i1 %[[CHCK]], label %dtor.entry_cont, label %dtor.call_dtor
//
// CHECK-MSABI: dtor.call_dtor:
// CHECK-MSABI64-NEXT: call void @"??1I@@UEAA@XZ"({{.*}})
// CHECK-MSABI32-NEXT: call x86_thiscallcc void @"??1I@@UAE@XZ"({{.*}})
// CHECK-MSABI-NEXT: br label %dtor.entry_cont
//
// CHECK-MSABI: dtor.entry_cont:
// CHECK-MSABI-NEXT: and i32 %[[IMP_PARAM]], 1
// CHECK-MSABI-NEXT: icmp eq i32 %{{.*}}, 0
// CHECK-MSABI-NEXT: br i1 %{{.*}}, label %dtor.continue, label %dtor.call_delete
//
// CHECK-MSABI: dtor.call_delete:
// CHECK-MSABI-NEXT: %[[THIRDBIT1:.*]] = and i32 %[[IMP_PARAM]], 4
// CHECK-MSABI-NEXT: %[[CHCK2:.*]] = icmp eq i32 %[[THIRDBIT1]], 0
// CHECK-MSABI-NEXT: br i1 %[[CHCK2]], label %dtor.call_class_delete, label %dtor.call_glob_delete
//
// CHECK-MSABI-LABEL: dtor.call_glob_delete:
// CHECK-MSABI64: call void @"??3@YAXPEAX_KW4align_val_t@std@@@Z"(ptr noundef %{{.*}}, i64 noundef 96, i64 noundef 32)
// CHECK-MSABI32: call void @"??3@YAXPAXIW4align_val_t@std@@@Z"(ptr noundef %{{.*}}, i32 noundef 64, i32 noundef 32)
// CHECK-MSABI-NEXT: br label %[[RETURN:.*]]
//
// CHECK_MSABI: dtor.call_class_delete:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the additions here look good, but we're missing a test for a call to ::delete for a class that declares a destroying operator delete. Such a test is interesting for both the Itanium and MSVC ABIs for different reasons (the complete object destructor is called followed by global operator delete for Itanium; the deleting destructor is called and it calls the complete destructor and then the global operator delete for MSVC).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the test. I noticed though that MSVC always calls scalar deleting dtor even when class declares non-virtual destructor. clang doesn't even generate deleting destructors when class destructor is not virtual. I'm not yet sure which implications it has, need more experimenting. At least unlike the virtual case the wrong operator delete is not called.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not seeing that with this simple example (https://godbolt.org/z/xdThaEb78). g() calls the complete destructor followed by operator delete.

struct S {
  ~S();
};
S* f() {
  return new S;
}
void g(S *p) {
  ::delete p;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks. I probably hallucinated. I wasn't able to break this case while testing object files built with clang and msvc either.

// CHECK-MSABI-NOT: call{{ }}
// CHECK-MSABI64: getelementptr {{.*}}, i64 24
// CHECK-MSABI32: getelementptr {{.*}}, i32 20
Expand Down
44 changes: 41 additions & 3 deletions clang/test/CodeGenCXX/microsoft-abi-structors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ struct C {
// DTORS: store ptr %{{.*}}, ptr %[[RETVAL:retval]]
// DTORS: %[[SHOULD_DELETE_VALUE:[0-9a-z._]+]] = load i32, ptr %[[SHOULD_DELETE_VAR]]
// DTORS: call x86_thiscallcc void @"??1C@basic@@UAE@XZ"(ptr {{[^,]*}} %[[THIS:[0-9a-z]+]])
// DTORS-NEXT: %[[CONDITION:[0-9]+]] = icmp eq i32 %[[SHOULD_DELETE_VALUE]], 0
// DTORS-NEXT: %[[AND:[0-9]+]] = and i32 %[[SHOULD_DELETE_VALUE]], 1
// DTORS-NEXT: %[[CONDITION:[0-9]+]] = icmp eq i32 %[[AND]], 0
// DTORS-NEXT: br i1 %[[CONDITION]], label %[[CONTINUE_LABEL:[0-9a-z._]+]], label %[[CALL_DELETE_LABEL:[0-9a-z._]+]]
//
// DTORS: [[CALL_DELETE_LABEL]]
Expand Down Expand Up @@ -113,8 +114,7 @@ void call_deleting_dtor_and_global_delete(C *obj_ptr) {
// CHECK-NEXT: %[[VTABLE:.*]] = load ptr, ptr %[[OBJ_PTR_VALUE]]
// CHECK-NEXT: %[[PVDTOR:.*]] = getelementptr inbounds ptr, ptr %[[VTABLE]], i64 0
// CHECK-NEXT: %[[VDTOR:.*]] = load ptr, ptr %[[PVDTOR]]
// CHECK-NEXT: %[[CALL:.*]] = call x86_thiscallcc ptr %[[VDTOR]](ptr {{[^,]*}} %[[OBJ_PTR_VALUE]], i32 0)
// CHECK-NEXT: call void @"??3@YAXPAX@Z"(ptr %[[CALL]])
// CHECK-NEXT: %[[CALL:.*]] = call x86_thiscallcc ptr %[[VDTOR]](ptr {{[^,]*}} %[[OBJ_PTR_VALUE]], i32 5)
// CHECK: ret void
}

Expand Down Expand Up @@ -458,3 +458,41 @@ class G {
extern void testG() {
G g;
}

namespace operator_delete {

class H { virtual ~H();
void operator delete(void *);
};
H::~H() { }

void checkH() {
new H();
}
// DTORS: define linkonce_odr dso_local x86_thiscallcc ptr @"??_GH@operator_delete@@EAEPAXI@Z"(ptr {{[^,]*}} %this, i32 %should_call_delete) {{.*}} comdat {{.*}} {
// DTORS: store i32 %should_call_delete, ptr %[[SHOULD_DELETE_VAR:[0-9a-z._]+]], align 4
// DTORS: store ptr %{{.*}}, ptr %[[RETVAL:retval]]
// DTORS: %[[SHOULD_DELETE_VALUE:[0-9a-z._]+]] = load i32, ptr %[[SHOULD_DELETE_VAR]]
// DTORS: call x86_thiscallcc void @"??1H@operator_delete@@EAE@XZ"(ptr {{[^,]*}} %[[THIS:[0-9a-z]+]])
// DTORS-NEXT: %[[AND:[0-9]+]] = and i32 %[[SHOULD_DELETE_VALUE]], 1
// DTORS-NEXT: %[[CONDITION:[0-9]+]] = icmp eq i32 %[[AND]], 0
// DTORS-NEXT: br i1 %[[CONDITION]], label %[[CONTINUE_LABEL:[0-9a-z._]+]], label %[[CALL_DELETE_LABEL:[0-9a-z._]+]]
//
// DTORS: [[CALL_DELETE_LABEL]]
// DTORS-NEXT: %[[AND:[0-9]+]] = and i32 %[[SHOULD_DELETE_VALUE]], 4
// DTORS-NEXT: %[[CONDITION1:[0-9]+]] = icmp eq i32 %[[AND]], 0
// DTORS-NEXT: br i1 %[[CONDITION1]], label %[[CALL_CLASS_DELETE:[0-9a-z._]+]], label %[[CALL_GLOB_DELETE:[0-9a-z._]+]]
//
// DTORS: [[CALL_GLOB_DELETE]]
// DTORS-NEXT: call void @"??3@YAXPAX@Z"(ptr %[[THIS]])
// DTORS-NEXT: br label %[[CONTINUE_LABEL]]
//
// DTORS: [[CALL_CLASS_DELETE]]
// DTORS-NEXT: call void @"??3H@operator_delete@@CAXPAX@Z"(ptr %[[THIS]])
// DTORS-NEXT: br label %[[CONTINUE_LABEL]]
//
// DTORS: [[CONTINUE_LABEL]]
// DTORS-NEXT: %[[RET:.*]] = load ptr, ptr %[[RETVAL]]
// DTORS-NEXT: ret ptr %[[RET]]

}