[analyzer] Add support for constructors of arguments.

Once CFG-side support for argument construction contexts landed in r338436,
the analyzer could make use of them to evaluate argument constructors properly.

When evaluated as calls, constructors of arguments now use the variable region
of the parameter as their target. The corresponding stack frame does not yet
exist when the parameter is constructed, and this stack frame is created
eagerly.

Construction of functions whose body is unavailable and of virtual functions
is not yet supported. Part of the reason is the analyzer doesn't consistently
use canonical declarations o identify the function in these cases, and every
re-declaration or potential override comes with its own set of parameter
declarations. Also it is less important because if the function is not
inlined, there's usually no benefit in inlining the argument constructor.

Differential Revision: https://reviews.llvm.org/D49443


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@339745 91177308-0d34-0410-b5e6-96231b3b80d8

11 files changed, 485 insertions, 55 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 5e319c7fd1..6557f38213 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -428,21 +428,23 @@ public:
   bool isArgumentConstructedDirectly(unsigned Index) const {
     // This assumes that the object was not yet removed from the state.
     return ExprEngine::getObjectUnderConstruction(
-        getState(), {getOriginExpr(), Index}, getCalleeStackFrame()).hasValue();
+        getState(), {getOriginExpr(), Index}, getLocationContext()).hasValue();
   }
 
   /// Some calls have parameter numbering mismatched from argument numbering.
   /// This function converts an argument index to the corresponding
   /// parameter index. Returns None is the argument doesn't correspond
   /// to any parameter variable.
-  Optional<unsigned> getAdjustedParameterIndex(unsigned ArgumentIndex) const {
-    if (dyn_cast_or_null<CXXOperatorCallExpr>(getOriginExpr()) &&
-        dyn_cast_or_null<CXXMethodDecl>(getDecl())) {
-      // For member operator calls argument 0 on the expression corresponds
-      // to implicit this-parameter on the declaration.
-      return (ArgumentIndex > 0) ? Optional<unsigned>(ArgumentIndex - 1) : None;
-    }
-    return ArgumentIndex;
+  virtual Optional<unsigned>
+  getAdjustedParameterIndex(unsigned ASTArgumentIndex) const {
+    return ASTArgumentIndex;
+  }
+
+  /// Some call event sub-classes conveniently adjust mismatching AST indices
+  /// to match parameter indices. This function converts an argument index
+  /// as understood by CallEvent to the argument index as understood by the AST.
+  virtual unsigned getASTArgumentIndex(unsigned CallArgumentIndex) const {
+    return CallArgumentIndex;
   }
 
   // Iterator access to formal parameters and their types.
@@ -769,6 +771,20 @@ public:
   static bool classof(const CallEvent *CA) {
     return CA->getKind() == CE_CXXMemberOperator;
   }
+
+  Optional<unsigned>
+  getAdjustedParameterIndex(unsigned ASTArgumentIndex) const override {
+    // For member operator calls argument 0 on the expression corresponds
+    // to implicit this-parameter on the declaration.
+    return (ASTArgumentIndex > 0) ? Optional<unsigned>(ASTArgumentIndex - 1)
+                                  : None;
+  }
+
+  unsigned getASTArgumentIndex(unsigned CallArgumentIndex) const override {
+    // For member operator calls argument 0 on the expression corresponds
+    // to implicit this-parameter on the declaration.
+    return CallArgumentIndex + 1;
+  }
 };
 
 /// Represents an implicit call to a C++ destructor.
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 06a90fa847..e279bff86a 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -662,6 +662,11 @@ public:
                        const EvalCallOptions &CallOpts = {});
 
 private:
+  ProgramStateRef finishArgumentConstruction(ProgramStateRef State,
+                                             const CallEvent &Call);
+  void finishArgumentConstruction(ExplodedNodeSet &Dst, ExplodedNode *Pred,
+                                  const CallEvent &Call);
+
   void evalLoadCommon(ExplodedNodeSet &Dst,
                       const Expr *NodeEx,  /* Eventually will be a CFGStmt */
                       const Expr *BoundEx,
diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 7d6358acbb..cff0f0a29b 100644
--- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -156,6 +156,11 @@ void NilArgChecker::warnIfNilArg(CheckerContext &C,
   if (!State->isNull(msg.getArgSVal(Arg)).isConstrainedTrue())
       return;
 
+  // NOTE: We cannot throw non-fatal errors from warnIfNilExpr,
+  // because it's called multiple times from some callers, so it'd cause
+  // an unwanted state split if two or more non-fatal errors are thrown
+  // within the same checker callback. For now we don't want to, but
+  // it'll need to be fixed if we ever want to.
   if (ExplodedNode *N = C.generateErrorNode()) {
     SmallString<128> sbuf;
     llvm::raw_svector_ostream os(sbuf);
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index fe9260e32d..b1688184d8 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -169,18 +169,27 @@ bool CallEvent::isGlobalCFunction(StringRef FunctionName) const {
 
 AnalysisDeclContext *CallEvent::getCalleeAnalysisDeclContext() const {
   const Decl *D = getDecl();
-
-  // If the callee is completely unknown, we cannot construct the stack frame.
   if (!D)
     return nullptr;
 
-  // FIXME: Skip virtual functions for now. There's no easy procedure to foresee
-  // the exact decl that should be used, especially when it's not a definition.
-  if (const Decl *RD = getRuntimeDefinition().getDecl())
-    if (RD != D)
-      return nullptr;
+  // TODO: For now we skip functions without definitions, even if we have
+  // our own getDecl(), because it's hard to find out which re-declaration
+  // is going to be used, and usually clients don't really care about this
+  // situation because there's a loss of precision anyway because we cannot
+  // inline the call.
+  RuntimeDefinition RD = getRuntimeDefinition();
+  if (!RD.getDecl())
+    return nullptr;
+
+  AnalysisDeclContext *ADC =
+      LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
+
+  // TODO: For now we skip virtual functions, because this also rises
+  // the problem of which decl to use, but now it's across different classes.
+  if (RD.mayHaveOtherDefinitions() || RD.getDecl() != ADC->getDecl())
+    return nullptr;
 
-  return LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
+  return ADC;
 }
 
 const StackFrameContext *CallEvent::getCalleeStackFrame() const {
@@ -218,7 +227,24 @@ const VarRegion *CallEvent::getParameterLocation(unsigned Index) const {
   if (!SFC)
     return nullptr;
 
-  const ParmVarDecl *PVD = parameters()[Index];
+  // Retrieve parameters of the definition, which are different from
+  // CallEvent's parameters() because getDecl() isn't necessarily
+  // the definition. SFC contains the definition that would be used
+  // during analysis.
+  const Decl *D = SFC->getDecl();
+
+  // TODO: Refactor into a virtual method of CallEvent, like parameters().
+  const ParmVarDecl *PVD = nullptr;
+  if (const auto *FD = dyn_cast<FunctionDecl>(D))
+    PVD = FD->parameters()[Index];
+  else if (const auto *BD = dyn_cast<BlockDecl>(D))
+    PVD = BD->parameters()[Index];
+  else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D))
+    PVD = MD->parameters()[Index];
+  else if (const auto *CD = dyn_cast<CXXConstructorDecl>(D))
+    PVD = CD->parameters()[Index];
+  assert(PVD && "Unexpected Decl kind!");
+
   const VarRegion *VR =
       State->getStateManager().getRegionManager().getVarRegion(PVD, SFC);
 
@@ -285,6 +311,20 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
         // TODO: Factor this out + handle the lower level const pointers.
 
     ValuesToInvalidate.push_back(getArgSVal(Idx));
+
+    // If a function accepts an object by argument (which would of course be a
+    // temporary that isn't lifetime-extended), invalidate the object itself,
+    // not only other objects reachable from it. This is necessary because the
+    // destructor has access to the temporary object after the call.
+    // TODO: Support placement arguments once we start
+    // constructing them directly.
+    // TODO: This is unnecessary when there's no destructor, but that's
+    // currently hard to figure out.
+    if (getKind() != CE_CXXAllocator)
+      if (isArgumentConstructedDirectly(Idx))
+        if (auto AdjIdx = getAdjustedParameterIndex(Idx))
+          if (const VarRegion *VR = getParameterLocation(*AdjIdx))
+            ValuesToInvalidate.push_back(loc::MemRegionVal(VR));
   }
 
   // Invalidate designated regions using the batch invalidation API.
@@ -433,6 +473,10 @@ static void addParameterValuesToBindings(const StackFrameContext *CalleeCtx,
     const ParmVarDecl *ParamDecl = *I;
     assert(ParamDecl && "Formal parameter has no decl?");
 
+    if (Call.getKind() != CE_CXXAllocator)
+      if (Call.isArgumentConstructedDirectly(Idx))
+        continue;
+
     SVal ArgVal = Call.getArgSVal(Idx);
     if (!ArgVal.isUnknown()) {
       Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx));
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 40259fb0c0..8469a601b0 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2199,17 +2199,21 @@ void ExprEngine::processBeginOfFunction(NodeBuilderContext &BC,
 void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
                                       ExplodedNode *Pred,
                                       const ReturnStmt *RS) {
+  ProgramStateRef State = Pred->getState();
+
+  if (!Pred->getStackFrame()->inTopFrame())
+    State = finishArgumentConstruction(
+        State, *getStateManager().getCallEventManager().getCaller(
+                   Pred->getStackFrame(), Pred->getState()));
+
   // FIXME: We currently cannot assert that temporaries are clear, because
   // lifetime extended temporaries are not always modelled correctly. In some
   // cases when we materialize the temporary, we do
   // createTemporaryRegionIfNeeded(), and the region changes, and also the
   // respective destructor becomes automatic from temporary. So for now clean up
-  // the state manually before asserting. Ideally, the code above the assertion
-  // should go away, but the assertion should remain.
+  // the state manually before asserting. Ideally, this braced block of code
+  // should go away.
   {
-    ExplodedNodeSet CleanUpObjects;
-    NodeBuilder Bldr(Pred, CleanUpObjects, BC);
-    ProgramStateRef State = Pred->getState();
     const LocationContext *FromLC = Pred->getLocationContext();
     const LocationContext *ToLC = FromLC->getStackFrame()->getParent();
     const LocationContext *LC = FromLC;
@@ -2228,15 +2232,20 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
         }
       LC = LC->getParent();
     }
-    if (State != Pred->getState()) {
-      Pred = Bldr.generateNode(Pred->getLocation(), State, Pred);
-      if (!Pred) {
-        // The node with clean temporaries already exists. We might have reached
-        // it on a path on which we initialize different temporaries.
-        return;
-      }
+  }
+
+  // Perform the transition with cleanups.
+  if (State != Pred->getState()) {
+    ExplodedNodeSet PostCleanup;
+    NodeBuilder Bldr(Pred, PostCleanup, BC);
+    Pred = Bldr.generateNode(Pred->getLocation(), State, Pred);
+    if (!Pred) {
+      // The node with clean temporaries already exists. We might have reached
+      // it on a path on which we initialize different temporaries.
+      return;
     }
   }
+
   assert(areAllObjectsFullyConstructed(Pred->getState(),
                                        Pred->getLocationContext(),
                                        Pred->getStackFrame()->getParent()));
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 90dcf2f22e..933d54e11c 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -292,8 +292,75 @@ std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
       return std::make_pair(State, V);
     }
     case ConstructionContext::ArgumentKind: {
-      // Function argument constructors. Not implemented yet.
-      break;
+      // Arguments are technically temporaries.
+      CallOpts.IsTemporaryCtorOrDtor = true;
+
+      const auto *ACC = cast<ArgumentConstructionContext>(CC);
+      const Expr *E = ACC->getCallLikeExpr();
+      unsigned Idx = ACC->getIndex();
+      const CXXBindTemporaryExpr *BTE = ACC->getCXXBindTemporaryExpr();
+
+      CallEventManager &CEMgr = getStateManager().getCallEventManager();
+      SVal V = UnknownVal();
+      auto getArgLoc = [&](CallEventRef<> Caller) -> Optional<SVal> {
+        const LocationContext *FutureSFC = Caller->getCalleeStackFrame();
+        // Return early if we are unable to reliably foresee
+        // the future stack frame.
+        if (!FutureSFC)
+          return None;
+
+        // This should be equivalent to Caller->getDecl() for now, but
+        // FutureSFC->getDecl() is likely to support better stuff (like
+        // virtual functions) earlier.
+        const Decl *CalleeD = FutureSFC->getDecl();
+
+        // FIXME: Support for variadic arguments is not implemented here yet.
+        if (CallEvent::isVariadic(CalleeD))
+          return None;
+
+        // Operator arguments do not correspond to operator parameters
+        // because this-argument is implemented as a normal argument in
+        // operator call expressions but not in operator declarations.
+        const VarRegion *VR = Caller->getParameterLocation(
+            *Caller->getAdjustedParameterIndex(Idx));
+        if (!VR)
+          return None;
+
+        return loc::MemRegionVal(VR);
+      };
+
+      if (const auto *CE = dyn_cast<CallExpr>(E)) {
+        CallEventRef<> Caller = CEMgr.getSimpleCall(CE, State, LCtx);
+        if (auto OptV = getArgLoc(Caller))
+          V = *OptV;
+        else
+          break;
+        State = addObjectUnderConstruction(State, {CE, Idx}, LCtx, V);
+      } else if (const auto *CCE = dyn_cast<CXXConstructExpr>(E)) {
+        // Don't bother figuring out the target region for the future
+        // constructor because we won't need it.
+        CallEventRef<> Caller =
+            CEMgr.getCXXConstructorCall(CCE, /*Target=*/nullptr, State, LCtx);
+        if (auto OptV = getArgLoc(Caller))
+          V = *OptV;
+        else
+          break;
+        State = addObjectUnderConstruction(State, {CCE, Idx}, LCtx, V);
+      } else if (const auto *ME = dyn_cast<ObjCMessageExpr>(E)) {
+        CallEventRef<> Caller = CEMgr.getObjCMethodCall(ME, State, LCtx);
+        if (auto OptV = getArgLoc(Caller))
+          V = *OptV;
+        else
+          break;
+        State = addObjectUnderConstruction(State, {ME, Idx}, LCtx, V);
+      }
+
+      assert(!V.isUnknown());
+
+      if (BTE)
+        State = addObjectUnderConstruction(State, BTE, LCtx, V);
+
+      return std::make_pair(State, V);
     }
     }
   }
@@ -502,8 +569,15 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
     }
   }
 
+  ExplodedNodeSet DstPostArgumentCleanup;
+  for (auto I : DstEvaluated)
+    finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
+
+  // If there were other constructors called for object-type arguments
+  // of this constructor, clean them up.
   ExplodedNodeSet DstPostCall;
-  getCheckerManager().runCheckersForPostCall(DstPostCall, DstEvaluated,
+  getCheckerManager().runCheckersForPostCall(DstPostCall,
+                                             DstPostArgumentCleanup,
                                              *Call, *this);
   getCheckerManager().runCheckersForPostStmt(destNodes, DstPostCall, CE, *this);
 }
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 3ee67f3d68..6e78111b0c 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -505,6 +505,49 @@ void ExprEngine::VisitCallExpr(const CallExpr *CE, ExplodedNode *Pred,
                                              *this);
 }
 
+ProgramStateRef ExprEngine::finishArgumentConstruction(ProgramStateRef State,
+                                                       const CallEvent &Call) {
+  const Expr *E = Call.getOriginExpr();
+  // FIXME: Constructors to placement arguments of operator new
+  // are not supported yet.
+  if (!E || isa<CXXNewExpr>(E))
+    return State;
+
+  const LocationContext *LC = Call.getLocationContext();
+  for (unsigned CallI = 0, CallN = Call.getNumArgs(); CallI != CallN; ++CallI) {
+    unsigned I = Call.getASTArgumentIndex(CallI);
+    if (Optional<SVal> V =
+            getObjectUnderConstruction(State, {E, I}, LC)) {
+      SVal VV = *V;
+      assert(cast<VarRegion>(VV.castAs<loc::MemRegionVal>().getRegion())
+                 ->getStackFrame()->getParent()
+                 ->getStackFrame() == LC->getStackFrame());
+      State = finishObjectConstruction(State, {E, I}, LC);
+    }
+  }
+
+  return State;
+}
+
+void ExprEngine::finishArgumentConstruction(ExplodedNodeSet &Dst,
+                                            ExplodedNode *Pred,
+                                            const CallEvent &Call) {
+  ProgramStateRef State = Pred->getState();
+  ProgramStateRef CleanedState = finishArgumentConstruction(State, Call);
+  if (CleanedState == State) {
+    Dst.insert(Pred);
+    return;
+  }
+
+  const Expr *E = Call.getOriginExpr();
+  const LocationContext *LC = Call.getLocationContext();
+  NodeBuilder B(Pred, Dst, *currBldrCtx);
+  static SimpleProgramPointTag Tag("ExprEngine",
+                                   "Finish argument construction");
+  PreStmt PP(E, LC, &Tag);
+  B.generateNode(PP, CleanedState, Pred);
+}
+
 void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
                           const CallEvent &Call) {
   // WARNING: At this time, the state attached to 'Call' may be older than the
@@ -516,7 +559,8 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
 
   // Run any pre-call checks using the generic call interface.
   ExplodedNodeSet dstPreVisit;
-  getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred, Call, *this);
+  getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred,
+                                            Call, *this);
 
   // Actually evaluate the function call.  We try each of the checkers
   // to see if the can evaluate the function call, and get a callback at
@@ -525,8 +569,14 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
   getCheckerManager().runCheckersForEvalCall(dstCallEvaluated, dstPreVisit,
                                              Call, *this);
 
+  // If there were other constructors called for object-type arguments
+  // of this call, clean them up.
+  ExplodedNodeSet dstArgumentCleanup;
+  for (auto I : dstCallEvaluated)
+    finishArgumentConstruction(dstArgumentCleanup, I, Call);
+
   // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstCallEvaluated,
+  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
                                              Call, *this);
 }
 
diff --git a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index d76b9cbcfa..d36d1fbd17 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -197,7 +197,8 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME,
 
       // Receiver is definitely nil, so run ObjCMessageNil callbacks and return.
       if (nilState && !notNilState) {
-        StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+        ExplodedNodeSet dstNil;
+        StmtNodeBuilder Bldr(Pred, dstNil, *currBldrCtx);
         bool HasTag = Pred->getLocation().getTag();
         Pred = Bldr.generateNode(ME, Pred, nilState, nullptr,
                                  ProgramPoint::PreStmtKind);
@@ -205,8 +206,12 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME,
         (void)HasTag;
         if (!Pred)
           return;
-        getCheckerManager().runCheckersForObjCMessageNil(Dst, Pred,
+
+        ExplodedNodeSet dstPostCheckers;
+        getCheckerManager().runCheckersForObjCMessageNil(dstPostCheckers, Pred,
                                                          *Msg, *this);
+        for (auto I : dstPostCheckers)
+          finishArgumentConstruction(Dst, I, *Msg);
         return;
       }
 
@@ -267,8 +272,13 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME,
     defaultEvalCall(Bldr, Pred, *UpdatedMsg);
   }
 
+  // If there were constructors called for object-type arguments, clean them up.
+  ExplodedNodeSet dstArgCleanup;
+  for (auto I : dstEval)
+    finishArgumentConstruction(dstArgCleanup, I, *Msg);
+
   ExplodedNodeSet dstPostvisit;
-  getCheckerManager().runCheckersForPostCall(dstPostvisit, dstEval,
+  getCheckerManager().runCheckersForPostCall(dstPostvisit, dstArgCleanup,
                                              *Msg, *this);
 
   // Finally, perform the post-condition check of the ObjCMessageExpr and store
diff --git a/test/Analysis/copy-elision.cpp b/test/Analysis/copy-elision.cpp
index cf77912ea6..eb91a3c71b 100644
--- a/test/Analysis/copy-elision.cpp
+++ b/test/Analysis/copy-elision.cpp
@@ -122,7 +122,7 @@ void foo(int coin) {
 namespace address_vector_tests {
 
 template <typename T> struct AddressVector {
-  T *buf[10];
+  T *buf[20];
   int len;
 
   AddressVector() : len(0) {}
@@ -138,13 +138,13 @@ class ClassWithoutDestructor {
 
 public:
   ClassWithoutDestructor(AddressVector<ClassWithoutDestructor> &v) : v(v) {
-    v.push(this);
+    push();
   }
 
-  ClassWithoutDestructor(ClassWithoutDestructor &&c) : v(c.v) { v.push(this); }
-  ClassWithoutDestructor(const ClassWithoutDestructor &c) : v(c.v) {
-    v.push(this);
-  }
+  ClassWithoutDestructor(ClassWithoutDestructor &&c) : v(c.v) { push(); }
+  ClassWithoutDestructor(const ClassWithoutDestructor &c) : v(c.v) { push(); }
+
+  void push() { v.push(this); }
 };
 
 ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) {
@@ -174,20 +174,44 @@ void testMultipleReturns() {
 #endif
 }
 
+void consume(ClassWithoutDestructor c) {
+  c.push();
+}
+
+void testArgumentConstructorWithoutDestructor() {
+  AddressVector<ClassWithoutDestructor> v;
+
+  consume(make3(v));
+
+#if ELIDE
+  clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
+#else
+  clang_analyzer_eval(v.len == 6); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] != v.buf[1]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[1] != v.buf[2]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[2] != v.buf[3]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[3] != v.buf[4]); // expected-warning{{TRUE}}
+  // We forced a push() in consume(), let's see if the address here matches
+  // the address during construction.
+  clang_analyzer_eval(v.buf[4] == v.buf[5]); // expected-warning{{TRUE}}
+#endif
+}
+
 class ClassWithDestructor {
   AddressVector<ClassWithDestructor> &v;
 
 public:
   ClassWithDestructor(AddressVector<ClassWithDestructor> &v) : v(v) {
-    v.push(this);
+    push();
   }
 
-  ClassWithDestructor(ClassWithDestructor &&c) : v(c.v) { v.push(this); }
-  ClassWithDestructor(const ClassWithDestructor &c) : v(c.v) {
-    v.push(this);
-  }
+  ClassWithDestructor(ClassWithDestructor &&c) : v(c.v) { push(); }
+  ClassWithDestructor(const ClassWithDestructor &c) : v(c.v) { push(); }
 
-  ~ClassWithDestructor() { v.push(this); }
+  ~ClassWithDestructor() { push(); }
+
+  void push() { v.push(this); }
 };
 
 void testVariable() {
@@ -301,4 +325,43 @@ void testMultipleReturnsWithDestructors() {
   clang_analyzer_eval(v.buf[7] == v.buf[9]); // expected-warning{{TRUE}}
 #endif
 }
+
+void consume(ClassWithDestructor c) {
+  c.push();
+}
+
+void testArgumentConstructorWithDestructor() {
+  AddressVector<ClassWithDestructor> v;
+
+  consume(make3(v));
+
+#if ELIDE
+  // 0. Construct the argument.
+  // 1. Forced push() in consume().
+  // 2. Destroy the argument.
+  clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[1] == v.buf[2]); // expected-warning{{TRUE}}
+#else
+  // 0. Construct the temporary in make1().
+  // 1. Construct the temporary in make2().
+  // 2. Destroy the temporary in make1().
+  // 3. Construct the temporary in make3().
+  // 4. Destroy the temporary in make2().
+  // 5. Construct the temporary here.
+  // 6. Destroy the temporary in make3().
+  // 7. Construct the argument.
+  // 8. Forced push() in consume().
+  // 9. Destroy the argument. Notice the reverse order!
+  // 10. Destroy the temporary here.
+  clang_analyzer_eval(v.len == 11); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[2]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[1] == v.buf[4]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[3] == v.buf[6]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[5] == v.buf[10]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[7] == v.buf[8]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[8] == v.buf[9]); // expected-warning{{TRUE}}
+#endif
+}
+
 } // namespace address_vector_tests
diff --git a/test/Analysis/temporaries.cpp b/test/Analysis/temporaries.cpp
index b5a157bb8d..26cfa51b82 100644
--- a/test/Analysis/temporaries.cpp
+++ b/test/Analysis/temporaries.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17
+// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s
+// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s
+// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11
+// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17
 
 // Note: The C++17 run-line doesn't -verify yet - it is a no-crash test.
 
@@ -962,6 +962,87 @@ C &&bar2() { return foo2(); } // no-crash
 } // end namespace pass_references_through
 
 
+namespace arguments {
+int glob;
+
+struct S {
+  int x;
+  S(int x): x(x) {}
+  S(const S &s) : x(s.x) {}
+  ~S() {}
+
+  S &operator+(S s) {
+    glob = s.x;
+    x += s.x;
+    return *this;
+  }
+};
+
+class C {
+public:
+  virtual void bar3(S s) {}
+};
+
+class D: public C {
+public:
+  D() {}
+  virtual void bar3(S s) override { glob = s.x; }
+};
+
+void bar1(S s) {
+  glob = s.x;
+}
+
+// Record-typed calls are a different CFGStmt, let's see if we handle that
+// as well.
+S bar2(S s) {
+  glob = s.x;
+  return S(3);
+}
+
+void bar5(int, ...);
+
+void foo(void (*bar4)(S)) {
+  bar1(S(1));
+  clang_analyzer_eval(glob == 1);
+#ifdef TEMPORARY_DTORS
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+
+  bar2(S(2));
+  clang_analyzer_eval(glob == 2);
+#ifdef TEMPORARY_DTORS
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+
+  C *c = new D();
+  c->bar3(S(3));
+  // FIXME: Should be TRUE.
+  clang_analyzer_eval(glob == 3); // expected-warning{{UNKNOWN}}
+  delete c;
+
+  // What if we've no idea what we're calling?
+  bar4(S(4)); // no-crash
+
+  S(5) + S(6);
+  clang_analyzer_eval(glob == 6);
+#ifdef TEMPORARY_DTORS
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+
+  // Variadic functions. This will __builtin_trap() because you cannot pass
+  // an object as a variadic argument.
+  bar5(7, S(7)); // no-crash
+  clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace arguments
+
 namespace ctor_argument {
 // Stripped down unique_ptr<int>
 struct IntPtr {
@@ -1004,3 +1085,70 @@ void foo() {
 }
 } // namespace operator_implicit_argument
 
+
+#if __cplusplus >= 201103L
+namespace argument_lazy_bindings {
+int glob;
+
+struct S {
+  int x, y, z;
+};
+
+struct T {
+  S s;
+  int w;
+  T(int w): s{5, 6, 7}, w(w) {}
+};
+
+void foo(T t) {
+  t.s = {1, 2, 3};
+  glob = t.w;
+}
+
+void bar() {
+  foo(T(4));
+  clang_analyzer_eval(glob == 4); // expected-warning{{TRUE}}
+}
+} // namespace argument_lazy_bindings
+#endif
+
+namespace operator_argument_cleanup {
+struct S {
+  S();
+};
+
+class C {
+public:
+  void operator=(S);
+};
+
+void foo() {
+  C c;
+  c = S(); // no-crash
+}
+} // namespace operator_argument_cleanup
+
+namespace argument_decl_lookup {
+class C {};
+int foo(C);
+int bar(C c) { foo(c); }
+int foo(C c) {}
+} // namespace argument_decl_lookup
+
+namespace argument_virtual_decl_lookup {
+class C {};
+
+struct T  {
+  virtual void foo(C);
+};
+
+void run() {
+  T *t;
+  t->foo(C()); // no-crash // expected-warning{{Called C++ object pointer is uninitialized}}
+}
+
+// This is after run() because the test is about picking the correct decl
+// for the parameter region, which should belong to the correct function decl,
+// and the non-definition decl should be found by direct lookup.
+void T::foo(C) {}
+} // namespace argument_virtual_decl_lookup
diff --git a/test/Analysis/temporaries.mm b/test/Analysis/temporaries.mm
index 3b6166db6e..43546ae344 100644
--- a/test/Analysis/temporaries.mm
+++ b/test/Analysis/temporaries.mm
@@ -2,6 +2,8 @@
 
 // expected-no-diagnostics
 
+#define nil ((id)0)
+
 // Stripped down unique_ptr<int>
 struct IntPtr {
   IntPtr(): i(new int) {}
@@ -15,9 +17,13 @@ struct IntPtr {
   -(void) foo: (IntPtr)arg;
 @end
 
-void bar(Foo *f) {
+void testArgumentRegionInvalidation(Foo *f) {
   IntPtr ptr;
   int *i = ptr.i;
   [f foo: static_cast<IntPtr &&>(ptr)];
   *i = 99; // no-warning
 }
+
+void testNilReceiverCleanup() {
+  [nil foo: IntPtr()];
+}