https://github.com/Fznamznon created https://github.com/llvm/llvm-project/pull/185653
Currently vector deleting destructor body emission is triggered if new[] is called for the type and if the destructor or the whole class is marked with attribute dllexport. The problem is that it is not emitted if new[] is called, destructor is not exported and it is only declared in the TU and defined in another. That makes vector deleting destructor body missing which leads to runtime failures in delete[]. This change forces vector deleting destructor body emission if new[] is called even if the destructor is only declared but not defined. Doing that replicates MSVC behavior and fixes runtime issues. Since vector deleting destructors have weak linkage, it should be safe to do so. Fixes https://github.com/llvm/llvm-project/issues/183255 AI usage: Claude was used to create LIT test cases which then were reviewed and reworked by me. >From 572b12dde33c986e3dade70a25de9c0802835cfc Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Wed, 4 Mar 2026 05:21:09 -0800 Subject: [PATCH] [clang][win] Define vector deleting dtor body for declared-only dtors if needed Currently vector deleting destructor body emission is triggered if new[] is called for the type and if the destructor or the whole class is marked with attribute dllexport. The problem is that it is not emitted if new[] is called, destructor is not exported and it is only declared in the TU and defined in another. That makes vector deleting destructor body missing which leads to runtime failures in delete[]. This change forces vector deleting destructor body emission if new[] is called even if the destructor is only declared but not defined. Doing that replicates MSVC behavior and fixes runtime issues. Since vector deleting destructors have weak linkage, it should be safe to do so. Fixes https://github.com/llvm/llvm-project/issues/183255 AI usage: Claude was used to create LIT test cases which then were reviewed and reworked by me. --- clang/lib/AST/ASTContext.cpp | 2 + clang/lib/CodeGen/CGExprCXX.cpp | 4 + clang/lib/CodeGen/CodeGenModule.cpp | 5 + clang/lib/Sema/SemaDeclCXX.cpp | 2 + clang/lib/Sema/SemaExprCXX.cpp | 24 +++- ...rosoft-vector-deleting-dtors-new-array.cpp | 115 ++++++++++++++++++ clang/test/SemaCXX/gh134265.cpp | 13 ++ 7 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 5fbdff280073f..7d98764b57989 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13631,6 +13631,8 @@ bool ASTContext::classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD) { if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts())) return false; CXXDestructorDecl *Dtor = RD->getDestructor(); + if (!Dtor || !Dtor->isVirtual()) + return false; // The compiler can't know if new[]/delete[] will be used outside of the DLL, // so just force vector deleting destructor emission if dllexport is present. // This matches MSVC behavior. diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 6caef19d3be0f..17e790e3ddeb5 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1202,6 +1202,10 @@ void CodeGenFunction::EmitNewArrayInitializer( EmitCXXAggrConstructorCall(Ctor, NumElements, CurPtr, CCE, /*NewPointerIsChecked*/ true, CCE->requiresZeroInitialization()); + if (getContext().classNeedsVectorDeletingDestructor(Ctor->getParent())) { + CXXDestructorDecl *Dtor = Ctor->getParent()->getDestructor(); + CGM.EmitGlobal(GlobalDecl(Dtor, Dtor_VectorDeleting)); + } return; } diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 3b64be7a477d6..11480374697c2 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -4374,6 +4374,11 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) { // Forward declarations are emitted lazily on first use. if (!FD->doesThisDeclarationHaveABody()) { + if (auto *Dtor = dyn_cast<CXXDestructorDecl>(FD)) + if (GD.getDtorType() == Dtor_VectorDeleting && + Context.classNeedsVectorDeletingDestructor(Dtor->getParent())) + addDeferredDeclToEmit(GD); + if (!FD->doesDeclarationForceExternallyVisibleDefinition() && (!FD->isMultiVersion() || !getTarget().getTriple().isAArch64())) return; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 5837ecd6b9163..941e14d3c8af7 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -19047,6 +19047,8 @@ void Sema::MarkVTableUsed(SourceLocation Loc, CXXRecordDecl *Class, // delete(). ContextRAII SavedContext(*this, DD); CheckDestructor(DD); + if (!DD->getOperatorDelete()) + DD->setInvalidDecl(); } else { MarkFunctionReferenced(Loc, Class->getDestructor()); } diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 31b3a06bf10d0..9b77592d0ba60 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2640,11 +2640,25 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, // are required for delete[] call but MSVC triggers emission of them // whenever new[] is called for an object of the class and we do the same // for compatibility. - if (const CXXConstructExpr *CCE = - dyn_cast_or_null<CXXConstructExpr>(Initializer); - CCE && ArraySize) { - Context.setClassNeedsVectorDeletingDestructor( - CCE->getConstructor()->getParent()); + if (Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) { + if (const CXXConstructExpr *CCE = + dyn_cast_or_null<CXXConstructExpr>(Initializer); + CCE && ArraySize) { + CXXRecordDecl *ClassDecl = CCE->getConstructor()->getParent(); + auto *Dtor = ClassDecl->getDestructor(); + if (Dtor && Dtor->isVirtual() && !Dtor->isDeleted()) { + Context.setClassNeedsVectorDeletingDestructor(ClassDecl); + if (!Dtor->isDefined() && !Dtor->isInvalidDecl()) { + // Call CheckDestructor even if Destructor is not defined. This is + // needed to find operators delete and delete[] for vector deleting + // destructor body because new[] will trigger emission of vector + // deleting destructor body even if destructor is defined in another + // translation unit. + ContextRAII SavedContext(*this, Dtor); + CheckDestructor(Dtor); + } + } + } } return CXXNewExpr::Create(Context, UseGlobal, OperatorNew, OperatorDelete, diff --git a/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp b/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp new file mode 100644 index 0000000000000..fd720e5d1a7e8 --- /dev/null +++ b/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp @@ -0,0 +1,115 @@ +// RUN: %clang_cc1 -emit-llvm -fms-extensions %s -triple=x86_64-pc-windows-msvc -o - | FileCheck %s + +// Test that vector deleting destructors are emitted when new[] is used, +// even when the destructor definition is in another translation unit. + +struct ForwardDeclared { + ForwardDeclared(); + virtual ~ForwardDeclared(); +}; + +struct DefinedInTU { + virtual ~DefinedInTU(); +}; + +struct NonVirtualDtor { + ~NonVirtualDtor(); +}; + +struct NoDtor { + virtual void foo(); + int x; +}; + +struct DeclDerived : ForwardDeclared { + ~DeclDerived() override; +}; + +struct InlineDefaulted { + virtual ~InlineDefaulted() = default; +}; + +struct OutOfLineDefaulted { + virtual ~OutOfLineDefaulted(); +}; + +OutOfLineDefaulted::~OutOfLineDefaulted() = default; + +template<typename T> +struct Container { + T data; + virtual ~Container(); +}; + +extern template class Container<int>; +Container<int> *arr = new Container<int>[5]; + +struct ImplicitVDtorDerived : ForwardDeclared{ + int data; +}; + +struct __declspec(dllimport) DllImported { + virtual ~DllImported(); +}; + +struct VirtualDerived : virtual ForwardDeclared { + ~VirtualDerived() override; +}; + +struct TemplateNotAllocated { + TemplateNotAllocated(); + virtual ~TemplateNotAllocated(); +}; + +struct TemplateAllocated { + TemplateAllocated(); + virtual ~TemplateAllocated(); +}; + +template <int T> +void allocate() { + TemplateNotAllocated *arr = new TemplateNotAllocated[T]; +} + +template <typename T> +void actuallyAllocate() { + T *arr = new T[10]; + delete[] arr; +} + +void cases() { + ForwardDeclared *arr = new ForwardDeclared[5]; + DefinedInTU *arr1 = new DefinedInTU[5]; + NonVirtualDtor *arr2 = new NonVirtualDtor[5]; + NoDtor *arr3 = new NoDtor[5]; + ForwardDeclared *arr4 = new DeclDerived[5]; + InlineDefaulted *arr5 = new InlineDefaulted[5]; + OutOfLineDefaulted *arr6 = new OutOfLineDefaulted[5]; + ImplicitVDtorDerived *arr7 = new ImplicitVDtorDerived[5]; + DllImported *arr8 = new DllImported[5]; + VirtualDerived *arr9 = new VirtualDerived[3]; + actuallyAllocate<TemplateAllocated>(); +} + + +// CHECK-DAG: declare dso_local void @"??1ForwardDeclared@@UEAA@XZ"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EForwardDeclared@@UEAAPEAXI@Z"( +// CHECK-DAG: define dso_local void @"??1DefinedInTU@@UEAA@XZ"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EDefinedInTU@@UEAAPEAXI@Z"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EDeclDerived@@UEAAPEAXI@Z"( +// CHECK-DAG: declare dso_local void @"??1DeclDerived@@UEAA@XZ"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EInlineDefaulted@@UEAAPEAXI@Z"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EOutOfLineDefaulted@@UEAAPEAXI@Z"( +// CHECK-DAG: declare dso_local void @"??1?$Container@H@@UEAA@XZ"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_E?$Container@H@@UEAAPEAXI@Z"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EImplicitVDtorDerived@@UEAAPEAXI@Z"( +// CHECK-DAG: declare dllimport void @"??1DllImported@@UEAA@XZ"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EDllImported@@UEAAPEAXI@Z"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_EVirtualDerived@@UEAAPEAXI@Z"( +// CHECK-DAG: declare dso_local void @"??1TemplateAllocated@@UEAA@XZ"( +// CHECK-DAG: define weak dso_local noundef ptr @"??_ETemplateAllocated@@UEAAPEAXI@Z"( +// CHECK-NOT: @"??_ETemplateNotAllocated@@ +// CHECK-NOT: @"??_ENonVirtualDtor@@ +// CHECK-NOT: @"??_ENoDtor@@ + +DefinedInTU::~DefinedInTU() {} diff --git a/clang/test/SemaCXX/gh134265.cpp b/clang/test/SemaCXX/gh134265.cpp index 790165411c938..1c0ff6627e053 100644 --- a/clang/test/SemaCXX/gh134265.cpp +++ b/clang/test/SemaCXX/gh134265.cpp @@ -56,7 +56,20 @@ struct Final1 : BaseDelete1, BaseDelete2, BaseDestructor { }; #endif // MS +// Make sure there is no double diagnosing for delcared only destructors. +struct DeclaredOnly { + virtual ~DeclaredOnly(); // ms-error {{attempt to use a deleted function}} + static void operator delete(void* ptr) = delete; // ms-note {{explicitly marked deleted here}} +}; + +struct DeclaredOnlyArr { + virtual ~DeclaredOnlyArr(); + static void operator delete[](void* ptr) = delete; +}; + void foo() { Final* a = new Final[10](); FinalExplicit* b = new FinalExplicit[10](); + DeclaredOnly *d = new DeclaredOnly[5](); + DeclaredOnlyArr *e = new DeclaredOnlyArr[5](); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
