From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Wed, 25 Apr 2018 23:02:06 +0000 (+0000)
Subject: [analyzer] Fix a crash on lifetime extension through aggregate initialization.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=58aba33e0faf376c36a5e72e066bdf03335f5bba;p=clang

[analyzer] Fix a crash on lifetime extension through aggregate initialization.

If 'A' is a C++ aggregate with a reference field of type 'C', in code like
  A a = { C() };
C() is lifetime-extended by 'a'. The analyzer wasn't expecting this pattern and
crashing. Additionally, destructors aren't added in the CFG for this case,
so for now we shouldn't be inlining the constructor for C().

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


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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index ba42d816d8..0d2d4aa015 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -110,6 +110,11 @@ public:
     /// 'const int &x = C().x;'.
     bool IsTemporaryLifetimeExtendedViaSubobject = false;
 
+    /// This call is a constructor for a temporary that is lifetime-extended
+    /// by binding it to a reference-type field within an aggregate,
+    /// for example 'A { const C &c; }; A a = { C() };'
+    bool IsTemporaryLifetimeExtendedViaAggregate = false;
+
     EvalCallOptions() {}
   };
 
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index a6aca7e242..31d198ecb2 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -180,14 +180,25 @@ ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
     }
     case ConstructionContext::TemporaryObjectKind: {
       const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC);
-      // See if we're lifetime-extended via our field. If so, take a note.
-      // Because automatic destructors aren't quite working in this case.
       if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) {
         if (const ValueDecl *VD = MTE->getExtendingDecl()) {
-          assert(VD->getType()->isReferenceType());
-          if (VD->getType()->getPointeeType().getCanonicalType() !=
-              MTE->GetTemporaryExpr()->getType().getCanonicalType()) {
-            CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true;
+          // Pattern-match various forms of lifetime extension that aren't
+          // currently supported by the CFG.
+          // FIXME: Is there a better way to retrieve this information from
+          // the MaterializeTemporaryExpr?
+          assert(MTE->getStorageDuration() != SD_FullExpression);
+          if (VD->getType()->isReferenceType()) {
+            assert(VD->getType()->isReferenceType());
+            if (VD->getType()->getPointeeType().getCanonicalType() !=
+                MTE->GetTemporaryExpr()->getType().getCanonicalType()) {
+              // We're lifetime-extended via our field. Automatic destructors
+              // aren't quite working in this case.
+              CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true;
+            }
+          } else {
+            // We're lifetime-extended by a surrounding aggregate.
+            // Automatic destructors aren't quite working in this case.
+            CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
           }
         }
       }
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 6fafadda0d..2b1d55d7e7 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -699,6 +699,11 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
       // within it to a reference, automatic destructors don't work properly.
       if (CallOpts.IsTemporaryLifetimeExtendedViaSubobject)
         return CIP_DisallowedOnce;
+
+      // If the temporary is lifetime-extended by binding it to a reference-typ
+      // field within an aggregate, automatic destructors don't work properly.
+      if (CallOpts.IsTemporaryLifetimeExtendedViaAggregate)
+        return CIP_DisallowedOnce;
     }
 
     break;
diff --git a/test/Analysis/lifetime-extension.cpp b/test/Analysis/lifetime-extension.cpp
index c9038a2b5b..9173129ac1 100644
--- a/test/Analysis/lifetime-extension.cpp
+++ b/test/Analysis/lifetime-extension.cpp
@@ -147,6 +147,37 @@ void f5() {
   // FIXME: Should be TRUE. Should not warn about garbage value.
   clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
 }
+
+struct A { // A is an aggregate.
+  const C &c;
+};
+
+void f6() {
+  C *after, *before;
+  {
+    A a{C(true, &after, &before)};
+  }
+  // FIXME: Should be TRUE. Should not warn about garbage value.
+  clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
+}
+
+void f7() {
+  C *after, *before;
+  {
+    A a = {C(true, &after, &before)};
+  }
+  // FIXME: Should be TRUE. Should not warn about garbage value.
+  clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
+}
+
+void f8() {
+  C *after, *before;
+  {
+    A a[2] = {C(false, nullptr, nullptr), C(true, &after, &before)};
+  }
+  // FIXME: Should be TRUE. Should not warn about garbage value.
+  clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
+}
 } // end namespace maintain_original_object_address_on_lifetime_extension
 
 namespace maintain_original_object_address_on_move {