[analyzer] Make sure base-region and its sub-regions are either all alive or all dead.

SymbolReaper now realizes that our liveness analysis isn't sharp enough
to discriminate between liveness of, say, variables and their fields.
Surprisingly, this didn't quite work before: having a variable live only
through Environment (eg., calling a C++ method on a local variable
as the last action ever performed on that variable) would not keep the
region value symbol of a field of that variable alive.

It would have been broken in the opposite direction as well, but both
Environment and RegionStore use the scanReachableSymbols mechanism for finding
live symbols regions within their values, and due to that they accidentally
end up marking the whole chain of super-regions as live when at least one
sub-region is known to be live.

It is now a direct responsibility of SymbolReaper to maintain this invariant,
and a unit test was added in order to make sure it stays that way.

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

rdar://problem/46914108


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351499 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 10 insertions, 4 deletions

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 151eef56fe..cbc022f49a 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -198,7 +198,9 @@ ExprEngine::ExprEngine(cross_tu::CrossTranslationUnitContext &CTU,
                mgr.getConstraintManagerCreator(), G.getAllocator(),
                this),
       SymMgr(StateMgr.getSymbolManager()),
-      svalBuilder(StateMgr.getSValBuilder()), ObjCNoRet(mgr.getASTContext()),
+      MRMgr(StateMgr.getRegionManager()),
+      svalBuilder(StateMgr.getSValBuilder()),
+      ObjCNoRet(mgr.getASTContext()),
       BR(mgr, *this),
       VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
   unsigned TrimInterval = mgr.options.GraphTrimInterval;
diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp
index 66273f099a..c60c3d0f37 100644
--- a/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -405,7 +405,7 @@ void SymbolReaper::markLive(SymbolRef sym) {
 }
 
 void SymbolReaper::markLive(const MemRegion *region) {
-  RegionRoots.insert(region);
+  RegionRoots.insert(region->getBaseRegion());
   markElementIndicesLive(region);
 }
 
@@ -426,11 +426,15 @@ void SymbolReaper::markInUse(SymbolRef sym) {
 }
 
 bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
+  // TODO: For now, liveness of a memory region is equivalent to liveness of its
+  // base region. In fact we can do a bit better: say, if a particular FieldDecl
+  // is not used later in the path, we can diagnose a leak of a value within
+  // that field earlier than, say, the variable that contains the field dies.
+  MR = MR->getBaseRegion();
+
   if (RegionRoots.count(MR))
     return true;
 
-  MR = MR->getBaseRegion();
-
   if (const auto *SR = dyn_cast<SymbolicRegion>(MR))
     return isLive(SR->getSymbol());