[PATCH] D44239: [analyzer] Re-enable constructor inlining when lifetime extension through fields occurs.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
NoQ added a dependency: D44238: [CFG] Fix automatic destructors when a member is bound to a reference..

In code like

  const int &x = C().x;

destructors were missing in the CFG, so it was unsafe to inline the constructors, so constructor inlining was disabled in https://reviews.llvm.org/D43689. But i think i found a way to fix the CFG in https://reviews.llvm.org/D44238, so it should be safe to re-enable it now.


Repository:
  rC Clang

https://reviews.llvm.org/D44239

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


Index: test/Analysis/lifetime-extension.cpp
===================================================================
--- test/Analysis/lifetime-extension.cpp
+++ test/Analysis/lifetime-extension.cpp
@@ -42,10 +42,18 @@
   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
 
@@ -140,8 +148,12 @@
   {
     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
 }
 } // end namespace maintain_original_object_address_on_lifetime_extension
 
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -694,11 +694,6 @@
       // the fake temporary target.
       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;
     }
 
     break;
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -179,18 +179,6 @@
       break;
     }
     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;
-          }
-        }
-      }
       // TODO: Support temporaries lifetime-extended via static references.
       // They'd need a getCXXStaticTempObjectRegion().
       CallOpts.IsTemporaryCtorOrDtor = true;
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -105,11 +105,6 @@
     /// This call is a constructor or a destructor of a temporary value.
     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;
-
     EvalCallOptions() {}
   };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44239.137521.patch
Type: text/x-patch
Size: 3933 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180308/dd68d888/attachment.bin>