[ms-cxxabi] Emit linkonce complete dtors in TUs that need them

Based on Peter Collingbourne's destructor patches.

Prior to this change, clang was considering ?1 to be the complete
destructor and the base destructor, which was wrong.  This lead to
crashes when clang tried to emit two LLVM functions with the same name.

In this ABI, TUs with non-inline dtors might not emit a complete
destructor.  They are emitted as inline thunks in TUs that need them,
and they always delegate to the base dtors of the complete class and its
virtual bases.  This change uses the DeferredDecls machinery to emit
complete dtors as needed.

Currently in clang try body destructors can catch exceptions thrown by
virtual base destructors.  In the Microsoft C++ ABI, clang may not have
the destructor definition, in which case clang won't wrap the virtual
virtual base destructor calls in a try-catch.  Diagnosing this in user
code is TODO.

Finally, for classes that don't use virtual inheritance, MSVC always
calls the base destructor (?1) directly.  This is a useful code size
optimization that avoids emitting lots of extra thunks or aliases.
Implementing it also means our existing tests continue to pass, and is
consistent with MSVC's output.

We can do the same for Itanium by tweaking GetAddrOfCXXDestructor, but
it will require further testing.

Reviewers: rjmccall

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D1066

llvm-svn: 186828

8 files changed, 128 insertions, 36 deletions

diff --git a/clang/lib/CodeGen/CGCXX.cpp b/clang/lib/CodeGen/CGCXX.cpp
index b2a464e1cb1..6023bf7cc7d 100644
--- a/clang/lib/CodeGen/CGCXX.cpp
+++ b/clang/lib/CodeGen/CGCXX.cpp
@@ -228,22 +228,6 @@ CodeGenModule::GetAddrOfCXXConstructor(const CXXConstructorDecl *ctor,
                                                       /*ForVTable=*/false));
 }
 
-void CodeGenModule::EmitCXXDestructors(const CXXDestructorDecl *D) {
-  // The destructor in a virtual table is always a 'deleting'
-  // destructor, which calls the complete destructor and then uses the
-  // appropriate operator delete.
-  if (D->isVirtual())
-    EmitGlobal(GlobalDecl(D, Dtor_Deleting));
-
-  // The destructor used for destructing this as a most-derived class;
-  // call the base destructor and then destructs any virtual bases.
-  EmitGlobal(GlobalDecl(D, Dtor_Complete));
-
-  // The destructor used for destructing this as a base class; ignores
-  // virtual bases.
-  EmitGlobal(GlobalDecl(D, Dtor_Base));
-}
-
 void CodeGenModule::EmitCXXDestructor(const CXXDestructorDecl *dtor,
                                       CXXDtorType dtorType) {
   // The complete destructor is equivalent to the base destructor for
@@ -277,16 +261,27 @@ void CodeGenModule::EmitCXXDestructor(const CXXDestructorDecl *dtor,
 llvm::GlobalValue *
 CodeGenModule::GetAddrOfCXXDestructor(const CXXDestructorDecl *dtor,
                                       CXXDtorType dtorType,
-                                      const CGFunctionInfo *fnInfo) {
+                                      const CGFunctionInfo *fnInfo,
+                                      llvm::FunctionType *fnType) {
+  // If the class has no virtual bases, then the complete and base destructors
+  // are equivalent, for all C++ ABIs supported by clang.  We can save on code
+  // size by calling the base dtor directly, especially if we'd have to emit a
+  // thunk otherwise.
+  // FIXME: We should do this for Itanium, after verifying that nothing breaks.
+  if (dtorType == Dtor_Complete && dtor->getParent()->getNumVBases() == 0 &&
+      getCXXABI().useThunkForDtorVariant(dtor, Dtor_Complete))
+    dtorType = Dtor_Base;
+
   GlobalDecl GD(dtor, dtorType);
 
   StringRef name = getMangledName(GD);
   if (llvm::GlobalValue *existing = GetGlobalValue(name))
     return existing;
 
-  if (!fnInfo) fnInfo = &getTypes().arrangeCXXDestructor(dtor, dtorType);
-
-  llvm::FunctionType *fnType = getTypes().GetFunctionType(*fnInfo);
+  if (!fnType) {
+    if (!fnInfo) fnInfo = &getTypes().arrangeCXXDestructor(dtor, dtorType);
+    fnType = getTypes().GetFunctionType(*fnInfo);
+  }
   return cast<llvm::Function>(GetOrCreateLLVMFunction(name, fnType, GD,
                                                       /*ForVTable=*/false));
 }
diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index 6629e045749..27792d02c68 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -244,6 +244,15 @@ public:
                                         CanQualType &ResTy,
                                SmallVectorImpl<CanQualType> &ArgTys) = 0;
 
+  /// Returns true if the given destructor type should be emitted as a linkonce
+  /// delegating thunk, regardless of whether the dtor is defined in this TU or
+  /// not.
+  virtual bool useThunkForDtorVariant(const CXXDestructorDecl *Dtor,
+                                      CXXDtorType DT) const = 0;
+
+  /// Emit destructor variants required by this ABI.
+  virtual void EmitCXXDestructors(const CXXDestructorDecl *D) = 0;
+
   /// Build the ABI-specific portion of the parameter list for a
   /// function.  This generally involves a 'this' parameter and
   /// possibly some extra data for constructors and destructors.
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 2bb44552e26..dd75523d14a 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -1265,16 +1265,19 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   // If this is the complete variant, just invoke the base variant;
   // the epilogue will destruct the virtual bases.  But we can't do
   // this optimization if the body is a function-try-block, because
-  // we'd introduce *two* handler blocks.
+  // we'd introduce *two* handler blocks.  In the Microsoft ABI, we 
+  // always delegate because we might not have a definition in this TU.
   switch (DtorType) {
   case Dtor_Deleting: llvm_unreachable("already handled deleting case");
 
   case Dtor_Complete:
+    assert((Body || getTarget().getCXXABI().isMicrosoft()) &&
+           "can't emit a dtor without a body for non-Microsoft ABIs");
+
     // Enter the cleanup scopes for virtual bases.
     EnterDtorCleanups(Dtor, Dtor_Complete);
 
-    if (!isTryBody &&
-        CGM.getTarget().getCXXABI().hasDestructorVariants()) {
+    if (!isTryBody) {
       EmitCXXDestructorCall(Dtor, Dtor_Base, /*ForVirtualBase=*/false,
                             /*Delegating=*/false, LoadCXXThis());
       break;
@@ -1282,6 +1285,8 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
     // Fallthrough: act like we're in the base variant.
       
   case Dtor_Base:
+    assert(Body);
+
     // Enter the cleanup scopes for fields and non-virtual bases.
     EnterDtorCleanups(Dtor, Dtor_Base);
 
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 1520656c9b8..c74d88b9508 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -260,7 +260,7 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
   else
     FInfo = &CGM.getTypes().arrangeCXXMethodDeclaration(CalleeDecl);
 
-  llvm::Type *Ty = CGM.getTypes().GetFunctionType(*FInfo);
+  llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
   // C++ [class.virtual]p12:
   //   Explicit qualification with the scope operator (5.1) suppresses the
@@ -284,7 +284,7 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
           ME->hasQualifier())
         Callee = BuildAppleKextVirtualCall(MD, ME->getQualifier(), Ty);
       else if (!DevirtualizedMethod)
-        Callee = CGM.GetAddrOfFunction(GlobalDecl(Dtor, Dtor_Complete), Ty);
+        Callee = CGM.GetAddrOfCXXDestructor(Dtor, Dtor_Complete, FInfo, Ty);
       else {
         const CXXDestructorDecl *DDtor =
           cast<CXXDestructorDecl>(DevirtualizedMethod);
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 078edebb965..fd884db6ca2 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -513,6 +513,12 @@ void CodeGenModule::EmitCtorList(const CtorList &Fns, const char *GlobalName) {
 llvm::GlobalValue::LinkageTypes
 CodeGenModule::getFunctionLinkage(GlobalDecl GD) {
   const FunctionDecl *D = cast<FunctionDecl>(GD.getDecl());
+
+  if (isa<CXXDestructorDecl>(D) &&
+      getCXXABI().useThunkForDtorVariant(cast<CXXDestructorDecl>(D),
+                                         GD.getDtorType()))
+    return llvm::Function::LinkOnceODRLinkage;
+
   GVALinkage Linkage = getContext().GetGVALinkageForFunction(D);
 
   if (Linkage == GVA_Internal)
@@ -1348,6 +1354,14 @@ CodeGenModule::GetOrCreateLLVMFunction(StringRef MangledName,
     return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
   }
 
+  // All MSVC dtors other than the base dtor are linkonce_odr and delegate to
+  // each other bottoming out with the base dtor.  Therefore we emit non-base
+  // dtors on usage, even if there is no dtor definition in the TU.
+  if (D && isa<CXXDestructorDecl>(D) &&
+      getCXXABI().useThunkForDtorVariant(cast<CXXDestructorDecl>(D),
+                                         GD.getDtorType()))
+    DeferredDeclsToEmit.push_back(GD);
+
   // This function doesn't have a complete type (for example, the return
   // type is an incomplete struct). Use a fake type instead, and make
   // sure not to try to set attributes.
@@ -2869,7 +2883,7 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) {
   case Decl::CXXDestructor:
     if (cast<FunctionDecl>(D)->isLateTemplateParsed())
       return;
-    EmitCXXDestructors(cast<CXXDestructorDecl>(D));
+    getCXXABI().EmitCXXDestructors(cast<CXXDestructorDecl>(D));
     break;
 
   case Decl::StaticAssert:
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index c2cbe99a668..c99a0ecfc7e 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -752,7 +752,8 @@ public:
   /// given type.
   llvm::GlobalValue *GetAddrOfCXXDestructor(const CXXDestructorDecl *dtor,
                                             CXXDtorType dtorType,
-                                            const CGFunctionInfo *fnInfo = 0);
+                                            const CGFunctionInfo *fnInfo = 0,
+                                            llvm::FunctionType *fnType = 0);
 
   /// getBuiltinLibFunction - Given a builtin id for a function like
   /// "__builtin_fabsf", return a Function* for "fabsf".
@@ -980,6 +981,10 @@ public:
     DeferredVTables.push_back(RD);
   }
 
+  /// EmitGlobal - Emit code for a singal global function or var decl. Forward
+  /// declarations are emitted lazily.
+  void EmitGlobal(GlobalDecl D);
+
 private:
   llvm::GlobalValue *GetGlobalValue(StringRef Ref);
 
@@ -1011,10 +1016,6 @@ private:
                              llvm::Function *F,
                              bool IsIncompleteFunction);
 
-  /// EmitGlobal - Emit code for a singal global function or var decl. Forward
-  /// declarations are emitted lazily.
-  void EmitGlobal(GlobalDecl D);
-
   void EmitGlobalDefinition(GlobalDecl D);
 
   void EmitGlobalFunctionDefinition(GlobalDecl GD);
@@ -1040,10 +1041,6 @@ private:
   /// a C++ constructor Decl.
   void EmitCXXConstructor(const CXXConstructorDecl *D, CXXCtorType Type);
 
-  /// EmitCXXDestructors - Emit destructors (base, complete) from a
-  /// C++ destructor Decl.
-  void EmitCXXDestructors(const CXXDestructorDecl *D);
-
   /// EmitCXXDestructor - Emit a single destructor with the given type from
   /// a C++ destructor Decl.
   void EmitCXXDestructor(const CXXDestructorDecl *D, CXXDtorType Type);
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index cdfe909f9d0..55814b455bb 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -113,6 +113,16 @@ public:
                                 CanQualType &ResTy,
                                 SmallVectorImpl<CanQualType> &ArgTys);
 
+  bool useThunkForDtorVariant(const CXXDestructorDecl *Dtor,
+                              CXXDtorType DT) const {
+    // Itanium does not emit any destructor variant as an inline thunk.
+    // Delegating may occur as an optimization, but all variants are either
+    // emitted with external linkage or as linkonce if they are inline and used.
+    return false;
+  }
+
+  void EmitCXXDestructors(const CXXDestructorDecl *D);
+
   void BuildInstanceFunctionParams(CodeGenFunction &CGF,
                                    QualType &ResTy,
                                    FunctionArgList &Params);
@@ -761,6 +771,22 @@ void ItaniumCXXABI::BuildDestructorSignature(const CXXDestructorDecl *Dtor,
     ArgTys.push_back(Context.getPointerType(Context.VoidPtrTy));
 }
 
+void ItaniumCXXABI::EmitCXXDestructors(const CXXDestructorDecl *D) {
+  // The destructor in a virtual table is always a 'deleting'
+  // destructor, which calls the complete destructor and then uses the
+  // appropriate operator delete.
+  if (D->isVirtual())
+    CGM.EmitGlobal(GlobalDecl(D, Dtor_Deleting));
+
+  // The destructor used for destructing this as a most-derived class;
+  // call the base destructor and then destructs any virtual bases.
+  CGM.EmitGlobal(GlobalDecl(D, Dtor_Complete));
+
+  // The destructor used for destructing this as a base class; ignores
+  // virtual bases.
+  CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+}
+
 void ItaniumCXXABI::BuildInstanceFunctionParams(CodeGenFunction &CGF,
                                                 QualType &ResTy,
                                                 FunctionArgList &Params) {
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index 22d548857b7..0e01d8bacfc 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -65,11 +65,51 @@ public:
   llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF,
                                                   const CXXRecordDecl *RD);
 
-  void BuildDestructorSignature(const CXXDestructorDecl *Ctor,
+  // Background on MSVC destructors
+  // ==============================
+  //
+  // Both Itanium and MSVC ABIs have destructor variants.  The variant names
+  // roughly correspond in the following way:
+  //   Itanium       Microsoft
+  //   Base       -> no name, just ~Class
+  //   Complete   -> vbase destructor
+  //   Deleting   -> scalar deleting destructor
+  //                 vector deleting destructor
+  //
+  // The base and complete destructors are the same as in Itanium, although the
+  // complete destructor does not accept a VTT parameter when there are virtual
+  // bases.  A separate mechanism involving vtordisps is used to ensure that
+  // virtual methods of destroyed subobjects are not called.
+  //
+  // The deleting destructors accept an i32 bitfield as a second parameter.  Bit
+  // 1 indicates if the memory should be deleted.  Bit 2 indicates if the this
+  // pointer points to an array.  The scalar deleting destructor assumes that
+  // bit 2 is zero, and therefore does not contain a loop.
+  //
+  // For virtual destructors, only one entry is reserved in the vftable, and it
+  // always points to the vector deleting destructor.  The vector deleting
+  // destructor is the most general, so it can be used to destroy objects in
+  // place, delete single heap objects, or delete arrays.
+  //
+  // A TU defining a non-inline destructor is only guaranteed to emit a base
+  // destructor, and all of the other variants are emitted on an as-needed basis
+  // in COMDATs.  Because a non-base destructor can be emitted in a TU that
+  // lacks a definition for the destructor, non-base destructors must always
+  // delegate to or alias the base destructor.
+
+  void BuildDestructorSignature(const CXXDestructorDecl *Dtor,
                                 CXXDtorType Type,
                                 CanQualType &ResTy,
                                 SmallVectorImpl<CanQualType> &ArgTys);
 
+  /// Non-base dtors should be emitted as delegating thunks in this ABI.
+  bool useThunkForDtorVariant(const CXXDestructorDecl *Dtor,
+                              CXXDtorType DT) const {
+    return DT != Dtor_Base;
+  }
+
+  void EmitCXXDestructors(const CXXDestructorDecl *D);
+
   void BuildInstanceFunctionParams(CodeGenFunction &CGF,
                                    QualType &ResTy,
                                    FunctionArgList &Params);
@@ -387,6 +427,12 @@ void MicrosoftCXXABI::BuildDestructorSignature(const CXXDestructorDecl *Dtor,
   }
 }
 
+void MicrosoftCXXABI::EmitCXXDestructors(const CXXDestructorDecl *D) {
+  // The TU defining a dtor is only guaranteed to emit a base destructor.  All
+  // other destructor variants are delegating thunks.
+  CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+}
+
 static bool IsDeletingDtor(GlobalDecl GD) {
   const CXXMethodDecl* MD = cast<CXXMethodDecl>(GD.getDecl());
   if (isa<CXXDestructorDecl>(MD)) {