[analyzer] Re-enable C++17-specific return value constructors.

[NoQ]

Summary

The refactoring conducted in D47304 made it easy for the analyzer to find the target region for the constructor across multiple stack frames. We ascend to the parent stack frame recursively until we find a construction context that doesn't represent yet another return value of a function. This is the semantics of copy elision in return statements that return the object by value: they return it directly to a memory region (eg., a variable) that may be located a few (indefinitely many) stack frames above the statement. In particular, this is how return statements work in C++17 where copy elision is mandatory. This commit enables this feature for the AST of C++17, but extra work is necessary to perform copy elision in the AST produced by older language standard modes.

[NoQ]

test/Analysis/cxx17-mandatory-elision.cpp

185–191

This sounded like a good thing to check, so i added such checks in a few more places.

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

case ConstructionContext::NewAllocatedObjectKind: {
           }
           return std::make_pair(State, V);
         }
         // TODO: Detect when the allocator returns a null pointer.
         // Constructor shall not be called in this case.
       }
       break;
     }
-    case ConstructionContext::SimpleReturnedValueKind: {
+    case ConstructionContext::SimpleReturnedValueKind:
+    case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
       // The temporary is to be managed by the parent stack frame.
       // So build it in the parent stack frame if we're not in the
       // top frame of the analysis.
       const StackFrameContext *SFC = LCtx->getCurrentStackFrame();
       if (const LocationContext *CallerLCtx = SFC->getParent()) {
         auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
                        .getAs<CFGCXXRecordTypedCall>();
         if (!RTC) {
           // We were unable to find the correct construction context for the
           // call in the parent stack frame. This is equivalent to not being
           // able to find construction context at all.
           break;
-        } else if (!isa<TemporaryObjectConstructionContext>(
-                       RTC->getConstructionContext())) {
-          // FIXME: The return value is constructed directly into a
-          // non-temporary due to C++17 mandatory copy elision. This is not
-          // implemented yet.
-          assert(getContext().getLangOpts().CPlusPlus17);
-          break;
         }
-        CC = RTC->getConstructionContext();
-        LCtx = CallerLCtx;
+        return prepareForObjectConstruction(
+            cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+            RTC->getConstructionContext(), CallOpts);
       } else {
         // We are on the top frame of the analysis.
         // TODO: What exactly happens when we are? Does the temporary object
         // live long enough in the region store in this case? Would checkers
         // think that this object immediately goes out of scope?
         CallOpts.IsTemporaryCtorOrDtor = true;
         SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
         return std::make_pair(State, V);
       }
-
-      // Continue as if we have a temporary with a different location context.
-      // FALLTHROUGH.
+      llvm_unreachable("Unhandled return value construction context!");
     }
     case ConstructionContext::TemporaryObjectKind: {
       const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
       const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
       const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
 
       if (!BTE) {
         // FIXME: Lifetime extension for temporaries without destructors
case ConstructionContext::TemporaryObjectKind: {
         State = addObjectUnderConstruction(State, BTE, LCtx, V);
 
       if (MTE)
         State = addObjectUnderConstruction(State, MTE, LCtx, V);
 
       CallOpts.IsTemporaryCtorOrDtor = true;
       return std::make_pair(State, V);
     }
-    case ConstructionContext::CXX17ElidedCopyReturnedValueKind:
-      // Not implemented yet.
-      break;
     }
   }
   // If we couldn't find an existing region to construct into, assume we're
   // constructing a temporary. Notify the caller of our failure.
   CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
   return std::make_pair(
       State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
 }

test/Analysis/cxx17-mandatory-elision.cpp

ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) {
   return make2(v);
 }
 
 void testMultipleReturns() {
   AddressVector<ClassWithoutDestructor> v;
   ClassWithoutDestructor c = make3(v);
 
 #if __cplusplus >= 201703L
-  // FIXME: Both should be TRUE.
   clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
-  clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{FALSE}}
+  clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{TRUE}}
 #else
   clang_analyzer_eval(v.len == 5); // 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}}
   clang_analyzer_eval(v.buf[4] == &c); // expected-warning{{TRUE}}
 #endif
public:
 
   ~ClassWithDestructor() { v.push(this); }
 };
 
 void testVariable() {
   AddressVector<ClassWithDestructor> v;
   {
     ClassWithDestructor c = ClassWithDestructor(v);
+    // Check if the last destructor is an automatic destructor.
+    // A temporary destructor would have fired by now.
+#if __cplusplus >= 201703L
+    clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
+#else
+    clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}}
+#endif
   }
 #if __cplusplus >= 201703L
   // 0. Construct the variable.
   // 1. Destroy the variable.
   clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
 #else
   // 0. Construct the temporary.
struct TestCtorInitializer {
   TestCtorInitializer(AddressVector<ClassWithDestructor> &v)
     : c(ClassWithDestructor(v)) {}
 };
 
 void testCtorInitializer() {
   AddressVector<ClassWithDestructor> v;
   {
     TestCtorInitializer t(v);
+    // Check if the last destructor is an automatic destructor.
+    // A temporary destructor would have fired by now.
+#if __cplusplus >= 201703L
+    clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
+#else
+    clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}}
+#endif
   }
 #if __cplusplus >= 201703L
   // 0. Construct the member variable.
   // 1. Destroy the member variable.
   clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
 #else
   // 0. Construct the temporary.
   // 1. Construct the member variable.
   // 2. Destroy the temporary.
   // 3. Destroy the member variable.
   clang_analyzer_eval(v.len == 4); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[0] == v.buf[2]); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[1] == v.buf[3]); // expected-warning{{TRUE}}
 #endif
 }
 
+
+ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) {
+  return ClassWithDestructor(v);
+}
+ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) {
+  return make1(v);
+}
+ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) {
+  return make2(v);
+}
+
+void testMultipleReturnsWithDestructors() {
+  AddressVector<ClassWithDestructor> v;
+  {
+    ClassWithDestructor c = make3(v);
+    // Check if the last destructor is an automatic destructor.
+    // A temporary destructor would have fired by now.
+#if __cplusplus >= 201703L
+    clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
+#else
+    clang_analyzer_eval(v.len == 9); // expected-warning{{TRUE}}
+#endif
+  }
+
+#if __cplusplus >= 201703L
+  // 0. Construct the variable. Yes, constructor in make1() constructs
+  //    the variable 'c'.
+  // 1. Destroy the variable.
+  clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // 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 variable.
+  // 8. Destroy the temporary here.
+  // 9. Destroy the variable.
+  clang_analyzer_eval(v.len == 10); // 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[8]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[7] == v.buf[9]); // expected-warning{{TRUE}}
+#endif
+}
 } // namespace address_vector_tests