]> granicus.if.org Git - clang/commitdiff
[analyzer] Re-enable C++17-specific RVO construction contexts.
authorArtem Dergachev <artem.dergachev@gmail.com>
Thu, 14 Jun 2018 01:59:35 +0000 (01:59 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Thu, 14 Jun 2018 01:59:35 +0000 (01:59 +0000)
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

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

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/cxx17-mandatory-elision.cpp

index 1dd094b90f832b1361904a7d926b15d67c788d3e..45c8ed17728ce7b3a52a66eb91975eb7a907e151 100644 (file)
@@ -180,7 +180,8 @@ std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
       }
       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> ExprEngine::prepareForObjectConstruction(
           // 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> ExprEngine::prepareForObjectConstruction(
         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> ExprEngine::prepareForObjectConstruction(
       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
index be786be541e6d5c2d5d11fd3db9566bf4f12ca35..e82610527965a00e6f05d22b8557ec3ab2a77a8a 100644 (file)
@@ -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