r326240 - [analyzer] Disable constructor inlining when lifetime extending through a field.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Feb 27 12:14:06 2018
New Revision: 326240

URL: http://llvm.org/viewvc/llvm-project?rev=326240&view=rev
Log:
[analyzer] Disable constructor inlining when lifetime extending through a field.

Automatic destructors are missing in the CFG in situations like

  const int &x = C().x;

For now it's better to disable construction inlining, because inlining
constructors while doing nothing on destructors is very bad.

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

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=326240&r1=326239&r2=326240&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Tue Feb 27 12:14:06 2018
@@ -65,6 +65,10 @@ public:
     bool IsArrayCtorOrDtor = false;
     /// 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() {}
   };

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=326240&r1=326239&r2=326240&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Tue Feb 27 12:14:06 2018
@@ -168,6 +168,18 @@ ExprEngine::getRegionForConstructedObjec
       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;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=326240&r1=326239&r2=326240&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Tue Feb 27 12:14:06 2018
@@ -678,6 +678,11 @@ ExprEngine::mayInlineCallKind(const Call
       // 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;

Modified: cfe/trunk/test/Analysis/lifetime-extension.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=326240&r1=326239&r2=326240&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/lifetime-extension.cpp (original)
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp Tue Feb 27 12:14:06 2018
@@ -39,18 +39,10 @@ void f() {
   const int &y = A().j[1]; // no-crash
   const int &z = (A().j[1], A().j[0]); // no-crash
 
-  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
+  // 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}}
 }
 } // end namespace pr19539_crash_on_destroying_an_integer
 
@@ -144,12 +136,7 @@ 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);
-#ifdef TEMPORARIES
-  // expected-warning at -2{{The left operand of '==' is a garbage value}}
-#else
-  // expected-warning at -4{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
 }
 } // end namespace maintain_original_object_address_on_lifetime_extension