[PATCH] D27202: [analyzer] Do not conjure a symbol for return value of a conservatively evaluated function

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin.
NoQ added subscribers: cfe-commits, baloghadamsoftware.

Instead, return a `nonloc::LazyCompoundVal` of a temporary region for the call expression, with a trivial store in which the temporary region is invalidated.

Returning a conjured record-type symbol is painful because the return value of the function may suddenly change during materialization of a temporary - it will be hotfixed into the same lazy compound value anyway in some cases, eg. in `createTemporaryRegionIfNeeded` when handling a `MaterializeTemporaryExpr`. After this patch, we should be receiving the same temporary region during materialization, instead of constructing a new region, and materialization of a conjured structure starts doing a lot less things.

I'm including a change in the yet-to-land iterator checker (https://reviews.llvm.org/D25660), which should or should not be committed depending on the order of patches landing. This change demonstrates how the problem explained above causes checkers to hack around.


https://reviews.llvm.org/D27202

Files:
  lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/explain-svals.cpp


Index: test/Analysis/explain-svals.cpp
===================================================================
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -94,5 +94,5 @@
 
 void test_6() {
   clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of temporary object constructed at statement 'conjure_S\(\)'$}}}}
-  clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'struct S' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}}
+  clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -553,7 +553,28 @@
   QualType ResultTy = Call.getResultType();
   SValBuilder &SVB = getSValBuilder();
   unsigned Count = currBldrCtx->blockCount();
-  SVal R = SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
+  SVal R;
+
+  if (NonLoc::isCompoundType(ResultTy)) {
+    // Return a (lazy) compound value for compound types.
+    MemRegionManager &MemMgr = SVB.getRegionManager();
+    StoreManager &StoreMgr = SVB.getStateManager().getStoreManager();
+
+    const CXXTempObjectRegion *TR = MemMgr.getCXXTempObjectRegion(E, LCtx);
+
+    StoreRef EmptyStore = StoreMgr.getInitialStore(LCtx);
+
+    InvalidatedSymbols IS;
+    RegionAndSymbolInvalidationTraits ITraits;
+    ITraits.setTrait(TR, RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
+    StoreRef ConjuredStore = StoreMgr.invalidateRegions(
+        EmptyStore.getStore(), loc::MemRegionVal(TR), E, Count, LCtx, nullptr,
+        IS, ITraits, nullptr, nullptr);
+
+    R = SVB.makeLazyCompoundVal(ConjuredStore, TR);
+  } else {
+    R = SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
+  }
   return State->BindExpr(E, LCtx, R);
 }
 
Index: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
@@ -68,10 +68,9 @@
 
 class IteratorPastEndChecker
     : public Checker<
-          check::PreCall, check::PostCall, check::PreStmt<CXXOperatorCallExpr>,
-          check::PostStmt<CXXConstructExpr>, check::PostStmt<DeclStmt>,
-          check::PostStmt<MaterializeTemporaryExpr>, check::BeginFunction,
-          check::DeadSymbols, eval::Assume, eval::Call> {
+          check::PreCall, check::PostCall, check::PostStmt<CXXConstructExpr>,
+          check::PostStmt<DeclStmt>, check::PostStmt<MaterializeTemporaryExpr>,
+          check::BeginFunction, check::DeadSymbols, eval::Assume, eval::Call> {
   mutable IdentifierInfo *II_std = nullptr, *II_find = nullptr,
                          *II_find_end = nullptr, *II_find_first_of = nullptr,
                          *II_find_if = nullptr, *II_find_if_not = nullptr,
@@ -105,7 +104,6 @@
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-  void checkPreStmt(const CXXOperatorCallExpr *COCE, CheckerContext &C) const;
   void checkBeginFunction(CheckerContext &C) const;
   void checkPostStmt(const CXXConstructExpr *CCE, CheckerContext &C) const;
   void checkPostStmt(const DeclStmt *DS, CheckerContext &C) const;
@@ -216,27 +214,6 @@
   }
 }
 
-void IteratorPastEndChecker::checkPreStmt(const CXXOperatorCallExpr *COCE,
-                                          CheckerContext &C) const {
-  const auto *ThisExpr = COCE->getArg(0);
-
-  auto State = C.getState();
-  const auto *LCtx = C.getPredecessor()->getLocationContext();
-
-  const auto CurrentThis = State->getSVal(ThisExpr, LCtx);
-  if (const auto *Reg = CurrentThis.getAsRegion()) {
-    if (!Reg->getAs<CXXTempObjectRegion>())
-      return;
-    const auto OldState = C.getPredecessor()->getFirstPred()->getState();
-    const auto OldThis = OldState->getSVal(ThisExpr, LCtx);
-    const auto *Pos = getIteratorPosition(OldState, OldThis);
-    if (!Pos)
-      return;
-    State = setIteratorPosition(State, CurrentThis, *Pos);
-    C.addTransition(State);
-  }
-}
-
 void IteratorPastEndChecker::checkBeginFunction(CheckerContext &C) const {
   auto State = C.getState();
   const auto *LCtx = C.getLocationContext();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27202.79541.patch
Type: text/x-patch
Size: 4696 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161129/f35a1ce9/attachment-0001.bin>