From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Mon, 12 Mar 2018 23:52:36 +0000 (+0000)
Subject: [CFG] [analyzer] Don't add construction context to a return-by-reference call.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=25903b2d3f6caa41daecd98a6e3e1ff478eae6ce;p=clang

[CFG] [analyzer] Don't add construction context to a return-by-reference call.

Call expressions that return objects by an lvalue reference or an rvalue
reference have a value type in the AST but wear an auxiliary flag of being an
lvalue or an xvalue respectively.

Use the helper method for obtaining the actual return type of the function.

Fixes a crash.

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


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

diff --git a/include/clang/Analysis/CFG.h b/include/clang/Analysis/CFG.h
index 133d207ca6..bb66aac3f6 100644
--- a/include/clang/Analysis/CFG.h
+++ b/include/clang/Analysis/CFG.h
@@ -183,14 +183,16 @@ class CFGCXXRecordTypedCall : public CFGStmt {
 public:
   /// Returns true when call expression \p CE needs to be represented
   /// by CFGCXXRecordTypedCall, as opposed to a regular CFGStmt.
-  static bool isCXXRecordTypedCall(CallExpr *CE) {
-    return CE->getType().getCanonicalType()->getAsCXXRecordDecl();
+  static bool isCXXRecordTypedCall(CallExpr *CE, const ASTContext &ACtx) {
+    return CE->getCallReturnType(ACtx).getCanonicalType()->getAsCXXRecordDecl();
   }
 
   explicit CFGCXXRecordTypedCall(CallExpr *CE,
-                             const TemporaryObjectConstructionContext *C)
+                                 const TemporaryObjectConstructionContext *C)
       : CFGStmt(CE, CXXRecordTypedCall) {
-    assert(isCXXRecordTypedCall(CE));
+    // FIXME: This is not protected against squeezing a non-record-typed-call
+    // into the constructor. An assertion would require passing an ASTContext
+    // which would mean paying for something we don't use.
     assert(C);
     Data2.setPointer(const_cast<TemporaryObjectConstructionContext *>(C));
   }
diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 6aab6b6904..98bffde891 100644
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -751,7 +751,7 @@ private:
 
   void appendCall(CFGBlock *B, CallExpr *CE) {
     if (BuildOpts.AddRichCXXConstructors) {
-      if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE)) {
+      if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) {
         if (const ConstructionContextLayer *Layer =
                 ConstructionContextMap.lookup(CE)) {
           const ConstructionContext *CC =
@@ -1265,7 +1265,7 @@ void CFGBuilder::findConstructionContexts(
   case Stmt::CXXOperatorCallExprClass:
   case Stmt::UserDefinedLiteralClass: {
     auto *CE = cast<CallExpr>(Child);
-    if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE))
+    if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context))
       consumeConstructionContext(Layer, CE);
     break;
   }
diff --git a/test/Analysis/temp-obj-dtors-cfg-output.cpp b/test/Analysis/temp-obj-dtors-cfg-output.cpp
index eecbabe5a1..62e5189f28 100644
--- a/test/Analysis/temp-obj-dtors-cfg-output.cpp
+++ b/test/Analysis/temp-obj-dtors-cfg-output.cpp
@@ -205,6 +205,21 @@ int testConsistencyNestedNormalReturn(bool value) {
   return 0;
 }
 
+namespace pass_references_through {
+class C {
+public:
+  ~C() {}
+};
+
+const C &foo1();
+C &&foo2();
+
+// In these examples the foo() expression has record type, not reference type.
+// Don't try to figure out how to perform construction of the record here.
+const C &bar1() { return foo1(); } // no-crash
+C &&bar2() { return foo2(); } // no-crash
+} // end namespace pass_references_through
+
 // CHECK:   [B1 (ENTRY)]
 // CHECK:     Succs (1): B0
 // CHECK:   [B0 (EXIT)]
@@ -1402,3 +1417,29 @@ int testConsistencyNestedNormalReturn(bool value) {
 // CHECK:     Succs (2): B8 B1
 // CHECK:   [B0 (EXIT)]
 // CHECK:     Preds (3): B1 B2 B4
+// CHECK:   [B1 (ENTRY)]
+// CHECK:     Succs (1): B0
+// CHECK:   [B0 (EXIT)]
+// CHECK:     Preds (1): B1
+// CHECK:   [B2 (ENTRY)]
+// CHECK:     Succs (1): B1
+// CHECK:   [B1]
+// CHECK:     1: foo1
+// CHECK:     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, const class pass_references_through::C &(*)(void))
+// CHECK:     3: [B1.2]()
+// CHECK:     4: return [B1.3];
+// CHECK:     Preds (1): B2
+// CHECK:     Succs (1): B0
+// CHECK:   [B0 (EXIT)]
+// CHECK:     Preds (1): B1
+// CHECK:   [B2 (ENTRY)]
+// CHECK:     Succs (1): B1
+// CHECK:   [B1]
+// CHECK:     1: foo2
+// CHECK:     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class pass_references_through::C &&(*)(void))
+// CHECK:     3: [B1.2]()
+// CHECK:     4: return [B1.3];
+// CHECK:     Preds (1): B2
+// CHECK:     Succs (1): B0
+// CHECK:   [B0 (EXIT)]
+// CHECK:     Preds (1): B1
diff --git a/test/Analysis/temporaries.cpp b/test/Analysis/temporaries.cpp
index 0d2c610d6b..b3851b42a0 100644
--- a/test/Analysis/temporaries.cpp
+++ b/test/Analysis/temporaries.cpp
@@ -1032,4 +1032,17 @@ void test() {
 }
 } // end namespace implicit_constructor_conversion
 
+namespace pass_references_through {
+class C {
+public:
+  ~C() {}
+};
+
+const C &foo1();
+C &&foo2();
 
+// In these examples the foo() expression has record type, not reference type.
+// Don't try to figure out how to perform construction of the record here.
+const C &bar1() { return foo1(); } // no-crash
+C &&bar2() { return foo2(); } // no-crash
+} // end namespace pass_references_through