[ms-cxxabi] Destroy temporary record arguments in the callee

Itanium destroys them in the caller at the end of the full expression,
but MSVC destroys them in the callee.  This is further complicated by
the need to emit EH-only destructor cleanups in the caller.

This should help clang compile MSVC's debug iterators more correctly.
There is still an outstanding issue in PR5064 of a memcpy emitted by the
LLVM backend, which is not correct for C++ records.

Fixes PR16226.

Reviewers: rjmccall

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

llvm-svn: 184543

5 files changed, 87 insertions, 5 deletions

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 80446393d5b..cb5c78a6d59 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1865,6 +1865,19 @@ static void emitWritebacks(CodeGenFunction &CGF,
     emitWriteback(CGF, *i);
 }
 
+static void deactivateArgCleanupsBeforeCall(CodeGenFunction &CGF,
+                                            const CallArgList &CallArgs) {
+  assert(CGF.getTarget().getCXXABI().isArgumentDestroyedByCallee());
+  ArrayRef<CallArgList::CallArgCleanup> Cleanups =
+    CallArgs.getCleanupsToDeactivate();
+  // Iterate in reverse to increase the likelihood of popping the cleanup.
+  for (ArrayRef<CallArgList::CallArgCleanup>::reverse_iterator
+         I = Cleanups.rbegin(), E = Cleanups.rend(); I != E; ++I) {
+    CGF.DeactivateCleanupBlock(I->Cleanup, I->IsActiveIP);
+    I->IsActiveIP->eraseFromParent();
+  }
+}
+
 static const Expr *maybeGetUnaryAddrOfOperand(const Expr *E) {
   if (const UnaryOperator *uop = dyn_cast<UnaryOperator>(E->IgnoreParens()))
     if (uop->getOpcode() == UO_AddrOf)
@@ -2016,8 +2029,31 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
     return args.add(EmitReferenceBindingToExpr(E), type);
   }
 
-  if (hasAggregateEvaluationKind(type) &&
-      isa<ImplicitCastExpr>(E) &&
+  bool HasAggregateEvalKind = hasAggregateEvaluationKind(type);
+
+  // In the Microsoft C++ ABI, aggregate arguments are destructed by the callee.
+  // However, we still have to push an EH-only cleanup in case we unwind before
+  // we make it to the call.
+  if (HasAggregateEvalKind &&
+      CGM.getTarget().getCXXABI().isArgumentDestroyedByCallee()) {
+    const CXXRecordDecl *RD = type->getAsCXXRecordDecl();
+    if (RD && RD->hasNonTrivialDestructor()) {
+      AggValueSlot Slot = CreateAggTemp(type, "agg.arg.tmp");
+      Slot.setExternallyDestructed();
+      EmitAggExpr(E, Slot);
+      RValue RV = Slot.asRValue();
+      args.add(RV, type);
+
+      pushDestroy(EHCleanup, RV.getAggregateAddr(), type, destroyCXXObject,
+                  /*useEHCleanupForArray*/ true);
+      // This unreachable is a temporary marker which will be removed later.
+      llvm::Instruction *IsActive = Builder.CreateUnreachable();
+      args.addArgCleanupDeactivation(EHStack.getInnermostEHScope(), IsActive);
+      return;
+    }
+  }
+
+  if (HasAggregateEvalKind && isa<ImplicitCastExpr>(E) &&
       cast<CastExpr>(E)->getCastKind() == CK_LValueToRValue) {
     LValue L = EmitLValue(cast<CastExpr>(E)->getSubExpr());
     assert(L.isSimple());
@@ -2428,6 +2464,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
     }
   }
 
+  if (!CallArgs.getCleanupsToDeactivate().empty())
+    deactivateArgCleanupsBeforeCall(*this, CallArgs);
+
   // If the callee is a bitcast of a function to a varargs pointer to function
   // type, check to see if we can remove the bitcast.  This handles some cases
   // with unprototyped functions.
diff --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h
index 85c3320ec0e..04e600536e5 100644
--- a/clang/lib/CodeGen/CGCall.h
+++ b/clang/lib/CodeGen/CGCall.h
@@ -16,6 +16,7 @@
 #define CLANG_CODEGEN_CGCALL_H
 
 #include "CGValue.h"
+#include "EHScopeStack.h"
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/Type.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -67,6 +68,14 @@ namespace CodeGen {
       llvm::Value *ToUse;
     };
 
+    struct CallArgCleanup {
+      EHScopeStack::stable_iterator Cleanup;
+
+      /// The "is active" insertion point.  This instruction is temporary and
+      /// will be removed after insertion.
+      llvm::Instruction *IsActiveIP;
+    };
+
     void add(RValue rvalue, QualType type, bool needscopy = false) {
       push_back(CallArg(rvalue, type, needscopy));
     }
@@ -92,8 +101,25 @@ namespace CodeGen {
     writeback_iterator writeback_begin() const { return Writebacks.begin(); }
     writeback_iterator writeback_end() const { return Writebacks.end(); }
 
+    void addArgCleanupDeactivation(EHScopeStack::stable_iterator Cleanup,
+                                   llvm::Instruction *IsActiveIP) {
+      CallArgCleanup ArgCleanup;
+      ArgCleanup.Cleanup = Cleanup;
+      ArgCleanup.IsActiveIP = IsActiveIP;
+      CleanupsToDeactivate.push_back(ArgCleanup);
+    }
+
+    ArrayRef<CallArgCleanup> getCleanupsToDeactivate() const {
+      return CleanupsToDeactivate;
+    }
+
   private:
     SmallVector<Writeback, 1> Writebacks;
+
+    /// Deactivate these cleanups immediately before making the call.  This
+    /// is used to cleanup objects that are owned by the callee once the call
+    /// occurs.
+    SmallVector<CallArgCleanup, 1> CleanupsToDeactivate;
   };
 
   /// A class for recording the number of arguments that a function
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 429d2c8ccac..519f03aba16 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1651,10 +1651,18 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg,
   }
 
   llvm::Value *DeclPtr;
+  bool HasNonScalarEvalKind = !CodeGenFunction::hasScalarEvaluationKind(Ty);
   // If this is an aggregate or variable sized value, reuse the input pointer.
-  if (!Ty->isConstantSizeType() ||
-      !CodeGenFunction::hasScalarEvaluationKind(Ty)) {
+  if (HasNonScalarEvalKind || !Ty->isConstantSizeType()) {
     DeclPtr = Arg;
+    // Push a destructor cleanup for this parameter if the ABI requires it.
+    if (HasNonScalarEvalKind &&
+        getTarget().getCXXABI().isArgumentDestroyedByCallee()) {
+      if (const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl()) {
+        if (RD->hasNonTrivialDestructor())
+          pushDestroy(QualType::DK_cxx_destructor, DeclPtr, Ty);
+      }
+    }
   } else {
     // Otherwise, create a temporary to hold the value.
     llvm::AllocaInst *Alloc = CreateTempAlloca(ConvertTypeForMem(Ty),
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index 1ff794695cb..929130bc476 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -36,7 +36,7 @@ public:
   }
 
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const {
-    if (RD->hasNonTrivialCopyConstructor())
+    if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialDestructor())
       return RAA_DirectInMemory;
     return RAA_Default;
   }
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 16b12ea91e1..87fe7635891 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5777,6 +5777,15 @@ bool Sema::CheckParmsForFunctionDef(ParmVarDecl **P, ParmVarDecl **PEnd,
       }
       PType= AT->getElementType();
     }
+
+    // MSVC destroys objects passed by value in the callee.  Therefore a
+    // function definition which takes such a parameter must be able to call the
+    // object's destructor.
+    if (getLangOpts().CPlusPlus &&
+        Context.getTargetInfo().getCXXABI().isArgumentDestroyedByCallee()) {
+      if (const RecordType *RT = Param->getType()->getAs<RecordType>())
+        FinalizeVarWithDestructor(Param, RT);
+    }
   }
 
   return HasInvalidParm;