From: Reid Kleckner Date: Wed, 29 Jun 2016 18:29:21 +0000 (+0000) Subject: Re-land "[MS] Don't expect vftables to be provided for extern template instantiations" X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=57eca4de7c2b16af3cf08348821b8f7ce26ff1ae;p=clang Re-land "[MS] Don't expect vftables to be provided for extern template instantiations" Reverts r273305 and re-instates r273296. We needed to fix a bug in Sema::MarkVTableUsed to ensure that operator delete lookup occurs when the vtable is referenced. We already had a special case to look up operator delete when dllimport was used, but I think should really mark virtual destructors referenced any time the vtable is used. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@274147 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp index e43acc4de7..640fbf47ae 100644 --- a/lib/AST/VTableBuilder.cpp +++ b/lib/AST/VTableBuilder.cpp @@ -2545,14 +2545,13 @@ public: MostDerivedClassLayout(Context.getASTRecordLayout(MostDerivedClass)), WhichVFPtr(*Which), Overriders(MostDerivedClass, CharUnits(), MostDerivedClass) { - // Only include the RTTI component if we know that we will provide a - // definition of the vftable. We always provide the definition of - // dllimported classes. + // Provide the RTTI component if RTTIData is enabled. If the vftable would + // be available externally, we should not provide the RTTI componenent. It + // is currently impossible to get available externally vftables with either + // dllimport or extern template instantiations, but eventually we may add a + // flag to support additional devirtualization that needs this. if (Context.getLangOpts().RTTIData) - if (MostDerivedClass->hasAttr() || - MostDerivedClass->getTemplateSpecializationKind() != - TSK_ExplicitInstantiationDeclaration) - HasRTTIComponent = true; + HasRTTIComponent = true; LayoutVFTable(); diff --git a/lib/CodeGen/CGVTables.cpp b/lib/CodeGen/CGVTables.cpp index 83974e0d21..9570550338 100644 --- a/lib/CodeGen/CGVTables.cpp +++ b/lib/CodeGen/CGVTables.cpp @@ -794,6 +794,10 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) { return DiscardableODRLinkage; case TSK_ExplicitInstantiationDeclaration: + // Explicit instantiations in MSVC do not provide vtables, so we must emit + // our own. + if (getTarget().getCXXABI().isMicrosoft()) + return DiscardableODRLinkage; return shouldEmitAvailableExternallyVTable(*this, RD) ? llvm::GlobalVariable::AvailableExternallyLinkage : llvm::GlobalVariable::ExternalLinkage; @@ -839,9 +843,9 @@ CodeGenVTables::GenerateClassData(const CXXRecordDecl *RD) { bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) { assert(RD->isDynamicClass() && "Non-dynamic classes have no VTable."); - // We always synthesize vtables on the import side regardless of whether or - // not it is an explicit instantiation declaration. - if (CGM.getTarget().getCXXABI().isMicrosoft() && RD->hasAttr()) + // We always synthesize vtables if they are needed in the MS ABI. MSVC doesn't + // emit them even if there is an explicit template instantiation. + if (CGM.getTarget().getCXXABI().isMicrosoft()) return false; // If we have an explicit instantiation declaration (and not a diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index d8bb3d0d95..948ad5fa9c 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -13425,20 +13425,18 @@ void Sema::MarkVTableUsed(SourceLocation Loc, CXXRecordDecl *Class, // checks (i.e. operator delete() lookup) when the vtable is marked used, as // the deleting destructor is emitted with the vtable, not with the // destructor definition as in the Itanium ABI. - // If it has a definition, we do the check at that point instead. if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { - if (Class->hasUserDeclaredDestructor() && - !Class->getDestructor()->isDefined() && - !Class->getDestructor()->isDeleted()) { - CXXDestructorDecl *DD = Class->getDestructor(); - ContextRAII SavedContext(*this, DD); - CheckDestructor(DD); - } else if (Class->hasAttr()) { - // We always synthesize vtables on the import side. To make sure - // CheckDestructor gets called, mark the destructor referenced. - assert(Class->getDestructor() && - "The destructor has always been declared on a dllimport class"); - MarkFunctionReferenced(Loc, Class->getDestructor()); + CXXDestructorDecl *DD = Class->getDestructor(); + if (DD && DD->isVirtual() && !DD->isDeleted()) { + if (Class->hasUserDeclaredDestructor() && !DD->isDefined()) { + // If this is an out-of-line declaration, marking it referenced will + // not do anything. Manually call CheckDestructor to look up operator + // delete(). + ContextRAII SavedContext(*this, DD); + CheckDestructor(DD); + } else { + MarkFunctionReferenced(Loc, Class->getDestructor()); + } } } } diff --git a/test/CodeGenCXX/microsoft-abi-extern-template.cpp b/test/CodeGenCXX/microsoft-abi-extern-template.cpp new file mode 100644 index 0000000000..de46d5b5c3 --- /dev/null +++ b/test/CodeGenCXX/microsoft-abi-extern-template.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -fno-rtti-data -O1 -disable-llvm-optzns %s -emit-llvm -o - -triple x86_64-windows-msvc | FileCheck %s + +// Even though Foo has an extern template declaration, we have to emit our +// own copy the vftable when emitting the available externally constructor. + +// CHECK: @"\01??_7?$Foo@H@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [ +// CHECK-SAME: i8* bitcast (i8* (%struct.Foo*, i32)* @"\01??_G?$Foo@H@@UEAAPEAXI@Z" to i8*) +// CHECK-SAME: ], comdat + +// CHECK-LABEL: define %struct.Foo* @"\01?f@@YAPEAU?$Foo@H@@XZ"() +// CHECK: call %struct.Foo* @"\01??0?$Foo@H@@QEAA@XZ"(%struct.Foo* %{{.*}}) + +// CHECK: define available_externally %struct.Foo* @"\01??0?$Foo@H@@QEAA@XZ"(%struct.Foo* returned %this) +// CHECK: store {{.*}} @"\01??_7?$Foo@H@@6B@" + +// CHECK: define linkonce_odr i8* @"\01??_G?$Foo@H@@UEAAPEAXI@Z"(%struct.Foo* %this, i32 %should_call_delete) + +struct Base { + virtual ~Base(); +}; +template struct Foo : Base { + Foo() {} +}; +extern template class Foo; +Foo *f() { return new Foo(); } diff --git a/test/CodeGenCXX/microsoft-abi-vbtables.cpp b/test/CodeGenCXX/microsoft-abi-vbtables.cpp index 9cce6f8698..df06894238 100644 --- a/test/CodeGenCXX/microsoft-abi-vbtables.cpp +++ b/test/CodeGenCXX/microsoft-abi-vbtables.cpp @@ -537,5 +537,5 @@ template struct B : virtual A { extern template class B; template B::B(); -// CHECK-DAG: @"\01??_8?$B@H@Test30@@7B@" = external unnamed_addr constant [2 x i32]{{$}} +// CHECK-DAG: @"\01??_8?$B@H@Test30@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 0, i32 4], comdat } diff --git a/test/CodeGenCXX/microsoft-abi-vftables.cpp b/test/CodeGenCXX/microsoft-abi-vftables.cpp index d94c9c1a90..0c9b58bbb4 100644 --- a/test/CodeGenCXX/microsoft-abi-vftables.cpp +++ b/test/CodeGenCXX/microsoft-abi-vftables.cpp @@ -33,7 +33,7 @@ struct __declspec(dllexport) V { namespace { struct W { - virtual ~W(); + virtual ~W() {} } w; } // RTTI-DAG: [[VTABLE_W:@.*]] = private unnamed_addr constant [2 x i8*] [i8* bitcast ({{.*}} @"\01??_R4W@?A@@6B@" to i8*), i8* bitcast ({{.*}} @"\01??_GW@?A@@UAEPAXI@Z" to i8*)] @@ -49,5 +49,7 @@ template struct Y : virtual X { extern template class Y; template Y::Y(); -// RTTI-DAG: @"\01??_7?$Y@H@@6B@" = external unnamed_addr constant [1 x i8*] -// NO-RTTI-DAG: @"\01??_7?$Y@H@@6B@" = external unnamed_addr constant [1 x i8*] +// RTTI-DAG: [[VTABLE_Y:@.*]] = private unnamed_addr constant [2 x i8*] [i8* bitcast (%rtti.CompleteObjectLocator* @"\01??_R4?$Y@H@@6B@" to i8*), i8* bitcast (i8* (%struct.Y*, i32)* @"\01??_G?$Y@H@@UAEPAXI@Z" to i8*)], comdat($"\01??_7?$Y@H@@6B@") +// RTTI-DAG: @"\01??_7?$Y@H@@6B@" = unnamed_addr alias i8*, getelementptr inbounds ([2 x i8*], [2 x i8*]* [[VTABLE_Y]], i32 0, i32 1) + +// NO-RTTI-DAG: @"\01??_7?$Y@H@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [i8* bitcast (i8* (%struct.Y*, i32)* @"\01??_G?$Y@H@@UAEPAXI@Z" to i8*)], comdat