r334684 - [analyzer] Re-enable C++17-specific RVO construction contexts.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 13 18:59:35 PDT 2018


Author: dergachev
Date: Wed Jun 13 18:59:35 2018
New Revision: 334684

URL: http://llvm.org/viewvc/llvm-project?rev=334684&view=rev
Log:
[analyzer] Re-enable C++17-specific RVO construction contexts.

Not contexts themselves, but rather support for them in the analyzer.

Such construction contexts appear when C++17 mandatory copy elision occurs
while returning an object from a function, and presence of a destructor causes
a CXXBindTemporaryExpr to appear in the AST.

Additionally, such construction contexts may be chained, because a return-value
construction context doesn't really explain where the object is being returned
into, but only points to the parent stack frame, where the object may be
consumed by literally anything including another return statement. This
behavior is now modeled correctly by the analyzer as long as the object is not
returned beyond the boundaries of the analysis.

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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=334684&r1=334683&r2=334684&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Jun 13 18:59:35 2018
@@ -180,7 +180,8 @@ std::pair<ProgramStateRef, SVal> ExprEng
       }
       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.
@@ -193,16 +194,10 @@ std::pair<ProgramStateRef, SVal> ExprEng
           // 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
@@ -212,9 +207,7 @@ std::pair<ProgramStateRef, SVal> ExprEng
         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);
@@ -261,9 +254,6 @@ std::pair<ProgramStateRef, SVal> ExprEng
       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

Modified: cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp?rev=334684&r1=334683&r2=334684&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp (original)
+++ cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp Wed Jun 13 18:59:35 2018
@@ -150,9 +150,8 @@ void testMultipleReturns() {
   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}}
@@ -183,6 +182,13 @@ 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.
@@ -210,6 +216,13 @@ 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.
@@ -227,4 +240,53 @@ void testCtorInitializer() {
 #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




More information about the cfe-commits mailing list