Remove 'SelfRegion' field from both BasicStoreManager and RegionStoreManager.

SelfRegion represented the object bound to 'self' (when analyzing Objective-C
methods) upon entry to a method. Having this region stored on the side ignores
the current stack frame that we might be analyzing (among other things), and is
a problem for interprocedural analysis.

For RegionStoreManager, the value for SelfRegion is just lazily created.

For BasicStoreManager, the value for SelfRegion is bound eagerly to 'self', but
no explicit tracking of SelfRegion on the side is made.

As part of this change, remove the restriction in BasicStoreManager that we only
track ivars for 'self'. This shouldn't actually change anything in terms of
precision, and simplifies the logic.


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

8 files changed, 58 insertions, 88 deletions

diff --git a/include/clang/Analysis/PathSensitive/AnalysisContext.h b/include/clang/Analysis/PathSensitive/AnalysisContext.h
index 74e175ef73..e716d17300 100644
--- a/include/clang/Analysis/PathSensitive/AnalysisContext.h
+++ b/include/clang/Analysis/PathSensitive/AnalysisContext.h
@@ -26,7 +26,8 @@ class Stmt;
 class CFG;
 class LiveVariables;
 class ParentMap;
-
+class ImplicitParamDecl;
+  
 /// AnalysisContext contains the context data for the function or method under
 /// analysis.
 class AnalysisContext {
@@ -48,6 +49,10 @@ public:
   CFG *getCFG();
   ParentMap &getParentMap();
   LiveVariables *getLiveVariables();
+  
+  /// Return the ImplicitParamDecl* associated with 'self' if this
+  /// AnalysisContext wraps an ObjCMethodDecl.  Returns NULL otherwise.
+  const ImplicitParamDecl *getSelfDecl() const;
 };
 
 class AnalysisContextManager {
@@ -86,6 +91,10 @@ public:
   LiveVariables *getLiveVariables() const { 
     return getAnalysisContext()->getLiveVariables();
   }
+  
+  const ImplicitParamDecl *getSelfDecl() const {
+    return Ctx->getSelfDecl();
+  }
 
   void Profile(llvm::FoldingSetNodeID &ID) {
     Profile(ID, Kind, Ctx, Parent);
diff --git a/include/clang/Analysis/PathSensitive/GRState.h b/include/clang/Analysis/PathSensitive/GRState.h
index 5ab4dceed7..f5faf51b1a 100644
--- a/include/clang/Analysis/PathSensitive/GRState.h
+++ b/include/clang/Analysis/PathSensitive/GRState.h
@@ -208,8 +208,6 @@ public:
 
   const VarRegion* getRegion(const VarDecl *D, const LocationContext *LC) const;
 
-  const MemRegion* getSelfRegion() const;
-
   //==---------------------------------------------------------------------==//
   // Binding and retrieving values to/from the environment and symbolic store.
   //==---------------------------------------------------------------------==//
@@ -606,10 +604,6 @@ inline const VarRegion* GRState::getRegion(const VarDecl *D,
   return Mgr->getRegionManager().getVarRegion(D, LC);
 }
 
-inline const MemRegion* GRState::getSelfRegion() const {
-  return Mgr->StoreMgr->getSelfRegion(getStore());
-}
-  
 inline const GRState *GRState::assume(SVal Cond, bool Assumption) const {
   return Mgr->ConstraintMgr->Assume(this, Cond, Assumption);
 }
diff --git a/include/clang/Analysis/PathSensitive/Store.h b/include/clang/Analysis/PathSensitive/Store.h
index df56963e48..ffc3e24196 100644
--- a/include/clang/Analysis/PathSensitive/Store.h
+++ b/include/clang/Analysis/PathSensitive/Store.h
@@ -136,11 +136,6 @@ public:
     return UnknownVal();
   }
   
-  /// getSelfRegion - Returns the region for the 'self' (Objective-C) or
-  ///  'this' object (C++).  When used when analyzing a normal function this
-  ///  method returns NULL.
-  virtual const MemRegion* getSelfRegion(Store store) = 0;
-
   virtual void RemoveDeadBindings(GRState &state, Stmt* Loc,
                                   SymbolReaper& SymReaper,
                       llvm::SmallVectorImpl<const MemRegion*>& RegionRoots) = 0;
diff --git a/lib/Analysis/AnalysisContext.cpp b/lib/Analysis/AnalysisContext.cpp
index 996d5c879a..324fdf2dce 100644
--- a/lib/Analysis/AnalysisContext.cpp
+++ b/lib/Analysis/AnalysisContext.cpp
@@ -37,6 +37,13 @@ Stmt *AnalysisContext::getBody() {
   llvm::llvm_unreachable("unknown code decl");
 }
 
+const ImplicitParamDecl *AnalysisContext::getSelfDecl() const {
+  if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))
+    return MD->getSelfDecl();
+  
+  return NULL;
+}
+
 CFG *AnalysisContext::getCFG() {
   if (!cfg) 
     cfg = CFG::buildCFG(getBody(), &D->getASTContext());
diff --git a/lib/Analysis/BasicStore.cpp b/lib/Analysis/BasicStore.cpp
index 65efb66148..50b90f5813 100644
--- a/lib/Analysis/BasicStore.cpp
+++ b/lib/Analysis/BasicStore.cpp
@@ -36,11 +36,9 @@ public:
   
 class VISIBILITY_HIDDEN BasicStoreManager : public StoreManager {
   BindingsTy::Factory VBFactory;
-  const MemRegion* SelfRegion;
-  
 public:
   BasicStoreManager(GRStateManager& mgr)
-    : StoreManager(mgr), VBFactory(mgr.getAllocator()), SelfRegion(0) {}
+    : StoreManager(mgr), VBFactory(mgr.getAllocator()) {}
   
   ~BasicStoreManager() {}
 
@@ -58,7 +56,8 @@ public:
     return state->makeWithStore(BindInternal(state->getStore(), L, V));
   }
 
-  Store scanForIvars(Stmt *B, const Decl* SelfDecl, Store St);
+  Store scanForIvars(Stmt *B, const Decl* SelfDecl,
+                     const MemRegion *SelfRegion, Store St);
   
   Store BindInternal(Store St, Loc loc, SVal V);  
   Store Remove(Store St, Loc loc);
@@ -88,11 +87,6 @@ public:
   /// ArrayToPointer - Used by GRExprEngine::VistCast to handle implicit
   ///  conversions between arrays and pointers.
   SVal ArrayToPointer(Loc Array) { return Array; }
-
-  /// getSelfRegion - Returns the region for the 'self' (Objective-C) or
-  ///  'this' object (C++).  When used when analyzing a normal function this
-  ///  method returns NULL.
-  const MemRegion* getSelfRegion(Store) { return SelfRegion; }
     
   /// RemoveDeadBindings - Scans a BasicStore of 'state' for dead values.
   ///  It updatees the GRState object in place with the values removed.
@@ -148,7 +142,8 @@ SVal BasicStoreManager::getLValueCompoundLiteral(const GRState *state,
   return ValMgr.makeLoc(MRMgr.getCompoundLiteralRegion(CL));
 }
 
-SVal BasicStoreManager::getLValueIvar(const GRState *state, const ObjCIvarDecl* D,
+SVal BasicStoreManager::getLValueIvar(const GRState *state,
+                                      const ObjCIvarDecl* D,
                                       SVal Base) {
   
   if (Base.isUnknownOrUndef())
@@ -158,9 +153,7 @@ SVal BasicStoreManager::getLValueIvar(const GRState *state, const ObjCIvarDecl*
 
   if (isa<loc::MemRegionVal>(BaseL)) {
     const MemRegion *BaseR = cast<loc::MemRegionVal>(BaseL).getRegion();
-
-    if (BaseR == SelfRegion)
-      return ValMgr.makeLoc(MRMgr.getObjCIvarRegion(D, BaseR));
+    return ValMgr.makeLoc(MRMgr.getObjCIvarRegion(D, BaseR));
   }
   
   return UnknownVal();
@@ -343,12 +336,7 @@ Store BasicStoreManager::BindInternal(Store store, Loc loc, SVal V) {
       
   if (!(isa<VarRegion>(R) || isa<ObjCIvarRegion>(R)))
     return store;
-      
-  // We only track bindings to self.ivar.
-  if (const ObjCIvarRegion *IVR = dyn_cast<ObjCIvarRegion>(R))
-    if (IVR->getSuperRegion() != SelfRegion)
-      return store;
-      
+
   if (nonloc::LocAsInteger *X = dyn_cast<nonloc::LocAsInteger>(&V)) {
     // Only convert 'V' to a location iff the underlying region type
     // is a location as well.
@@ -468,7 +456,8 @@ BasicStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc,
   state.setStore(store);
 }
 
-Store BasicStoreManager::scanForIvars(Stmt *B, const Decl* SelfDecl, Store St) {
+Store BasicStoreManager::scanForIvars(Stmt *B, const Decl* SelfDecl,
+                                      const MemRegion *SelfRegion, Store St) {
   for (Stmt::child_iterator CI=B->child_begin(), CE=B->child_end();
        CI != CE; ++CI) {
     
@@ -489,7 +478,7 @@ Store BasicStoreManager::scanForIvars(Stmt *B, const Decl* SelfDecl, Store St) {
       }
     }
     else
-      St = scanForIvars(*CI, SelfDecl, St);
+      St = scanForIvars(*CI, SelfDecl, SelfRegion, St);
   }
   
   return St;
@@ -511,17 +500,18 @@ Store BasicStoreManager::getInitialStore(const LocationContext *InitLoc) {
       const Decl& CD = *InitLoc->getDecl();      
       if (const ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(&CD)) {
         if (MD->getSelfDecl() == PD) {
-          // Create a region for "self".
-          assert (SelfRegion == 0);
-          SelfRegion = MRMgr.getObjCObjectRegion(MD->getClassInterface(),
-                                                 MRMgr.getHeapRegion());
+          // FIXME: Just use a symbolic region, and remove ObjCObjectRegion
+          // entirely.
+          const ObjCObjectRegion *SelfRegion =
+            MRMgr.getObjCObjectRegion(MD->getClassInterface(),
+                                      MRMgr.getHeapRegion());
           
           St = BindInternal(St, ValMgr.makeLoc(MRMgr.getVarRegion(PD, InitLoc)),
                             ValMgr.makeLoc(SelfRegion));
           
           // Scan the method for ivar references.  While this requires an
           // entire AST scan, the cost should not be high in practice.
-          St = scanForIvars(MD->getBody(), PD, St);
+          St = scanForIvars(MD->getBody(), PD, SelfRegion, St);
         }
       }
     }
@@ -641,11 +631,6 @@ const GRState *BasicStoreManager::InvalidateRegion(const GRState *state,
   if (!(isa<VarRegion>(R) || isa<ObjCIvarRegion>(R)))
       return state;
       
-  // We only track bindings to self.ivar.
-  if (const ObjCIvarRegion *IVR = dyn_cast<ObjCIvarRegion>(R))
-    if (IVR->getSuperRegion() != SelfRegion)
-      return state;
-
   QualType T = cast<TypedRegion>(R)->getValueType(R->getContext());
   SVal V = ValMgr.getConjuredSymbolVal(E, T, Count);
   return Bind(state, loc::MemRegionVal(R), V);
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 1b7a746f26..d1f293e40c 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -3028,13 +3028,19 @@ void CFRefCount::EvalObjCMessageExpr(ExplodedNodeSet& Dst,
     if (isa<ObjCMethodDecl>(&Eng.getGraph().getCodeDecl())) {      
       if (Expr* Receiver = ME->getReceiver()) {
         SVal X = St->getSValAsScalarOrLoc(Receiver);
-        if (loc::MemRegionVal* L = dyn_cast<loc::MemRegionVal>(&X))
-          if (L->getBaseRegion() == St->getSelfRegion()) {
-            // Update the summary to make the default argument effect
-            // 'StopTracking'.
-            Summ = Summaries.copySummary(Summ);
-            Summ->setDefaultArgEffect(StopTracking);
-          }
+        if (loc::MemRegionVal* L = dyn_cast<loc::MemRegionVal>(&X)) {          
+          // Get the region associated with 'self'.
+          const LocationContext *LC = Pred->getLocationContext();          
+          if (const ImplicitParamDecl *SelfDecl = LC->getSelfDecl()) {
+            SVal SelfVal = St->getSVal(St->getRegion(SelfDecl, LC));          
+            if (L->getBaseRegion() == SelfVal.getAsRegion()) {
+              // Update the summary to make the default argument effect
+              // 'StopTracking'.
+              Summ = Summaries.copySummary(Summ);
+              Summ->setDefaultArgEffect(StopTracking);
+            }
+          } 
+        }
       }
     }
   }
diff --git a/lib/Analysis/MemRegion.cpp b/lib/Analysis/MemRegion.cpp
index 2aacbf9065..ed53922655 100644
--- a/lib/Analysis/MemRegion.cpp
+++ b/lib/Analysis/MemRegion.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "clang/Analysis/PathSensitive/MemRegion.h"
 #include "clang/Analysis/PathSensitive/ValueManager.h"
+#include "clang/Analysis/PathSensitive/AnalysisContext.h"
 
 using namespace clang;
 
@@ -38,7 +39,6 @@ bool SubRegion::isSubRegionOf(const MemRegion* R) const {
   return false;
 }
 
-
 MemRegionManager* SubRegion::getMemRegionManager() const {
   const SubRegion* r = this;
   do {
@@ -253,8 +253,15 @@ StringRegion* MemRegionManager::getStringRegion(const StringLiteral* Str) {
   return getRegion<StringRegion>(Str);
 }
 
-VarRegion* MemRegionManager::getVarRegion(const VarDecl* D,
+VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
                                           const LocationContext *LC) {
+  
+  // FIXME: Once we implement scope handling, we will need to properly lookup
+  // 'D' to the proper LocationContext.  For now, just strip down to the
+  // StackFrame.
+  while (!isa<StackFrameContext>(LC))
+    LC = LC->getParent();
+  
   return getRegion<VarRegion>(D, LC);
 }
 
diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index 13768eca8c..53ef054c53 100644
--- a/lib/Analysis/RegionStore.cpp
+++ b/lib/Analysis/RegionStore.cpp
@@ -168,15 +168,11 @@ public:
 class VISIBILITY_HIDDEN RegionStoreManager : public StoreManager {
   const RegionStoreFeatures Features;
   RegionBindings::Factory RBFactory;
-
-  const MemRegion* SelfRegion;
-
 public:
   RegionStoreManager(GRStateManager& mgr, const RegionStoreFeatures &f) 
     : StoreManager(mgr),
       Features(f),
-      RBFactory(mgr.getAllocator()),
-    SelfRegion(0) {}
+      RBFactory(mgr.getAllocator()) {}
 
   virtual ~RegionStoreManager() {}
 
@@ -229,38 +225,9 @@ public:
                  NonLoc R, QualType resultTy);
 
   Store getInitialStore(const LocationContext *InitLoc) { 
-    RegionBindings B = RBFactory.GetEmptyMap();
-
-    // Eagerly bind 'self' to SelfRegion, because this is the only place we can
-    // get the ObjCMethodDecl.
-    typedef LiveVariables::AnalysisDataTy LVDataTy;
-    LVDataTy &D = InitLoc->getLiveVariables()->getAnalysisData();
-    for (LVDataTy::decl_iterator I=D.begin_decl(), E=D.end_decl(); I!=E; ++I){
-      const NamedDecl *ND = I->first;
-
-      if (const ImplicitParamDecl *PD = dyn_cast<ImplicitParamDecl>(ND)) {
-        const Decl &CD = *InitLoc->getDecl();
-        if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(&CD)) {
-          if (MD->getSelfDecl() == PD) {
-            SelfRegion = MRMgr.getObjCObjectRegion(MD->getClassInterface(),
-                                                   MRMgr.getHeapRegion());
-            B = RBFactory.Add(B, MRMgr.getVarRegion(PD, InitLoc),
-                              ValMgr.makeLoc(SelfRegion));
-          }
-        }
-      }
-    }
-
-    return B.getRoot();
-  }
-  
-  /// getSelfRegion - Returns the region for the 'self' (Objective-C) or
-  ///  'this' object (C++).  When used when analyzing a normal function this
-  ///  method returns NULL.
-  const MemRegion* getSelfRegion(Store) {
-    return SelfRegion;
+    return RBFactory.GetEmptyMap().getRoot();
   }
- 
+
   //===-------------------------------------------------------------------===//
   // Binding values to regions.
   //===-------------------------------------------------------------------===//