[MSVC][DLL] use local vftable mangling only exported classes with virtual destructor

[DmitryPolukhin]

Summary

MSVC uses non-local mangling for vftable if class with dllexport attribute has no virtual destructor. I checked all examples that I fixed with MSVC and it uses '_7' in all cases. For more info see old thread in microsoft.public.vc.language: https://groups.google.com/d/msg/microsoft.public.vc.language/atSh_2VSc2w/EgJ3r_7OzVUJ

[majnemer]

Hmm, I'm not so sure this will work with constexpr:

#include <stdio.h>
struct __declspec(dllimport) S {
    virtual void fn() const {printf("%s\n", "hi");}
    constexpr S() = default;
};

constexpr S s;
auto &x = s;

int main() {
    x.fn();
}

Before my change we would reference the imported vftable symbol from the .rdata section which will fail at link time.
IIRC, MSVC will miscompile this example.

Your change, if I understand correctly, will cause us to import the vftable symbol which would cause this example to break again.

[majnemer]

FWIW, I think that we would be OK to import vftables (and thus use the _7 mangling) in contexts where their construction doesn't need to be constant. It seemed pretty complex to implement but I'd be open to such an implementation.

[DmitryPolukhin]

David, do you know real programs that relay on constexpr and dllexport semantic that doesn't work on MSVC? If not, I think we might want to report error message instead of miss-compile it as MSVC does. Anyway current implementation is not compatible with MSVC in much more common case without constexp. At the moment my patch works with your example just because it only changes mangling but don't use imported vftable that seems to be wrong. Is it you changes somewhere that prevent using imported vftable?

[DmitryPolukhin]

I found the patch that prevents using imported vftable, http://llvm.org/viewvc/llvm-project?view=revision&revision=260548 but the patch has no examples of constexpr + dllimport.

[majnemer]

In D22034#474937, @DmitryPolukhin wrote:

David, do you know real programs that relay on constexpr and dllexport semantic that doesn't work on MSVC?

Yes, Chrome relied on these semantics.

Anyway current implementation is not compatible with MSVC in much more common case without constexp.

I don't see how it is not compatible. The address of the vftable is not observable as far as I know...

At the moment my patch works with your example just because it only changes mangling but don't use imported vftable that seems to be wrong.

Why is it wrong? It should obey the ABI in all practical matters but it might be less efficient space-wise.

[DmitryPolukhin]

In D22034#474985, @majnemer wrote:

In D22034#474937, @DmitryPolukhin wrote:

David, do you know real programs that relay on constexpr and dllexport semantic that doesn't work on MSVC?

Yes, Chrome relied on these semantics.

Hm, does it mean that Chrome has some workaround to bypass that MSVC doesn't support it?

Anyway current implementation is not compatible with MSVC in much more common case without constexp.

I don't see how it is not compatible. The address of the vftable is not observable as far as I know...

You can easily observe the difference if vftable is exported from DLL but some virtual functions are not (for example, private). MSVC will uses imported vftable and Clang will try to rebuild fvtable that will fail on link time.

At the moment my patch works with your example just because it only changes mangling but don't use imported vftable that seems to be wrong.

Why is it wrong? It should obey the ABI in all practical matters but it might be less efficient space-wise.

I think it is not ABI compatible use local vftable when MSVC uses imported. The easy way to observe the difference I already described but it might be other side effect if user start unloading DLL from address space. So user may expect that it is safe to unload library #1 that created object from library #2 because it doesn't have references to library #1 addresses but due to local fvtable it might result in crash.

[rnk]

In D22034#474889, @majnemer wrote:

Hmm, I'm not so sure this will work with constexpr:
...

Hah, my version of MSVC fails to install the vtable into 's', and crashes on the call to x.fn().

[majnemer]

In D22034#475002, @DmitryPolukhin wrote:

In D22034#474985, @majnemer wrote:

In D22034#474937, @DmitryPolukhin wrote:

David, do you know real programs that relay on constexpr and dllexport semantic that doesn't work on MSVC?

Yes, Chrome relied on these semantics.

Hm, does it mean that Chrome has some workaround to bypass that MSVC doesn't support it?

Not an explicit workaround, other MSVC bugs prevent them from miscompiling the exact sequence used in Chrome.

Anyway current implementation is not compatible with MSVC in much more common case without constexp.

I don't see how it is not compatible. The address of the vftable is not observable as far as I know...

You can easily observe the difference if vftable is exported from DLL but some virtual functions are not (for example, private). MSVC will uses imported vftable and Clang will try to rebuild fvtable that will fail on link time.

Wait, can you give an example of MSVC exporting a vftable but not all the virtual methods (other than the deleting destructor)? I don't believe I've ever come across an example of this.

[DmitryPolukhin]

In D22034#475331, @majnemer wrote:

Wait, can you give an example of MSVC exporting a vftable but not all the virtual methods (other than the deleting destructor)? I don't believe I've ever come across an example of this.

It is possible in code like this:
class B {

virtual void foo();

public:

virtual void bar();

};

class __declspec(dllexport) D : public B {
public:

virtual void bar();

};

void B::foo() {}
void B::bar() {}
void D::bar() {}

Here is B::foo is not exported but required to build vftable for D. Also user may want to explicitly control what should be exported from his library and may decide to remove some functions from exported interface.

[majnemer]

In D22034#475540, @DmitryPolukhin wrote:

In D22034#475331, @majnemer wrote:

Wait, can you give an example of MSVC exporting a vftable but not all the virtual methods (other than the deleting destructor)? I don't believe I've ever come across an example of this.

It is possible in code like this:
class B {

virtual void foo();

public:

virtual void bar();

};

class __declspec(dllexport) D : public B {
public:

virtual void bar();

};

void B::foo() {}
void B::bar() {}
void D::bar() {}

Here is B::foo is not exported but required to build vftable for D.

What happens if D also had a virtual destructor?

[rnk]

In D22034#475540, @DmitryPolukhin wrote:

Here is B::foo is not exported but required to build vftable for D. Also user may want to explicitly control what should be exported from his library and may decide to remove some functions from exported interface.

Sure, but there are other ways for clients of D to end up referencing the unexported symbol for B::foo, such as devirtualization. Surely you aren't proposing that we power that down just so that we can get the same import/export list as MSVC?

[majnemer]

Thinking about this some more, it is possible for clang to emit code that will make everybody happy:

If a class is being constructed in a constexpr context and all the vftable entries it references are marked import, emit local vftables and reference them in the object. If a vftable entry it references is not marked import, report an error.

If a class is constructed via operator new and all the vftable entries it references are marked import, emit local vftables and store it in the object after the constructors run. If a vftable entry it references is not marked import, report an error.

If a class is constructed via a local or global variable and all the vftable entries it references are marked import, create a dllimport available_externally vftable. Otherwise create a normal dllimport external vftable.

I believe this behavior captures the best behavior across the spectrum of functionality we all care about: it supports devirtualization, constexpr and importing classes which don't have all their vftable methods exported.

[DmitryPolukhin]

In D22034#475551, @majnemer wrote:

In D22034#475540, @DmitryPolukhin wrote:

Here is B::foo is not exported but required to build vftable for D.

What happens if D also had a virtual destructor?

It seems that in this case MSVC never inline x-tors so there is no problem with using local vftable. I don't propose to do the same inline decisions like MSVC does so Clang may need/use different set of exported members and MSVC depending on optimization levels and other things may change these sets. But it seems that never use exported vftable is not aligned with ABI. MSVC seems to be very consistent about uses exported vftable when it is possible.

[DmitryPolukhin]

In D22034#475603, @majnemer wrote:

Thinking about this some more, it is possible for clang to emit code that will make everybody happy:

If a class is being constructed in a constexpr context and all the vftable entries it references are marked import, emit local vftables and reference them in the object. If a vftable entry it references is not marked import, report an error.

If a class is constructed via operator new and all the vftable entries it references are marked import, emit local vftables and store it in the object after the constructors run. If a vftable entry it references is not marked import, report an error.

If a class is constructed via a local or global variable and all the vftable entries it references are marked import, create a dllimport available_externally vftable. Otherwise create a normal dllimport external vftable.

I believe this behavior captures the best behavior across the spectrum of functionality we all care about: it supports devirtualization, constexpr and importing classes which don't have all their vftable methods exported.

I'll try to implement this approach except for special handling for constriction via new that seems to be out of scope for this issue and can be implemented independent later. Small addition, I think there is no sense to check if vftable references to entry that is not marked as dllimport. Linker will do this work for us and in some cases function may come from libraries (static or dynamic) but in the source may not be properly marked as dllimport (it is the behavior that MSVC does check it in compiler and relays on linker).

[DmitryPolukhin]

Sorry for delay with patch rework, PTAL!

• added local vftable for constexpr
• added use of imported vftable

[majnemer]

Running your patch against the following:

struct __declspec(dllimport) S {
  virtual void f() {}
};
S x;
constexpr S y;

results in:

Global is marked as dllimport, but not external
[2 x i8*]* @"\01??_7S@@6B@"
fatal error: error in backend: Broken module found, compilation aborted!
clang-3.9: error: clang frontend command failed with exit code 70 (use -v to see invocation)

I don't think your approach will correctly handle instances where we need to emit a local vftable and reference an imported vftable simultaneously.

A flag on CXXRecordDecl which is sensitive to the most recent expression evaluation might not be the best way to go.
Perhaps we should be able to use the VFTableBuilder to build imported and local vftables for the same vftable? Not entirely sure though...

lib/AST/ExprConstant.cpp

5765

I'd use auto * here.

[rnk]

lib/AST/ExprConstant.cpp

5766

Even better:

if (auto *CD = E->getType()->getAsCXXRecordDecl())

[DmitryPolukhin]

In D22034#482925, @majnemer wrote:

A flag on CXXRecordDecl which is sensitive to the most recent expression evaluation might not be the best way to go.
Perhaps we should be able to use the VFTableBuilder to build imported and local vftables for the same vftable? Not entirely sure though...

The flag can be only set to true, there is no way to reset it false. So by the end of translation unit it should have proper value and many things are deferred until the end of translation unit (like CodeGenModule::EmitDeferredVTables). But your example shows that normal var generation is not deferred, I missed it. Deferring var generation in CodeGenModule::MayBeEmittedEagerly seems to work fine.

[DmitryPolukhin]

• defer var generation if their type is class with dllimport.

[rnk]

The approach makes sense to me, but the tests suggest it isn't doing what I'd expect.

test/CodeGenCXX/dllimport-rtti.cpp

7

I would've expected this to remain the same, since the implicit default ctor of 'S' is constexpr by default in C++14. It seems a lot better to emit a local vftable here and get static initialization for 's' than dynamic initialization.

[DmitryPolukhin]

test/CodeGenCXX/dllimport-rtti.cpp

7

The context of evaluation of the whole expression is not constexpr so this case can be done both ways. But implemented approach is how MSVC behaves in this case. MSVC has very predictable behavior when local vftable is used - only when class has virtual d-tor. Current Clang behavior is also very consistent - always use local vftable. But this patch makes it hard to predict which table will be used - it becomes use context sensitive instead of depending only on class declaration. Therefore different translation units could use different tables and it could cause strange artifacts. Using dllimported classes in pure constexpr case in my experience is very rare case so it was kind of fine but implicit constructors much more common case.

Also thinking more about my patch I realized that fix in MayBeEmittedEagerly doesn't work if dllimported class is a member of non-imported class so actual fix would require traversing for all base classes and members for the type.

So my proposal is to keep things as is for now and abandon this patch if you have no objection.

[rnk]

test/CodeGenCXX/dllimport-rtti.cpp

7

Sounds good, we also thought this was a reasonable compromise position last time we touched this.

include/clang/AST/DeclCXX.h

struct DefinitionData {
     unsigned HasDeclaredCopyAssignmentWithConstParam : 1;
 
     /// \brief Whether this class describes a C++ lambda.
     unsigned IsLambda : 1;
 
     /// \brief Whether we are currently parsing base specifiers.
     unsigned IsParsingBaseSpecifiers : 1;
 
+    /// \brief Whether class with dllimport attribute needs local vftable.
+    unsigned NeedLocalVFTable : 1;
+
     /// \brief The number of base class specifiers in Bases.
     unsigned NumBases;
 
     /// \brief The number of virtual base class specifiers in VBases.
     unsigned NumVBases;
 
     /// \brief Base classes of this class.
     ///
public:
   }
 
   void setIsParsingBaseSpecifiers() { data().IsParsingBaseSpecifiers = true; }
 
   bool isParsingBaseSpecifiers() const {
     return data().IsParsingBaseSpecifiers;
   }
 
+  void setNeedLocalVFTable() { data().NeedLocalVFTable = true; }
+  bool getNeedLocalVFTable() const { return data().NeedLocalVFTable; }
+
   /// \brief Sets the base classes of this struct or class.
   void setBases(CXXBaseSpecifier const * const *Bases, unsigned NumBases);
 
   /// \brief Retrieves the number of base classes of this class.
   unsigned getNumBases() const { return data().NumBases; }
 
   typedef llvm::iterator_range<base_class_iterator> base_class_range;
   typedef llvm::iterator_range<base_class_const_iterator>

lib/AST/DeclCXX.cpp

: UserDeclaredConstructor(false), UserDeclaredSpecialMembers(0),
       DefaultedDefaultConstructorIsConstexpr(true),
       HasConstexprDefaultConstructor(false),
       HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false),
       UserProvidedDefaultConstructor(false), DeclaredSpecialMembers(0),
       ImplicitCopyConstructorHasConstParam(true),
       ImplicitCopyAssignmentHasConstParam(true),
       HasDeclaredCopyConstructorWithConstParam(false),
       HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
-      IsParsingBaseSpecifiers(false), NumBases(0), NumVBases(0), Bases(),
-      VBases(), Definition(D), FirstFriend() {}
+      IsParsingBaseSpecifiers(false), NeedLocalVFTable(false), NumBases(0),
+      NumVBases(0), Bases(), VBases(), Definition(D), FirstFriend() {}
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
   return Bases.get(Definition->getASTContext().getExternalSource());
 }
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getVBasesSlowCase() const {
   return VBases.get(Definition->getASTContext().getExternalSource());
 }

lib/AST/ExprConstant.cpp

bool RecordExprEvaluator::VisitCXXStdInitializerListExpr(
 
   return true;
 }
 
 static bool EvaluateRecord(const Expr *E, const LValue &This,
                            APValue &Result, EvalInfo &Info) {
   assert(E->isRValue() && E->getType()->isRecordType() &&
          "can't evaluate expression as a record rvalue");
+  // The class will need local vftable because true const cannot use dllimported
+  // load time const.
+  if (Info.EvalMode == EvalInfo::EM_ConstantExpression)
+    if (auto *CD = E->getType()->getAsCXXRecordDecl())
+      CD->setNeedLocalVFTable();
   return RecordExprEvaluator(Info, This, Result).Visit(E);
 }
 
 //===----------------------------------------------------------------------===//
 // Temporary Evaluation
 //
 // Temporaries are represented in the AST as rvalues, but generally behave like
 // lvalues. The full-object of which the temporary is a subobject is implicitly

lib/AST/MicrosoftMangle.cpp

void MicrosoftMangleContextImpl::mangleCXXVFTable(
     const CXXRecordDecl *Derived, ArrayRef<const CXXRecordDecl *> BasePath,
     raw_ostream &Out) {
   // <mangled-name> ::= ?_7 <class-name> <storage-class>
   //                    <cvr-qualifiers> [<name>] @
   // NOTE: <cvr-qualifiers> here is always 'B' (const). <storage-class>
   // is always '6' for vftables.
   msvc_hashing_ostream MHO(Out);
   MicrosoftCXXNameMangler Mangler(*this, MHO);
-  if (Derived->hasAttr<DLLImportAttr>())
+  if (Derived->hasAttr<DLLImportAttr>() && Derived->getNeedLocalVFTable())
     Mangler.getStream() << "\01??_S";
   else
     Mangler.getStream() << "\01??_7";
   Mangler.mangleName(Derived);
   Mangler.getStream() << "6B"; // '6' for vftable, 'B' for const.
   for (const CXXRecordDecl *RD : BasePath)
     Mangler.mangleName(RD);
   Mangler.getStream() << '@';

lib/CodeGen/CGVTables.cpp

 /// not contain a vtable definition when that vtable is defined
 /// strongly elsewhere.  Otherwise, we'd just like to avoid emitting
 /// vtables when unnecessary.
 bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
   assert(RD->isDynamicClass() && "Non-dynamic classes have no VTable.");
 
   // 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 (CGM.getTarget().getCXXABI().isMicrosoft()) {
+    // But try to use dllexported vftable if we don't need local one in some
+    // context.
+    return RD->hasAttr<DLLImportAttr>() && !RD->getNeedLocalVFTable();
+  }
 
   // If we have an explicit instantiation declaration (and not a
   // definition), the vtable is defined elsewhere.
   TemplateSpecializationKind TSK = RD->getTemplateSpecializationKind();
   if (TSK == TSK_ExplicitInstantiationDeclaration)
     return true;
 
   // Otherwise, if the class is an instantiated template, the

lib/CodeGen/CodeGenModule.cpp

 }
 
 bool CodeGenModule::MayBeEmittedEagerly(const ValueDecl *Global) {
   if (const auto *FD = dyn_cast<FunctionDecl>(Global))
     if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
       // Implicit template instantiations may change linkage if they are later
       // explicitly instantiated, so they should not be emitted eagerly.
       return false;
-  if (const auto *VD = dyn_cast<VarDecl>(Global))
+  if (const auto *VD = dyn_cast<VarDecl>(Global)) {
     if (Context.getInlineVariableDefinitionKind(VD) ==
         ASTContext::InlineVariableDefinitionKind::WeakUnknown)
       // A definition of an inline constexpr static data member may change
       // linkage later if it's redeclared outside the class.
       return false;
+    // In Microsoft ABI mode we have to defer variable definition to check
+    // if class needs local vftable.
+    if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+      if (const auto *CD = VD->getType()->getAsCXXRecordDecl())
+        if (CD->hasAttr<DLLImportAttr>())
+          return false;
+  }
   // If OpenMP is enabled and threadprivates must be generated like TLS, delay
   // codegen for global variables, because they may be marked as threadprivate.
   if (LangOpts.OpenMP && LangOpts.OpenMPUseTLS &&
       getContext().getTargetInfo().isTLSSupported() && isa<VarDecl>(Global))
     return false;
 
   return true;
 }

lib/CodeGen/MicrosoftCXXABI.cpp

#endif
 
   // Classes marked __declspec(dllimport) need vftables generated on the
   // import-side in order to support features like constexpr.  No other
   // translation unit relies on the emission of the local vftable, translation
   // units are expected to generate them as needed.
   //
   // Because of this unique behavior, we maintain this logic here instead of
   // getVTableLinkage.
-  llvm::GlobalValue::LinkageTypes VFTableLinkage =
-      RD->hasAttr<DLLImportAttr>() ? llvm::GlobalValue::LinkOnceODRLinkage
-                                   : CGM.getVTableLinkage(RD);
+  llvm::GlobalValue::LinkageTypes VFTableLinkage = CGM.getVTableLinkage(RD);
+  if (RD->hasAttr<DLLImportAttr>()) {
+    if (RD->getNeedLocalVFTable())
+      VFTableLinkage = llvm::GlobalValue::LinkOnceODRLinkage;
+    else
+      VFTableLinkage = llvm::GlobalValue::ExternalLinkage;
+  }
   bool VFTableComesFromAnotherTU =
       llvm::GlobalValue::isAvailableExternallyLinkage(VFTableLinkage) ||
       llvm::GlobalValue::isExternalLinkage(VFTableLinkage);
   bool VTableAliasIsRequred =
       !VFTableComesFromAnotherTU && getContext().getLangOpts().RTTIData;
 
   if (llvm::GlobalValue *VFTable =
           CGM.getModule().getNamedGlobal(VFTableName)) {
if (VTableAliasIsRequred) {
     // be referencing any RTTI data.
     // The GlobalVariable will end up being an appropriate definition of the
     // VFTable.
     VFTable = VTable;
   }
   if (C)
     VTable->setComdat(C);
 
-  if (RD->hasAttr<DLLExportAttr>())
+  if (RD->hasAttr<DLLImportAttr>() && !RD->getNeedLocalVFTable())
+    VFTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+  else if (RD->hasAttr<DLLExportAttr>())
     VFTable->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
 
   VFTablesMap[ID] = VFTable;
   return VTable;
 }
 
 // Compute the identity of the most derived class whose virtual table is located
 // at the given offset into RD.

lib/Sema/SemaDeclCXX.cpp

for (Decl *Member : Class->decls()) {
     if (!getDLLAttr(Member)) {
       auto *NewAttr =
           cast<InheritableAttr>(ClassAttr->clone(getASTContext()));
       NewAttr->setInherited(true);
       Member->addAttr(NewAttr);
     }
   }
 
+  if (!ClassExported) {
+    if (Class->getTemplateSpecializationKind() != TSK_Undeclared)
+      Class->setNeedLocalVFTable();
+    if (const CXXDestructorDecl *Destructor = Class->getDestructor()) {
+      // Classes with dllimport attribute needs local vftable if they have
+      // virtual d-tor.
+      if (Destructor->isVirtual())
+        Class->setNeedLocalVFTable();
+    }
+  }
+
   if (ClassExported)
     DelayedDllExportClasses.push_back(Class);
 }
 
 /// \brief Perform propagation of DLL attributes from a derived class to a
 /// templated base class for MS compatibility.
 void Sema::propagateDLLAttrToBaseClassTemplate(
     CXXRecordDecl *Class, Attr *ClassAttr,

test/CodeGenCXX/dllimport-rtti.cpp

 // RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++1y -fms-extensions -O1 -disable-llvm-optzns -o - %s | FileCheck %s --check-prefix=MSVC
 // RUN: %clang_cc1 -triple i686-windows-gnu  -emit-llvm -std=c++1y -fms-extensions -O1 -disable-llvm-optzns -o - %s | FileCheck %s --check-prefix=GNU
 
 struct __declspec(dllimport) S {
   virtual void f() {}
 } s;
-// MSVC: [[VF_S:.*]] = private unnamed_addr constant [2 x i8*]
-// MSVC-DAG: @"\01??_SS@@6B@" = unnamed_addr alias i8*, getelementptr inbounds ([2 x i8*], [2 x i8*]* [[VF_S]], i32 0, i32 1)
+// MSVC-DAG: @"\01??_7S@@6B@" = external dllimport unnamed_addr constant [2 x i8*]
 // MSVC-DAG: @"\01??_R0?AUS@@@8" = linkonce_odr
 // MSVC-DAG: @"\01??_R1A@?0A@EA@S@@8" = linkonce_odr
 // MSVC-DAG: @"\01??_R2S@@8" = linkonce_odr
 // MSVC-DAG: @"\01??_R3S@@8" = linkonce_odr
 
 // GNU-DAG: @_ZTV1S = available_externally dllimport
 // GNU-DAG: @_ZTI1S = external dllimport
 
+struct __declspec(dllimport) S2 {
+  virtual void f() {}
+  virtual ~S2() {}
+} s2;
+// MSVC-DAG: [[VF_S2:.*]] = private unnamed_addr constant [3 x i8*] [i8* bitcast (%rtti.CompleteObjectLocator* @"\01??_R4S2@@6B@" to i8*)
+// MSVC-DAG: @"\01??_SS2@@6B@" = unnamed_addr alias i8*, getelementptr inbounds ([3 x i8*], [3 x i8*]* [[VF_S2]], i32 0, i32 1)
+// MSVC-DAG: @"\01??_R0?AUS2@@@8" = linkonce_odr
+// MSVC-DAG: @"\01??_R1A@?0A@EA@S2@@8" = linkonce_odr
+// MSVC-DAG: @"\01??_R2S2@@8" = linkonce_odr
+
 struct U : S {
 } u;
 
 struct __declspec(dllimport) V {
   virtual void f();
 } v;
 // GNU-DAG: @_ZTV1V = available_externally dllimport
 // GNU-DAG: @_ZTS1V = linkonce_odr

test/CodeGenCXX/dllimport.cpp

 template <typename T> struct __declspec(dllimport) U { void foo() {} };
 // MO1-DAG: define available_externally dllimport x86_thiscallcc void @"\01?foo@?$U@H@@QAEXXZ"
 struct __declspec(dllimport) V : public U<int> { };
 USEMEMFUNC(V, foo)
 
 struct __declspec(dllimport) W { virtual void foo() {} };
 USECLASS(W)
 // vftable:
-// MO1-DAG: @"\01??_SW@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [i8* bitcast (void (%struct.W*)* @"\01?foo@W@@UAEXXZ" to i8*)]
+// MO1-DAG: @"\01??_7W@@6B@" = external dllimport unnamed_addr constant [1 x i8*]
 // GO1-DAG: @_ZTV1W = available_externally dllimport unnamed_addr constant [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.W*)* @_ZN1W3fooEv to i8*)]
 
+struct __declspec(dllimport) W2 {
+  virtual ~W2();
+  virtual void foo() {}
+};
+USECLASS(W2)
+// vftable:
+// MO1-DAG: @"\01??_SW2@@6B@" = linkonce_odr unnamed_addr constant [2 x i8*]
+// GO1-DAG: @_ZTV2W2 = external dllimport unnamed_addr constant [5 x i8*]
+
+struct __declspec(dllimport) W3 {
+  virtual void fn() const {
+  }
+  constexpr W3() = default;
+};
+
+W3 w3;
+W3& w3r = w3;
+
+constexpr W3 cw3;
+const W3 &cw3r = cw3;
+// MO1-DAG: @"\01??_SW3@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*]
+
 struct __declspec(dllimport) KeyFuncClass {
   constexpr KeyFuncClass() {}
   virtual void foo();
 };
 extern constexpr KeyFuncClass keyFuncClassVar = {};
 // G32-DAG: @_ZTV12KeyFuncClass = external dllimport unnamed_addr constant [3 x i8*]
 
 struct __declspec(dllimport) X : public virtual W {};