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

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Apr 25 16:02:06 2018
New Revision: 330882

URL: http://llvm.org/viewvc/llvm-project?rev=330882&view=rev
Log:
[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

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

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=330882&r1=330881&r2=330882&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Wed Apr 25 16:02:06 2018
@@ -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() {}
   };
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=330882&r1=330881&r2=330882&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Apr 25 16:02:06 2018
@@ -180,14 +180,25 @@ ExprEngine::getRegionForConstructedObjec
     }
     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;
           }
         }
       }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=330882&r1=330881&r2=330882&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Apr 25 16:02:06 2018
@@ -699,6 +699,11 @@ ExprEngine::mayInlineCallKind(const Call
       // 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;

Modified: cfe/trunk/test/Analysis/lifetime-extension.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=330882&r1=330881&r2=330882&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/lifetime-extension.cpp (original)
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp Wed Apr 25 16:02:06 2018
@@ -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 {