r274147 - Re-land "[MS] Don't expect vftables to be provided for extern template instantiations"

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 29 11:29:22 PDT 2016


Author: rnk
Date: Wed Jun 29 13:29:21 2016
New Revision: 274147

URL: http://llvm.org/viewvc/llvm-project?rev=274147&view=rev
Log:
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.

Added:
    cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp
Modified:
    cfe/trunk/lib/AST/VTableBuilder.cpp
    cfe/trunk/lib/CodeGen/CGVTables.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp

Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=274147&r1=274146&r2=274147&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Wed Jun 29 13:29:21 2016
@@ -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<DLLImportAttr>() ||
-          MostDerivedClass->getTemplateSpecializationKind() !=
-              TSK_ExplicitInstantiationDeclaration)
-        HasRTTIComponent = true;
+      HasRTTIComponent = true;
 
     LayoutVFTable();
 

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=274147&r1=274146&r2=274147&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Jun 29 13:29:21 2016
@@ -794,6 +794,10 @@ CodeGenModule::getVTableLinkage(const CX
       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
 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<DLLImportAttr>())
+  // 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

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=274147&r1=274146&r2=274147&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jun 29 13:29:21 2016
@@ -13425,20 +13425,18 @@ void Sema::MarkVTableUsed(SourceLocation
     // 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<DLLImportAttr>()) {
-        // 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());
+        }
       }
     }
   }

Added: cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp?rev=274147&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp Wed Jun 29 13:29:21 2016
@@ -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<int> has an extern template declaration, we have to emit our
+// own copy the vftable when emitting the available externally constructor.
+
+// CHECK: @"\01??_7?$Foo at H@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [
+// CHECK-SAME:   i8* bitcast (i8* (%struct.Foo*, i32)* @"\01??_G?$Foo at H@@UEAAPEAXI at Z" to i8*)
+// CHECK-SAME: ], comdat
+
+// CHECK-LABEL: define %struct.Foo* @"\01?f@@YAPEAU?$Foo at H@@XZ"()
+// CHECK: call %struct.Foo* @"\01??0?$Foo at H@@QEAA at XZ"(%struct.Foo* %{{.*}})
+
+// CHECK: define available_externally %struct.Foo* @"\01??0?$Foo at H@@QEAA at XZ"(%struct.Foo* returned %this)
+// CHECK:   store {{.*}} @"\01??_7?$Foo at H@@6B@"
+
+// CHECK: define linkonce_odr i8* @"\01??_G?$Foo at H@@UEAAPEAXI at Z"(%struct.Foo* %this, i32 %should_call_delete)
+
+struct Base {
+  virtual ~Base();
+};
+template <typename T> struct Foo : Base {
+  Foo() {}
+};
+extern template class Foo<int>;
+Foo<int> *f() { return new Foo<int>(); }

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp?rev=274147&r1=274146&r2=274147&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp Wed Jun 29 13:29:21 2016
@@ -537,5 +537,5 @@ template <class> struct B : virtual A {
 
 extern template class B<int>;
 template B<int>::B();
-// CHECK-DAG: @"\01??_8?$B at H@Test30@@7B@" = external unnamed_addr constant [2 x i32]{{$}}
+// CHECK-DAG: @"\01??_8?$B at H@Test30@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 0, i32 4], comdat
 }

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp?rev=274147&r1=274146&r2=274147&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp Wed Jun 29 13:29:21 2016
@@ -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 at Z" to i8*)]
@@ -49,5 +49,7 @@ template <class> struct Y : virtual X {
 
 extern template class Y<int>;
 template Y<int>::Y();
-// RTTI-DAG: @"\01??_7?$Y at H@@6B@" = external unnamed_addr constant [1 x i8*]
-// NO-RTTI-DAG: @"\01??_7?$Y at 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 at H@@6B@" to i8*), i8* bitcast (i8* (%struct.Y*, i32)* @"\01??_G?$Y at H@@UAEPAXI at Z" to i8*)], comdat($"\01??_7?$Y at H@@6B@")
+// RTTI-DAG: @"\01??_7?$Y at 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 at H@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [i8* bitcast (i8* (%struct.Y*, i32)* @"\01??_G?$Y at H@@UAEPAXI at Z" to i8*)], comdat




More information about the cfe-commits mailing list