[CFG] [analyzer] Don't treat argument constructors as temporary constructors.

[NoQ]

Summary

Function argument constructors (that are used for passing objects into functions by value) are completely unlike temporary object constructors, but we were treating them as such because they are also wrapped into a CXXBindTemporaryExpr.

This patch adds a partial construction context layer for call argument values, but doesn't proceed to transform it into an actual construction context yet. This is tells the clients that we aren't supporting these constructors yet.

A lesson to learn here is that construction contexts aren't as safe as i originally expected - even though they work by pattern-matching small specific AST patterns.

[NoQ]

Add tests where the argument is passed by reference. These tests work correctly (i.e. they correctly identify the argument constructor as a temporary constructor) because such constructors would include a MaterializeTemporaryExpr that is not allowed in the middle of the partial construction context. For the same reason remove the defensive check in the branch of ConstructionContext::createFromLayers that deals with MaterializeTemporaryExpr-based layers.

lib/Analysis/CFG.cpp

CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) {
   if (calleeType == Context->BoundMemberTy) {
     QualType boundType = Expr::findBoundMemberType(C->getCallee());
 
     // We should only get a null bound type if processing a dependent
     // CFG.  Recover by assuming nothing.
     if (!boundType.isNull()) calleeType = boundType;
   }
 
+  // FIXME: Once actually implemented, this construction context layer should
+  // include the number of the argument as well.
+  for (auto Arg: C->arguments()) {
+    findConstructionContexts(
+        ConstructionContextLayer::create(cfg->getBumpVectorContext(), C), Arg);
+  }
+
   // If this is a call to a no-return function, this stops the block here.
   bool NoReturn = getFunctionExtInfo(*calleeType).getNoReturn();
 
   bool AddEHEdge = false;
 
   // Languages without exceptions are assumed to not throw.
   if (Context->getLangOpts().Exceptions) {
     if (BuildOpts.AddEHEdges)

lib/Analysis/ConstructionContext.cpp

if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(S)) {
         assert(ParentLayer->getTriggerStmt() &&
                "Non-statement-based layers have been handled above!");
         // This is the normal, non-C++17 case: a temporary object which has
         // both destruction and materialization info attached to it in the AST.
         if ((MTE = dyn_cast<MaterializeTemporaryExpr>(
                  ParentLayer->getTriggerStmt()))) {
           return create<TemporaryObjectConstructionContext>(C, BTE, MTE);
         }
+        // This is a constructor into a function argument. Not implemented yet.
+        if (auto *CE = dyn_cast<CallExpr>(ParentLayer->getTriggerStmt()))
+          return nullptr;
         // This is C++17 copy-elided construction into return statement.
         if (auto *RS = dyn_cast<ReturnStmt>(ParentLayer->getTriggerStmt())) {
           assert(!RS->getRetValue()->getType().getCanonicalType()
                     ->getAsCXXRecordDecl()->hasTrivialDestructor());
           return create<CXX17ElidedCopyReturnedValueConstructionContext>(C,
                                                                        RS, BTE);
         }
         // This is C++17 copy-elided construction into a simple variable.
if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(S)) {
       // If the object requires destruction and is not lifetime-extended,
       // then it must have a BTE within its MTE.
       // FIXME: This should be an assertion.
       if (!(MTE->getType().getCanonicalType()
                 ->getAsCXXRecordDecl()->hasTrivialDestructor() ||
              MTE->getStorageDuration() != SD_FullExpression))
         return nullptr;
 
+      // This is a constructor into a function argument. Not implemented yet.
+      if (const ConstructionContextLayer *ParentLayer = TopLayer->getParent())
+        if (auto *CE = dyn_cast<CallExpr>(ParentLayer->getTriggerStmt()))
+          return nullptr;
       assert(TopLayer->isLast());
       return create<TemporaryObjectConstructionContext>(C, nullptr, MTE);
     }
     if (const auto *RS = dyn_cast<ReturnStmt>(S)) {
       assert(TopLayer->isLast());
       return create<SimpleReturnedValueConstructionContext>(C, RS);
     }
+    // This is a constructor into a function argument. Not implemented yet.
+    if (auto *CE = dyn_cast<CallExpr>(TopLayer->getTriggerStmt()))
+      return nullptr;
     llvm_unreachable("Unexpected construction context with statement!");
   } else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) {
     assert(TopLayer->isLast());
     return create<SimpleConstructorInitializerConstructionContext>(C, I);
   }
   llvm_unreachable("Unexpected construction context!");
 }

test/Analysis/cfg-rich-constructors.cpp

 // CHECK-NEXT:     9: [B1.8]
 // CHECK-NEXT:    10: const implicit_constructor_conversion::B &b = get();
 // CHECK-NEXT:    11: [B1.10].~B() (Implicit destructor)
 void implicitConstructionConversionFromFunctionValueWithLifetimeExtension() {
   const B &b = get(); // no-crash
 }
 
 } // end namespace implicit_constructor_conversion
+
+namespace argument_constructors {
+class D {
+public:
+  D();
+  ~D();
+};
+
+void useC(C c);
+void useD(D d);
+
+// FIXME: Find construction context for the argument.
+// CHECK: void passArgument()
+// CHECK:          1: useC
+// CHECK-NEXT:     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(class C))
+// CXX11-NEXT:     3: C() (CXXConstructExpr, [B1.4], class C)
+// CXX11-NEXT:     4: [B1.3]
+// CXX11-NEXT:     5: [B1.4] (CXXConstructExpr, class C)
+// CXX11-NEXT:     6: [B1.2]([B1.5])
+// CXX17-NEXT:     3: C() (CXXConstructExpr, class C)
+// CXX17-NEXT:     4: [B1.2]([B1.3])
+void passArgument() {
+  useC(C());
+}
+
+// FIXME: Find construction context for the argument.
+// CHECK: void passArgumentWithDestructor()
+// CHECK:          1: useD
+// CHECK-NEXT:     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(class argument_constructors::D))
+// CXX11-NEXT:     3: argument_constructors::D() (CXXConstructExpr, [B1.4], [B1.6], class argument_constructors::D)
+// CXX11-NEXT:     4: [B1.3] (BindTemporary)
+// CXX11-NEXT:     5: [B1.4] (ImplicitCastExpr, NoOp, const class argument_constructors::D)
+// CXX11-NEXT:     6: [B1.5]
+// CXX11-NEXT:     7: [B1.6] (CXXConstructExpr, class argument_constructors::D)
+// CXX11-NEXT:     8: [B1.7] (BindTemporary)
+// CXX11-NEXT:     9: [B1.2]([B1.8])
+// CXX11-NEXT:    10: ~argument_constructors::D() (Temporary object destructor)
+// CXX11-NEXT:    11: ~argument_constructors::D() (Temporary object destructor)
+// CXX17-NEXT:     3: argument_constructors::D() (CXXConstructExpr, class argument_constructors::D)
+// CXX17-NEXT:     4: [B1.3] (BindTemporary)
+// CXX17-NEXT:     5: [B1.2]([B1.4])
+// CXX17-NEXT:     6: ~argument_constructors::D() (Temporary object destructor)
+void passArgumentWithDestructor() {
+  useD(D());
+}
+} // end namespace argument_constructors