diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def --- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def +++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def @@ -233,4 +233,8 @@ /// const-qualified reference parameter or a non-reference parameter. FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR) +/// Indicates if the complete destructor has been implicitly declared +/// yet. Only relevant in the Microsoft C++. +FIELD(DefinedImplicitCompleteDestructor, 1, MERGE_OR) + #undef FIELD diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -956,6 +956,18 @@ return data().NeedOverloadResolutionForDestructor; } + /// Indicates if the complete destructor has been implicitly declared + /// yet. Only relevant in the Microsoft C++. + bool definedImplicitCompleteDestructor() const { + return data().DefinedImplicitCompleteDestructor; + } + + void setDefinedImplicitDestructor() { + assert(!data().DefinedImplicitCompleteDestructor && + "defining complete dtor more than once"); + data().DefinedImplicitCompleteDestructor = true; + } + /// Determine whether this class describes a lambda function object. bool isLambda() const { // An update record can't turn a non-lambda into a lambda. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5559,6 +5559,11 @@ void DefineImplicitDestructor(SourceLocation CurrentLocation, CXXDestructorDecl *Destructor); + /// Define an implicit complete constructor for classes with vbases in the MS + /// ABI. + void DefineImplicitCompleteDestructor(SourceLocation CurrentLocation, + CXXDestructorDecl *Dtor); + /// Build an exception spec for destructors that don't have one. /// /// C++11 says that user-defined destructors with no exception spec get one diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -103,7 +103,8 @@ ImplicitCopyConstructorCanHaveConstParamForNonVBase(true), ImplicitCopyAssignmentHasConstParam(true), HasDeclaredCopyConstructorWithConstParam(false), - HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false), + HasDeclaredCopyAssignmentWithConstParam(false), + DefinedImplicitCompleteDestructor(false), IsLambda(false), IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false), HasODRHash(false), Definition(D) {} diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -13202,6 +13202,55 @@ } } +void Sema::DefineImplicitCompleteDestructor(SourceLocation CurrentLocation, + CXXDestructorDecl *Destructor) { + if (Destructor->isInvalidDecl()) + return; + + CXXRecordDecl *ClassDecl = Destructor->getParent(); + assert(Context.getTargetInfo().getCXXABI().isMicrosoft() && + "implicit complete dtors unneeded outside MS ABI"); + assert(ClassDecl->getNumVBases() > 0 && + "complete dtor only exists for classes with vbases"); + + SynthesizedFunctionScope Scope(*this, Destructor); + + // Add a context note for diagnostics produced after this point. + Scope.addContextNote(CurrentLocation); + + // No need to resolve the exception specification of the base destructor or + // mark vtables referenced. That will be done when the base dtor is defined. + + // Similar to what is done above in MarkBaseAndMemberDestructorsReferenced, + // but only for virtual bases. + for (const CXXBaseSpecifier &VBase : ClassDecl->vbases()) { + // Bases are always records in a well-formed non-dependent class. + CXXRecordDecl *BaseRD = VBase.getType()->getAsCXXRecordDecl(); + + // If our base class is invalid, we probably can't get its dtor anyway. + if (!BaseRD || BaseRD->isInvalidDecl() || BaseRD->hasIrrelevantDestructor()) + continue; + + CXXDestructorDecl *Dtor = LookupDestructor(BaseRD); + assert(Dtor && "No dtor found for BaseRD!"); + if (CheckDestructorAccess( + ClassDecl->getLocation(), Dtor, + PDiag(diag::err_access_dtor_vbase) + << Context.getTypeDeclType(ClassDecl) << VBase.getType(), + Context.getTypeDeclType(ClassDecl)) == AR_accessible) { + CheckDerivedToBaseConversion(Context.getTypeDeclType(ClassDecl), + VBase.getType(), diag::err_access_dtor_vbase, + 0, ClassDecl->getLocation(), SourceRange(), + DeclarationName(), nullptr); + } + + MarkFunctionReferenced(Dtor->getLocation(), Dtor); + DiagnoseUseOfDecl(Dtor, Dtor->getLocation()); + } + + ClassDecl->setDefinedImplicitDestructor(); +} + /// Perform any semantic analysis which needs to be delayed until all /// pending class member declarations have been parsed. void Sema::ActOnFinishCXXMemberDecls() { diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -15998,10 +15998,20 @@ } else if (CXXDestructorDecl *Destructor = dyn_cast(Func)) { Destructor = cast(Destructor->getFirstDecl()); - if (Destructor->isDefaulted() && !Destructor->isDeleted()) { - if (Destructor->isTrivial() && !Destructor->hasAttr()) - return; - DefineImplicitDestructor(Loc, Destructor); + if (!Destructor->isDeleted()) { + if (Destructor->isDefaulted()) { + if (Destructor->isTrivial() && + !Destructor->hasAttr()) + return; + DefineImplicitDestructor(Loc, Destructor); + } else if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { + // In the MS ABI, the complete destructor is implicitly defined, + // even if the base destructor is user defined. + CXXRecordDecl *Parent = Destructor->getParent(); + if (Parent->getNumVBases() > 0 && + !Parent->definedImplicitCompleteDestructor()) + DefineImplicitCompleteDestructor(Loc, Destructor); + } } if (Destructor->isVirtual() && getLangOpts().AppleKext) MarkVTableUsed(Loc, Destructor->getParent()); diff --git a/clang/test/CXX/class.access/p4.cpp b/clang/test/CXX/class.access/p4.cpp --- a/clang/test/CXX/class.access/p4.cpp +++ b/clang/test/CXX/class.access/p4.cpp @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s -// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s -// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify %s // C++0x [class.access]p4: diff --git a/clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp b/clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -std=c++17 -emit-llvm %s -triple x86_64-windows-msvc -o - | FileCheck %s + +// Make sure virtual base base destructors get referenced and emitted if +// necessary when the complete ("vbase") destructor is emitted. In this case, +// clang previously did not emit ~DefaultedDtor. +struct HasDtor { ~HasDtor(); }; +struct DefaultedDtor { + ~DefaultedDtor() = default; + HasDtor o; +}; +struct HasCompleteDtor : virtual DefaultedDtor { + ~HasCompleteDtor(); +}; +void useCompleteDtor(HasCompleteDtor *p) { delete p; } + +// CHECK-LABEL: define dso_local void @"?useCompleteDtor@@YAXPEAUHasCompleteDtor@@@Z"(%struct.HasCompleteDtor* %p) +// CHECK: call void @"??_DHasCompleteDtor@@QEAAXXZ"({{.*}}) + +// CHECK-LABEL: define linkonce_odr dso_local void @"??_DHasCompleteDtor@@QEAAXXZ"(%struct.HasCompleteDtor* %this) +// CHECK: call void @"??1HasCompleteDtor@@QEAA@XZ"({{.*}}) +// CHECK: call void @"??1DefaultedDtor@@QEAA@XZ"({{.*}}) + +// CHECK-LABEL: define linkonce_odr dso_local void @"??1DefaultedDtor@@QEAA@XZ"(%struct.DefaultedDtor* %this) +// CHECK: call void @"??1HasDtor@@QEAA@XZ"(%struct.HasDtor* %{{.*}}) + diff --git a/clang/test/SemaCXX/ms-implicit-complete-dtor.cpp b/clang/test/SemaCXX/ms-implicit-complete-dtor.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/ms-implicit-complete-dtor.cpp @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 -std=c++17 -verify -Wno-defaulted-function-deleted %s -triple x86_64-windows-msvc + +// MSVC emits the complete destructor as if it were its own special member. +// Clang attempts to do the same. This affects the diagnostics clang emits, +// because deleting a type with a user declared constructor implicitly +// references the destructors of virtual bases, which might be deleted or access +// controlled. + +namespace t1 { +struct A { + ~A() = delete; // expected-note {{deleted here}} +}; +struct B { // expected-error {{attempt to use a deleted function}} + B() = default; + A o; // expected-note {{destructor of 'B' is implicitly deleted because field 'o' has a deleted destructor}} +}; +struct C : virtual B { + ~C(); +}; +void delete1(C *p) { delete p; } // expected-note {{in implicit destructor for 't1::C' first required here}} +void delete2(C *p) { delete p; } +} + +namespace t2 { +struct A { +private: + ~A(); +}; +struct B { // expected-error {{attempt to use a deleted function}} + B() = default; + A o; // expected-note {{destructor of 'B' is implicitly deleted because field 'o' has an inaccessible destructor}} +}; +struct C : virtual B { + ~C(); +}; +void useCompleteDtor(C *p) { delete p; } // expected-note {{in implicit destructor for 't2::C' first required here}} +} + +namespace t3 { +template +class Base { ~Base(); }; // expected-note 1{{declared private here}} +// No diagnostic. +class Derived0 : virtual Base<0> { ~Derived0(); }; +class Derived1 : virtual Base<1> {}; +// Emitting complete dtor causes a diagnostic. +struct Derived2 : // expected-error {{inherited virtual base class 'Base<2>' has private destructor}} + virtual Base<2> { + ~Derived2(); +}; +void useCompleteDtor(Derived2 *p) { delete p; } // expected-note {{in implicit destructor for 't3::Derived2' first required here}} +}