[cfe-commits] r79694 - in /cfe/trunk: include/clang/Analysis/PathSensitive/AnalysisContext.h include/clang/Analysis/PathSensitive/GRState.h include/clang/Analysis/PathSensitive/Store.h lib/Analysis/AnalysisContext.cpp lib/Analysis/BasicStore.cpp lib/Analysis/CFRefCount.cpp lib/Analysis/MemRegion.cpp lib/Analysis/RegionStore.cpp

Ted Kremenek kremenek at apple.com
Fri Aug 21 16:25:55 PDT 2009


Author: kremenek
Date: Fri Aug 21 18:25:54 2009
New Revision: 79694

URL: http://llvm.org/viewvc/llvm-project?rev=79694&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Analysis/PathSensitive/AnalysisContext.h
    cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h
    cfe/trunk/include/clang/Analysis/PathSensitive/Store.h
    cfe/trunk/lib/Analysis/AnalysisContext.cpp
    cfe/trunk/lib/Analysis/BasicStore.cpp
    cfe/trunk/lib/Analysis/CFRefCount.cpp
    cfe/trunk/lib/Analysis/MemRegion.cpp
    cfe/trunk/lib/Analysis/RegionStore.cpp

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/AnalysisContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/AnalysisContext.h?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/AnalysisContext.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/AnalysisContext.h Fri Aug 21 18:25:54 2009
@@ -26,7 +26,8 @@
 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 @@
   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 @@
   LiveVariables *getLiveVariables() const { 
     return getAnalysisContext()->getLiveVariables();
   }
+  
+  const ImplicitParamDecl *getSelfDecl() const {
+    return Ctx->getSelfDecl();
+  }
 
   void Profile(llvm::FoldingSetNodeID &ID) {
     Profile(ID, Kind, Ctx, Parent);

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h Fri Aug 21 18:25:54 2009
@@ -208,8 +208,6 @@
 
   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 @@
   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);
 }

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/Store.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/Store.h?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/Store.h Fri Aug 21 18:25:54 2009
@@ -136,11 +136,6 @@
     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;

Modified: cfe/trunk/lib/Analysis/AnalysisContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisContext.cpp?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/AnalysisContext.cpp (original)
+++ cfe/trunk/lib/Analysis/AnalysisContext.cpp Fri Aug 21 18:25:54 2009
@@ -37,6 +37,13 @@
   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());

Modified: cfe/trunk/lib/Analysis/BasicStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BasicStore.cpp?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/BasicStore.cpp (original)
+++ cfe/trunk/lib/Analysis/BasicStore.cpp Fri Aug 21 18:25:54 2009
@@ -36,11 +36,9 @@
   
 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 @@
     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 @@
   /// 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 @@
   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 @@
 
   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 @@
       
   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 @@
   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 @@
       }
     }
     else
-      St = scanForIvars(*CI, SelfDecl, St);
+      St = scanForIvars(*CI, SelfDecl, SelfRegion, St);
   }
   
   return St;
@@ -511,17 +500,18 @@
       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 @@
   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);

Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Fri Aug 21 18:25:54 2009
@@ -3028,13 +3028,19 @@
     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);
+            }
+          } 
+        }
       }
     }
   }

Modified: cfe/trunk/lib/Analysis/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/MemRegion.cpp?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/MemRegion.cpp (original)
+++ cfe/trunk/lib/Analysis/MemRegion.cpp Fri Aug 21 18:25:54 2009
@@ -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 @@
   return false;
 }
 
-
 MemRegionManager* SubRegion::getMemRegionManager() const {
   const SubRegion* r = this;
   do {
@@ -253,8 +253,15 @@
   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);
 }
 

Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=79694&r1=79693&r2=79694&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
+++ cfe/trunk/lib/Analysis/RegionStore.cpp Fri Aug 21 18:25:54 2009
@@ -168,15 +168,11 @@
 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 @@
                  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.
   //===-------------------------------------------------------------------===//





More information about the cfe-commits mailing list