r333946 - [analyzer] Re-enable constructors when lifetime extension through fields occurs.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 4 13:18:37 PDT 2018


Author: dergachev
Date: Mon Jun  4 13:18:37 2018
New Revision: 333946

URL: http://llvm.org/viewvc/llvm-project?rev=333946&view=rev
Log:
[analyzer] Re-enable constructors when lifetime extension through fields occurs.

Temporary object constructor inlining was disabled in r326240 for code like

    const int &x = A().x;

because automatic destructor for the lifetime-extended object A() was not
working correctly in CFG.

CFG was fixed in r333941, so inlining can be re-enabled. CFG for lifetime
extension through aggregates still needs to be fixed.

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

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=333946&r1=333945&r2=333946&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Mon Jun  4 13:18:37 2018
@@ -106,11 +106,6 @@ public:
     bool IsTemporaryCtorOrDtor = false;
 
     /// This call is a constructor for a temporary that is lifetime-extended
-    /// by binding a smaller object within it to a reference, for example
-    /// '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;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=333946&r1=333945&r2=333946&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Mon Jun  4 13:18:37 2018
@@ -181,22 +181,13 @@ SVal ExprEngine::getLocationForConstruct
       const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC);
       if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) {
         if (const ValueDecl *VD = MTE->getExtendingDecl()) {
-          // 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 {
+          if (!VD->getType()->isReferenceType()) {
             // We're lifetime-extended by a surrounding aggregate.
-            // Automatic destructors aren't quite working in this case.
+            // Automatic destructors aren't quite working in this case
+            // on the CFG side. We should warn the caller about that.
+            // FIXME: Is there a better way to retrieve this information from
+            // the MaterializeTemporaryExpr?
             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=333946&r1=333945&r2=333946&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Jun  4 13:18:37 2018
@@ -696,12 +696,7 @@ ExprEngine::mayInlineCallKind(const Call
       if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion)
         return CIP_DisallowedOnce;
 
-      // If the temporary is lifetime-extended by binding a smaller object
-      // 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
+      // If the temporary is lifetime-extended by binding it to a reference-type
       // field within an aggregate, automatic destructors don't work properly.
       if (CallOpts.IsTemporaryLifetimeExtendedViaAggregate)
         return CIP_DisallowedOnce;

Modified: cfe/trunk/test/Analysis/lifetime-extension.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=333946&r1=333945&r2=333946&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/lifetime-extension.cpp (original)
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp Mon Jun  4 13:18:37 2018
@@ -46,10 +46,18 @@ void f() {
   const int &y = A().j[1]; // no-crash
   const int &z = (A().j[1], A().j[0]); // no-crash
 
-  // FIXME: All of these should be TRUE, but constructors aren't inlined.
-  clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(y == 3); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(z == 2); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x == 1);
+  clang_analyzer_eval(y == 3);
+  clang_analyzer_eval(z == 2);
+#ifdef TEMPORARIES
+  // expected-warning at -4{{TRUE}}
+  // expected-warning at -4{{TRUE}}
+  // expected-warning at -4{{TRUE}}
+#else
+  // expected-warning at -8{{UNKNOWN}}
+  // expected-warning at -8{{UNKNOWN}}
+  // expected-warning at -8{{UNKNOWN}}
+#endif
 }
 } // end namespace pr19539_crash_on_destroying_an_integer
 
@@ -144,8 +152,12 @@ void f5() {
   {
     const bool &x = C(true, &after, &before).x; // no-crash
   }
-  // FIXME: Should be TRUE. Should not warn about garbage value.
-  clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+  // expected-warning at -2{{TRUE}}
+#else
+  // expected-warning at -4{{UNKNOWN}}
+#endif
 }
 
 struct A { // A is an aggregate.




More information about the cfe-commits mailing list