[analyzer] Support temporaries conjured by conservatively evaluated functions.

[NoQ]

Summary

If a conservatively evaluated function returns a C++ object by value, it no longer returns a conjured symbol. Instead it creates a proper temporary region, wipes it with a conjured symbol, returns the compound value, and then lifetime-extends/destroys the newly created object properly, while making sure its this-region is known from the beginning to the end of the object's lifetime and doesn't change in the process.

This review obsoletes D27202 and is better because the temporary is not only created but also correctly managed.

It allows us, in particular, to inline the destructor of such conjured temporary objects. I do not know whether it is a good idea or we should disable inlining specifically via a separate flag. But even without inlining, this change would hopefully help various C++ checkers track the object by keeping both its lvalue and its rvalue stable in all circumstances (as long as the construction context is available). For now i did not have a look at how current checkers are affected, but @baloghadamsoftware and @szepet might be interested.

C objects are untouched (i.e. still represented by conjured symbols) because they don't require lifetime management or even having a "this".

[NoQ]

Rebase.

[george.karpenkov]

LGTM

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) {
 
     // If the constructed object is a temporary prvalue, get its bindings.
     if (isTemporaryPRValue(cast<CXXConstructExpr>(E), ThisV))
       ThisV = State->getSVal(ThisV.castAs<Loc>());
 
     return State->BindExpr(E, LCtx, ThisV);
   }
 
-  // Conjure a symbol if the return value is unknown.
+  SVal R;
   QualType ResultTy = Call.getResultType();
-  SValBuilder &SVB = getSValBuilder();
   unsigned Count = currBldrCtx->blockCount();
+  if (auto RTC = getCurrentCFGElement().getAs<CFGCXXRecordTypedCall>()) {
+    // Conjure a temporary if the function returns an object by value.
+    MemRegionManager &MRMgr = svalBuilder.getRegionManager();
+    const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx);
+    State = addAllNecessaryTemporaryInfo(State, RTC->getConstructionContext(),
+                                         LCtx, TR);
+
+    // Invalidate the region so that it didn't look uninitialized. Don't notify
+    // the checkers.
+    State = State->invalidateRegions(TR, E, Count, LCtx,
+                                     /* CausedByPointerEscape=*/false, nullptr,
+                                     &Call, nullptr);
+
+    R = State->getSVal(TR, E->getType());
+  } else {
+    // Conjure a symbol if the return value is unknown.
 
     // See if we need to conjure a heap pointer instead of
     // a regular unknown pointer.
     bool IsHeapPointer = false;
     if (const auto *CNE = dyn_cast<CXXNewExpr>(E))
       if (CNE->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
         // FIXME: Delegate this to evalCall in MallocChecker?
         IsHeapPointer = true;
       }
 
-  SVal R = IsHeapPointer
-               ? SVB.getConjuredHeapSymbolVal(E, LCtx, Count)
-               : SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
+    R = IsHeapPointer ? svalBuilder.getConjuredHeapSymbolVal(E, LCtx, Count)
+                      : svalBuilder.conjureSymbolVal(nullptr, E, LCtx, ResultTy,
+                                                     Count);
+  }
   return State->BindExpr(E, LCtx, R);
 }
 
 // Conservatively evaluate call by invalidating regions and binding
 // a conjured return value.
 void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
                                       ExplodedNode *Pred,
                                       ProgramStateRef State) {

test/Analysis/explain-svals.cpp

void test_5(int i) {
     clang_analyzer_explain(&x[i]); // expected-warning-re{{{{^pointer to element of type 'int' with index 'argument 'i'' of field 'x' of 'this' object$}}}}
     clang_analyzer_explain(__builtin_alloca(i)); // expected-warning-re{{{{^pointer to region allocated by '__builtin_alloca\(i\)'$}}}}
   }
 };
 } // end of anonymous namespace
 
 void test_6() {
   clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of temporary object constructed at statement 'conjure_S\(\)'$}}}}
-  clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'struct S' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}}
+  clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}}
 }

test/Analysis/temporaries.cpp

 #else
     // expected-warning@-6{{UNKNOWN}}
     // expected-warning@-6{{UNKNOWN}}
 #endif
   }
 }
 } // namespace test_match_constructors_and_destructors
 
+namespace destructors_for_return_values {
+
+class C {
+public:
+  ~C() {
+    1 / 0; // expected-warning{{Division by zero}}
+  }
+};
+
+C make();
+
+void testFloatingCall() {
+  make();
+  // Should have divided by zero in the destructor.
+  clang_analyzer_warnIfReached();
+#ifndef TEMPORARY_DTORS
+    // expected-warning@-2{{REACHABLE}}
+#endif
+}
+
+void testLifetimeExtendedCall() {
+  {
+    const C &c = make();
+    clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }
+  // Should have divided by zero in the destructor.
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void testCopiedCall() {
+  C c = make();
+  // Should have divided by zero in the temporary destructor.
+  clang_analyzer_warnIfReached();
+#ifndef TEMPORARY_DTORS
+    // expected-warning@-2{{REACHABLE}}
+#endif
+}
+} // namespace destructors_for_return_values
+
 namespace dont_forget_destructor_around_logical_op {
 int glob;
 
 class C {
 public:
   ~C() {
     glob = 1;
     clang_analyzer_checkInlined(true);

 void test(int coin) {
   // Here temporaries are being cleaned up after && is evaluated. There are two
   // temporaries: the return value of get() and the elidable copy constructor
   // of that return value into is(). According to the CFG, we need to cleanup
   // both of them depending on whether the temporary corresponding to the
   // return value of get() was initialized. However, we didn't track
   // temporaries returned from functions, so we took the wrong branch.
   coin && is(get()); // no-crash
-  // FIXME: Should be true once we inline all destructors.
+  if (coin) {
+    clang_analyzer_eval(glob);
+#ifdef TEMPORARY_DTORS
+    // expected-warning@-2{{TRUE}}
+#else
+    // expected-warning@-4{{UNKNOWN}}
+#endif
+  } else {
+    // The destructor is not called on this branch.
     clang_analyzer_eval(glob); // expected-warning{{UNKNOWN}}
   }
+}
 } // namespace dont_forget_destructor_around_logical_op
 
 #if __cplusplus >= 201103L
 namespace temporary_list_crash {
 class C {
 public:
   C() {}
   ~C() {}