Author: Mariya Podchishchaeva Date: 2026-03-17T11:06:38+01:00 New Revision: 9b5084f894cb941d76b4638c1305bf3f869c78d4
URL: https://github.com/llvm/llvm-project/commit/9b5084f894cb941d76b4638c1305bf3f869c78d4 DIFF: https://github.com/llvm/llvm-project/commit/9b5084f894cb941d76b4638c1305bf3f869c78d4.diff LOG: [clang][win] Define vector deleting dtor body for declared-only dtor if needed (#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. Added: clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp Modified: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/lib/CodeGen/CGExprCXX.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExprCXX.cpp clang/test/SemaCXX/gh134265.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 05302c30d18d1..8a3d202871cf8 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -389,8 +389,9 @@ class ASTContext : public RefCountedBase<ASTContext> { mutable llvm::DenseMap<const CXXDestructorDecl *, FunctionDecl *> GlobalArrayOperatorDeletesForVirtualDtor; - /// To remember which types did require a vector deleting dtor. - llvm::DenseSet<const CXXRecordDecl *> RequireVectorDeletingDtor; + /// To remember for which types we met new[] call, these potentially require a + /// vector deleting dtor. + llvm::DenseSet<const CXXRecordDecl *> MaybeRequireVectorDeletingDtor; /// The next string literal "version" to allocate during constant evaluation. /// This is used to distinguish between repeated evaluations of the same @@ -3561,8 +3562,8 @@ class ASTContext : public RefCountedBase<ASTContext> { OperatorDeleteKind K) const; bool dtorHasOperatorDelete(const CXXDestructorDecl *Dtor, OperatorDeleteKind K) const; - void setClassNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); - bool classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); + void setClassMaybeNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); + bool classMaybeNeedsVectorDeletingDestructor(const CXXRecordDecl *RD); /// Retrieve the context for computing mangling numbers in the given /// DeclContext. diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 4c672a03eb855..19e5609160a99 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13639,24 +13639,20 @@ ASTContext::getOperatorDeleteForVDtor(const CXXDestructorDecl *Dtor, return nullptr; } -bool ASTContext::classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD) { +bool ASTContext::classMaybeNeedsVectorDeletingDestructor( + const CXXRecordDecl *RD) { if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts())) return false; - CXXDestructorDecl *Dtor = RD->getDestructor(); - // 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. - if (Dtor && Dtor->isVirtual() && Dtor->hasAttr<DLLExportAttr>()) - return true; - return RequireVectorDeletingDtor.count(RD); + return MaybeRequireVectorDeletingDtor.count(RD); } -void ASTContext::setClassNeedsVectorDeletingDestructor( +void ASTContext::setClassMaybeNeedsVectorDeletingDestructor( const CXXRecordDecl *RD) { if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts())) return; - RequireVectorDeletingDtor.insert(RD); + + MaybeRequireVectorDeletingDtor.insert(RD); } MangleNumberingContext & diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 93ac0305df38f..82300c3ede183 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1201,6 +1201,12 @@ void CodeGenFunction::EmitNewArrayInitializer( EmitCXXAggrConstructorCall(Ctor, NumElements, CurPtr, CCE, /*NewPointerIsChecked*/ true, CCE->requiresZeroInitialization()); + if (getContext().getTargetInfo().emitVectorDeletingDtors( + getContext().getLangOpts())) { + CXXDestructorDecl *Dtor = Ctor->getParent()->getDestructor(); + if (Dtor && Dtor->isVirtual()) + CGM.requireVectorDestructorDefinition(Ctor->getParent()); + } return; } diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index dfec2ca00d9a7..43af159305953 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -8540,3 +8540,55 @@ std::string CodeGenModule::getPFPFieldName(const FieldDecl *FD) { Out << "." << FD->getName(); return OutName; } + +bool CodeGenModule::classNeedsVectorDestructor(const CXXRecordDecl *RD) { + if (!Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) + return false; + CXXDestructorDecl *Dtor = RD->getDestructor(); + // 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. + if (Dtor && Dtor->isVirtual() && Dtor->hasAttr<DLLExportAttr>()) + return true; + + return RequireVectorDeletingDtor.count(RD); +} + +void CodeGenModule::requireVectorDestructorDefinition(const CXXRecordDecl *RD) { + if (!Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) + return; + RequireVectorDeletingDtor.insert(RD); + + // To reduce code size in general case we lazily emit scalar deleting + // destructor definition and an alias from vector deleting destructor to + // scalar deleting destructor. It may happen that we first emitted the scalar + // deleting destructor definition and the alias and then discovered that the + // definition of the vector deleting destructor is required. Then we need to + // remove the alias and the scalar deleting destructor and queue vector + // deleting destructor body for emission. Check if that is the case. + CXXDestructorDecl *DtorD = RD->getDestructor(); + GlobalDecl ScalarDtorGD(DtorD, Dtor_Deleting); + StringRef MangledName = getMangledName(ScalarDtorGD); + llvm::GlobalValue *Entry = GetGlobalValue(MangledName); + GlobalDecl VectorDtorGD(DtorD, Dtor_VectorDeleting); + if (Entry && !Entry->isDeclaration()) { + StringRef VDName = getMangledName(VectorDtorGD); + llvm::GlobalValue *VDEntry = GetGlobalValue(VDName); + // It exists and it should be an alias. + assert(VDEntry && isa<llvm::GlobalAlias>(VDEntry)); + auto *NewFn = llvm::Function::Create( + cast<llvm::FunctionType>(VDEntry->getValueType()), + llvm::Function::ExternalLinkage, VDName, &getModule()); + SetFunctionAttributes(VectorDtorGD, NewFn, /*IsIncompleteFunction*/ false, + /*IsThunk*/ false); + NewFn->takeName(VDEntry); + VDEntry->replaceAllUsesWith(NewFn); + VDEntry->eraseFromParent(); + Entry->replaceAllUsesWith(NewFn); + Entry->eraseFromParent(); + } + // Always add a deferred decl to emit once we confirmed that vector deleting + // destructor definition is required. That helps to enforse its generation + // even if destructor is only declared. + addDeferredDeclToEmit(VectorDtorGD); +} diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 0081bf5c4cf5f..0a697c84b66a7 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -529,6 +529,11 @@ class CodeGenModule : public CodeGenTypeCache { /// that we don't re-emit the initializer. llvm::DenseMap<const Decl*, unsigned> DelayedCXXInitPosition; + /// To remember which types did require a vector deleting destructor body. + /// This set basically contains classes that have virtual destructor and new[] + /// was emitted for the class. + llvm::SmallPtrSet<const CXXRecordDecl *, 16> RequireVectorDeletingDtor; + typedef std::pair<OrderGlobalInitsOrStermFinalizers, llvm::Function *> GlobalInitData; @@ -1578,6 +1583,13 @@ class CodeGenModule : public CodeGenTypeCache { /// are emitted lazily. void EmitGlobal(GlobalDecl D); + /// Record that new[] was called for the class, transform vector deleting + /// destructor definition in a form of alias to the actual definition. + void requireVectorDestructorDefinition(const CXXRecordDecl *RD); + + /// Check that class need vector deleting destructor body. + bool classNeedsVectorDestructor(const CXXRecordDecl *RD); + bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D); void EmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 06fce6171eb28..d959b89f860e4 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -4107,7 +4107,7 @@ void MicrosoftCXXABI::emitCXXStructor(GlobalDecl GD) { return; if (GD.getDtorType() == Dtor_VectorDeleting && - !getContext().classNeedsVectorDeletingDestructor(dtor->getParent())) { + !CGM.classNeedsVectorDestructor(dtor->getParent())) { // Create GlobalDecl object with the correct type for the scalar // deleting destructor. GlobalDecl ScalarDtorGD(dtor, Dtor_Deleting); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 20e4663d670b2..2a24ee42bcb4d 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -11273,6 +11273,7 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { if (Context.getTargetInfo().emitVectorDeletingDtors( Context.getLangOpts())) { + bool DestructorIsExported = Destructor->hasAttr<DLLExportAttr>(); // Lookup delete[] too in case we have to emit a vector deleting dtor. DeclarationName VDeleteName = Context.DeclarationNames.getCXXOperatorName(OO_Array_Delete); @@ -11286,7 +11287,8 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { VDeleteName); Destructor->setGlobalOperatorArrayDelete(GlobalArrOperatorDelete); if (GlobalArrOperatorDelete && - Context.classNeedsVectorDeletingDestructor(RD)) + (Context.classMaybeNeedsVectorDeletingDestructor(RD) || + DestructorIsExported)) MarkFunctionReferenced(Loc, GlobalArrOperatorDelete); } else if (!ArrOperatorDelete) { ArrOperatorDelete = FindDeallocationFunctionForDestructor( @@ -11294,7 +11296,9 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) { /*LookForGlobal*/ true, VDeleteName); } Destructor->setOperatorArrayDelete(ArrOperatorDelete); - if (ArrOperatorDelete && Context.classNeedsVectorDeletingDestructor(RD)) + if (ArrOperatorDelete && + (Context.classMaybeNeedsVectorDeletingDestructor(RD) || + DestructorIsExported)) MarkFunctionReferenced(Loc, ArrOperatorDelete); } } @@ -19133,6 +19137,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 5a5bbf4d900dc..5de4a1e7475f2 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -2636,17 +2636,31 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, MarkFunctionReferenced(StartLoc, OperatorDelete); } - // For MSVC vector deleting destructors support we record that for the class - // new[] was called. We try to optimize the code size and only emit vector - // deleting destructors when they are required. Vector deleting destructors - // 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()); + // new[] will trigger vector deleting destructor emission if the class has + // virtual destructor for MSVC compatibility. Perform necessary checks. + if (Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts())) { + if (const CXXConstructExpr *CCE = + dyn_cast_or_null<CXXConstructExpr>(Initializer); + CCE && ArraySize) { + CXXRecordDecl *ClassDecl = CCE->getConstructor()->getParent(); + // We probably already did this for another new[] with this class so don't + // do it twice. + if (!Context.classMaybeNeedsVectorDeletingDestructor(ClassDecl)) { + auto *Dtor = ClassDecl->getDestructor(); + if (Dtor && Dtor->isVirtual() && !Dtor->isDeleted()) { + Context.setClassMaybeNeedsVectorDeletingDestructor(ClassDecl); + if (!Dtor->isDefined() && !Dtor->isInvalidDecl()) { + // Call CheckDestructor 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..b8b6e44b6b2f8 --- /dev/null +++ b/clang/test/CodeGenCXX/microsoft-vector-deleting-dtors-new-array.cpp @@ -0,0 +1,122 @@ +// 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 DeclaredCtorDefinedDtor { + DeclaredCtorDefinedDtor(); + virtual ~DeclaredCtorDefinedDtor() {} +}; + +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]; + DeclaredCtorDefinedDtor *arr10 = new DeclaredCtorDefinedDtor[5]; + 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: define weak dso_local noundef ptr @"??_EDeclaredCtorDefinedDtor@@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..421197cf3d7f7 100644 --- a/clang/test/SemaCXX/gh134265.cpp +++ b/clang/test/SemaCXX/gh134265.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -triple=x86_64-unknown-linux-gnu -// RUN: %clang_cc1 %s -verify=expected -fsyntax-only -triple=x86_64-unknown-linux-gnu -std=c++20 +// RUN: %clang_cc1 %s -verify=expected,noms -fsyntax-only -triple=x86_64-unknown-linux-gnu +// RUN: %clang_cc1 %s -verify=expected,noms -fsyntax-only -triple=x86_64-unknown-linux-gnu -std=c++20 // RUN: %clang_cc1 %s -verify=expected,ms -fms-extensions -fms-compatibility -triple=x86_64-pc-windows-msvc -DMS // Verify that clang doesn't emit additional errors when searching for @@ -56,7 +56,34 @@ struct Final1 : BaseDelete1, BaseDelete2, BaseDestructor { }; #endif // MS +// Make sure there is no double diagnosing for declared-only destructors and +// new[]. +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](); } + +// Make sure there is no double diagnosing for forward declared destructors +// and new[]. +namespace std { struct destroying_delete_t {}; } +struct A { + void operator delete( + A*, //expected-error {{cannot cast 'D' to its private base class 'A'}} + std::destroying_delete_t); +}; +struct B : private A { using A::operator delete; }; //expected-note {{declared private here}} +struct D : B { virtual ~D(); }; //ms-note {{while checking implicit 'delete this' for virtual destructor}} +void f() { new D[5]; } +D::~D() {} // noms-note {{while checking implicit 'delete this' for virtual destructor}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
