]> granicus.if.org Git - clang/commitdiff
[analyzer] Fix a crash on lifetime extension through aggregate initialization.
authorArtem Dergachev <artem.dergachev@gmail.com>
Wed, 25 Apr 2018 23:02:06 +0000 (23:02 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Wed, 25 Apr 2018 23:02:06 +0000 (23:02 +0000)
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

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/lifetime-extension.cpp

index ba42d816d8e5ef0d40ae87dbafde46d885f155b1..0d2d4aa0151651e764615c73316971c67130e910 100644 (file)
@@ -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() {}
   };
 
index a6aca7e242eea2aeeadb84866bb46dfc188c8640..31d198ecb28439788f27c30e0b99de5fae34b0a1 100644 (file)
@@ -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;
           }
         }
       }
index 6fafadda0d241225d31a1fc5dc9cbb3c3ae73fd0..2b1d55d7e78ce232092c2a539379be4f62dc1302 100644 (file)
@@ -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;
index c9038a2b5bba9cc4142ad4c16d898117135df3f6..9173129ac14eb8cf30de994a0adbdf9baf8e7aae 100644 (file)
@@ -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 {