[PATCH] D44273: [CFG] [analyzer] Fix a crash on finding construction context for an lvalue call expression.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 137661.
NoQ added a comment.

Same goes for rvalue references. Hmm, we have a fancy helper method for that exact purpose. But it's not super easy to use, had to sacrifice a sanity check assertion to use it.


https://reviews.llvm.org/D44273

Files:
  include/clang/Analysis/CFG.h
  lib/Analysis/CFG.cpp
  test/Analysis/temporaries.cpp


Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1032,4 +1032,17 @@
 }
 } // end namespace implicit_constructor_conversion
 
+namespace pass_references_through {
+class C {
+public:
+  ~C() {}
+};
+
+const C &foo1();
+C &&foo2();
 
+// In these example 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
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -729,7 +729,7 @@
 
   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 =
@@ -1217,7 +1217,7 @@
   case Stmt::CXXOperatorCallExprClass:
   case Stmt::UserDefinedLiteralClass: {
     auto *CE = cast<CallExpr>(Child);
-    if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE))
+    if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context))
       consumeConstructionContext(Layer, CE);
     break;
   }
Index: include/clang/Analysis/CFG.h
===================================================================
--- include/clang/Analysis/CFG.h
+++ include/clang/Analysis/CFG.h
@@ -181,14 +181,16 @@
 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));
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44273.137661.patch
Type: text/x-patch
Size: 2748 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180308/79df5d52/attachment.bin>