Remove lval::FieldOffset, lval::ArrayOffset. These will be replaced with regions.

Remove GRExprEngine::getLVal and RValues::MakeVal.
Enhance StoreManager "GetLValue" methods to dispatch for specific kinds of lvalue queries, as opposed to interogating the expression tree (GRExprEngine already does this).

Added FIXMEs.  In particular, we no longer "assume" that a base pointer in a field/array access is null (this logic was removed).  Perhaps we should do this when fetching the lvalue for fields and array elements?

llvm-svn: 57657

10 files changed, 70 insertions, 243 deletions

diff --git a/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h b/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h
index d1e25a5a664..5fc63224d85 100644
--- a/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -409,11 +409,7 @@ public:
   const GRState* SetRVal(const GRState* St, const Expr* Ex, RVal V) {
     return SetRVal(St, const_cast<Expr*>(Ex), V);
   }
-  
-  LVal getLVal(VarDecl* V) {
-    return getStateManager().getLVal(V);
-  }    
-  
+    
 protected:
  
   const GRState* SetBlkExprRVal(const GRState* St, Expr* Ex, RVal V) {
@@ -439,13 +435,6 @@ protected:
   RVal GetRVal(const GRState* St, LVal LV, QualType T = QualType()) {    
     return StateMgr.GetRVal(St, LV, T);
   }
-
-  // Get the lvalue of an expression.
-  // FIXME: Remove this method, and used specialized versions of GetLValue
-  // in GRStateManager.
-  RVal GetLValue(const GRState* St, const Expr* Ex) {
-    return StateMgr.GetLValue(St, Ex);
-  }
   
   inline NonLVal MakeConstantVal(uint64_t X, Expr* Ex) {
     return NonLVal::MakeVal(getBasicVals(), X, Ex->getType());
diff --git a/clang/include/clang/Analysis/PathSensitive/GRState.h b/clang/include/clang/Analysis/PathSensitive/GRState.h
index 2ae671cec33..841927bf3e5 100644
--- a/clang/include/clang/Analysis/PathSensitive/GRState.h
+++ b/clang/include/clang/Analysis/PathSensitive/GRState.h
@@ -341,22 +341,25 @@ public:
     return getRegionManager().getVarRegion(D);
   }
   
-  LVal getLVal(const VarDecl* D) {
-    return StoreMgr->getLVal(D);
+  // Get the lvalue for a variable reference.
+  RVal GetLValue(const GRState* St, const VarDecl* D) {
+    return StoreMgr->getLValueVar(St, D);
   }
-
-  // Get the lvalue of expression.
-  // FIXME: Remove this method, and implement specialized versions for
-  //  specific Decls.
-  RVal GetLValue(const GRState* St, const Expr* Ex) {
-    // Forward to store manager. The lvalue of an expression is determined by
-    // the store manager.
-    return StoreMgr->getLValue(St, Ex);
+  
+  // Get the lvalue for an ivar reference.
+  RVal GetLValue(const GRState* St, const ObjCIvarDecl* D, RVal Base) {
+    return StoreMgr->getLValueIvar(St, D, Base);
   }
   
-  RVal GetLValue(const GRState* St, ObjCIvarDecl* D, RVal Base) {
-    return StoreMgr->getLValue(St, D, Base);
+  // Get the lvalue for a field reference.
+  RVal GetLValue(const GRState* St, const FieldDecl* D, RVal Base) {
+    return StoreMgr->getLValueField(St, D, Base);
   }
+  
+  // Get the lvalue for an array index.
+  RVal GetLValue(const GRState* St, RVal Base, RVal Idx) {
+    return StoreMgr->getLValueElement(St, Base, Idx);
+  }  
 
   // Methods that query & manipulate the Environment.
   
@@ -587,6 +590,11 @@ public:
     return GRStateRef(Mgr->remove<T>(St, K, get_context<T>()), *Mgr);
   }
   
+  // Lvalue methods.
+  RVal GetLValue(const VarDecl* VD) {
+    return Mgr->GetLValue(St, VD);
+  }
+  
   // Pretty-printing.
   void print(std::ostream& Out, const char* nl = "\n",
              const char *sep = "") const;
diff --git a/clang/include/clang/Analysis/PathSensitive/RValues.h b/clang/include/clang/Analysis/PathSensitive/RValues.h
index 82e51193739..e41113d6c7f 100644
--- a/clang/include/clang/Analysis/PathSensitive/RValues.h
+++ b/clang/include/clang/Analysis/PathSensitive/RValues.h
@@ -95,8 +95,6 @@ public:
   symbol_iterator symbol_begin() const;
   symbol_iterator symbol_end() const;  
   
-  static RVal MakeVal(GRStateManager& SMgr, DeclRefExpr* E);
-  
   // Implement isa<T> support.
   static inline bool classof(const RVal*) { return true; }
 };
@@ -289,8 +287,7 @@ public:
 namespace lval {
   
 enum Kind { SymbolValKind, GotoLabelKind, MemRegionKind, FuncValKind,
-            ConcreteIntKind, StringLiteralValKind, FieldOffsetKind,
-            ArrayOffsetKind };
+            ConcreteIntKind, StringLiteralValKind };
 
 class SymbolVal : public LVal {
 public:
@@ -424,89 +421,6 @@ public:
   }
 };  
   
-class FieldOffset : public LVal {
-  FieldOffset(const std::pair<RVal, uintptr_t>& data)
-    : LVal(FieldOffsetKind, &data) {}
-  
-public:
-  
-  LVal getBase() const {
-    return reinterpret_cast<const std::pair<LVal,uintptr_t>*> (Data)->first;
-  }  
-  
-  const LVal& getPersistentBase() const {
-    return reinterpret_cast<const std::pair<LVal,uintptr_t>*> (Data)->first;
-  }    
-  
-    
-  FieldDecl* getFieldDecl() const {    
-    return (FieldDecl*)
-      reinterpret_cast<const std::pair<LVal,uintptr_t>*> (Data)->second;
-  }
-
-  // Implement isa<T> support.
-  static inline bool classof(const RVal* V) {
-    return V->getBaseKind() == LValKind &&
-           V->getSubKind() == FieldOffsetKind;
-  }
-  
-  static inline bool classof(const LVal* V) {
-    return V->getSubKind() == FieldOffsetKind;
-  }
-
-  static inline RVal Make(BasicValueFactory& Vals, RVal Base, FieldDecl* D) {
-    
-    if (Base.isUnknownOrUndef())
-      return Base;
-    
-    return FieldOffset(Vals.getPersistentRValWithData(cast<LVal>(Base),
-                                                      (uintptr_t) D));
-  }
-};
-  
-class ArrayOffset : public LVal {
-  ArrayOffset(const std::pair<RVal,RVal>& data) : LVal(ArrayOffsetKind,&data) {}  
-public:
-  
-  LVal getBase() const {
-    return reinterpret_cast<const std::pair<LVal,RVal>*> (Data)->first;
-  }  
-  
-  const LVal& getPersistentBase() const {
-    return reinterpret_cast<const std::pair<LVal,RVal>*> (Data)->first;
-  }   
-  
-  RVal getOffset() const {
-    return reinterpret_cast<const std::pair<LVal,RVal>*> (Data)->second;
-  }  
-  
-  const RVal& getPersistentOffset() const {
-    return reinterpret_cast<const std::pair<LVal,RVal>*> (Data)->second;
-  }   
-  
-
-  // Implement isa<T> support.
-  static inline bool classof(const RVal* V) {
-    return V->getBaseKind() == LValKind &&
-           V->getSubKind() == ArrayOffsetKind;
-  }
-  
-  static inline bool classof(const LVal* V) {
-    return V->getSubKind() == ArrayOffsetKind;
-  }
-  
-  static inline RVal Make(BasicValueFactory& Vals, RVal Base, RVal Offset) {
-    
-    if (Base.isUnknownOrUndef())
-      return Base;
-    
-    if (Offset.isUndef())
-      return Offset;
-    
-    return ArrayOffset(Vals.getPersistentRValPair(cast<LVal>(Base), Offset));
-  }
-};
-  
 } // end clang::lval namespace
 } // end clang namespace  
 
diff --git a/clang/include/clang/Analysis/PathSensitive/Store.h b/clang/include/clang/Analysis/PathSensitive/Store.h
index c61b4820980..60c8f528553 100644
--- a/clang/include/clang/Analysis/PathSensitive/Store.h
+++ b/clang/include/clang/Analysis/PathSensitive/Store.h
@@ -45,14 +45,16 @@ public:
   virtual Store Remove(Store St, LVal LV) = 0;
   virtual Store getInitialStore() = 0;
   virtual MemRegionManager& getRegionManager() = 0;
-  virtual LVal getLVal(const VarDecl* VD) = 0;  
 
-  // Get the lvalue of an expression.
-  // FIXME: Remove this method, and implement specialized versions for
-  //  specific Decls.
-  virtual RVal getLValue(const GRState* St, const Expr* Ex) = 0;
+  virtual RVal getLValueVar(const GRState* St, const VarDecl* VD) = 0;  
   
-  virtual RVal getLValue(const GRState* St, const ObjCIvarDecl* D, RVal Base)=0;
+  virtual RVal getLValueIvar(const GRState* St, const ObjCIvarDecl* D,
+                             RVal Base)=0;
+  
+  virtual RVal getLValueField(const GRState* St, const FieldDecl* D,
+                              RVal Base) = 0;
+  
+  virtual RVal getLValueElement(const GRState* St, RVal Base, RVal Offset) = 0;
   
 
   virtual Store
diff --git a/clang/lib/Analysis/BasicConstraintManager.cpp b/clang/lib/Analysis/BasicConstraintManager.cpp
index 66bf082b136..8d391bbb58c 100644
--- a/clang/lib/Analysis/BasicConstraintManager.cpp
+++ b/clang/lib/Analysis/BasicConstraintManager.cpp
@@ -136,14 +136,6 @@ const GRState* BasicConstraintManager::AssumeAux(const GRState* St, LVal Cond,
     isFeasible = Assumption;
     return St;
 
-  case lval::FieldOffsetKind:
-    return AssumeAux(St, cast<lval::FieldOffset>(Cond).getBase(),
-                     Assumption, isFeasible);
-
-  case lval::ArrayOffsetKind:
-    return AssumeAux(St, cast<lval::ArrayOffset>(Cond).getBase(),
-                     Assumption, isFeasible);
-
   case lval::ConcreteIntKind: {
     bool b = cast<lval::ConcreteInt>(Cond).getValue() != 0;
     isFeasible = b ? Assumption : !Assumption;
diff --git a/clang/lib/Analysis/BasicStore.cpp b/clang/lib/Analysis/BasicStore.cpp
index 15a20c843a0..000ea1bef2d 100644
--- a/clang/lib/Analysis/BasicStore.cpp
+++ b/clang/lib/Analysis/BasicStore.cpp
@@ -46,9 +46,11 @@ public:
   virtual LVal getLVal(const VarDecl* VD) {
     return lval::MemRegionVal(MRMgr.getVarRegion(VD));
   }
-
-  virtual RVal getLValue(const GRState* St, const Expr* Ex);
-  virtual RVal getLValue(const GRState* St, const ObjCIvarDecl* D, RVal Base);
+  
+  RVal getLValueVar(const GRState* St, const VarDecl* VD);
+  RVal getLValueIvar(const GRState* St, const ObjCIvarDecl* D, RVal Base);  
+  RVal getLValueField(const GRState* St, const FieldDecl* D, RVal Base);  
+  RVal getLValueElement(const GRState* St, RVal Base, RVal Offset);
   
   virtual Store
   RemoveDeadBindings(Store store, Stmt* Loc, const LiveVariables& Live,
@@ -76,34 +78,26 @@ public:
 StoreManager* clang::CreateBasicStoreManager(GRStateManager& StMgr) {
   return new BasicStoreManager(StMgr);
 }
+RVal BasicStoreManager::getLValueVar(const GRState* St, const VarDecl* VD) {
+  QualType T = VD->getType();
+  assert(!T->isArrayType() && "Array and struct variable have no lvalue.");
+  return lval::MemRegionVal(MRMgr.getVarRegion(VD));
+}
+  
+RVal BasicStoreManager::getLValueIvar(const GRState* St, const ObjCIvarDecl* D,
+                                      RVal Base) {
+  return UnknownVal();
+}
+  
+  
+RVal BasicStoreManager::getLValueField(const GRState* St, const FieldDecl* D,
+                                       RVal Base) {
+  return UnknownVal();
+}
 
-// FIXME: replace ArrayOffset and FieldOffset with some region value.
-RVal BasicStoreManager::getLValue(const GRState* St, const Expr* Ex) {
-  if (const DeclRefExpr* DRE = dyn_cast<DeclRefExpr>(Ex)) {
-    const VarDecl* VD = cast<VarDecl>(DRE->getDecl());
-    QualType T = VD->getType();
-
-    // Array and struct variable have no lvalue.
-    assert(!T->isArrayType());
-
-    return lval::MemRegionVal(MRMgr.getVarRegion(VD));
-
-  } else if (const ArraySubscriptExpr* A = dyn_cast<ArraySubscriptExpr>(Ex)) {
-    const Expr* Base = A->getBase()->IgnoreParens();
-    const Expr* Idx  = A->getIdx()->IgnoreParens();
-    RVal BaseV = StateMgr.GetRVal(St, Base);
-    RVal IdxV = StateMgr.GetRVal(St, Idx);
-    return lval::ArrayOffset::Make(StateMgr.getBasicVals(), BaseV, IdxV);
-
-  } else if (const MemberExpr* M = dyn_cast<MemberExpr>(Ex)) {
-    Expr* Base = M->getBase()->IgnoreParens();
-    RVal BaseV = StateMgr.GetRVal(St, Base);
-    return lval::FieldOffset::Make(StateMgr.getBasicVals(), BaseV, 
-                                   M->getMemberDecl());
-  } else {
-    Ex->dump();
-    assert(0);
-  }
+RVal BasicStoreManager::getLValueElement(const GRState* St, RVal Base,
+                                         RVal Offset) {
+  return UnknownVal();
 }
 
 RVal BasicStoreManager::GetRVal(Store St, LVal LV, QualType T) {
@@ -134,12 +128,7 @@ RVal BasicStoreManager::GetRVal(Store St, LVal LV, QualType T) {
       // Some clients may call GetRVal with such an option simply because
       // they are doing a quick scan through their LVals (potentially to
       // invalidate their bindings).  Just return Undefined.
-      return UndefinedVal();
-      
-    case lval::ArrayOffsetKind:
-    case lval::FieldOffsetKind:
-      return UnknownVal();
-      
+      return UndefinedVal();            
     case lval::FuncValKind:
       return LV;
       
@@ -154,11 +143,6 @@ RVal BasicStoreManager::GetRVal(Store St, LVal LV, QualType T) {
   
   return UnknownVal();
 }
-
-RVal BasicStoreManager::getLValue(const GRState* St, const ObjCIvarDecl* D,
-                                  RVal Base) {
-  return UnknownVal();  
-}
   
 Store BasicStoreManager::SetRVal(Store store, LVal LV, RVal V) {    
   switch (LV.getSubKind()) {      
diff --git a/clang/lib/Analysis/CheckNSError.cpp b/clang/lib/Analysis/CheckNSError.cpp
index 03c9af3a729..6e52dd952d9 100644
--- a/clang/lib/Analysis/CheckNSError.cpp
+++ b/clang/lib/Analysis/CheckNSError.cpp
@@ -216,7 +216,7 @@ void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState,
                                    GRExprEngine& Eng, GRBugReporter& BR,
                                    bool isNSErrorWarning) {
   
-  RVal ParamRVal = rootState.GetRVal(Eng.getLVal(Param));
+  RVal ParamRVal = rootState.GetLValue(Param);
 
   // FIXME: For now assume that ParamRVal is symbolic.  We need to generalize
   // this later.
diff --git a/clang/lib/Analysis/GRExprEngine.cpp b/clang/lib/Analysis/GRExprEngine.cpp
index f974dfc2c48..2bc46d59902 100644
--- a/clang/lib/Analysis/GRExprEngine.cpp
+++ b/clang/lib/Analysis/GRExprEngine.cpp
@@ -817,7 +817,8 @@ void GRExprEngine::VisitDeclRefExpr(DeclRefExpr* Ex, NodeTy* Pred, NodeSet& Dst,
       return;
     }
 
-    RVal V = GetLValue(St, Ex);
+    RVal V = StateMgr.GetLValue(St, VD);
+    
     if (asLValue)
       MakeNode(Dst, Ex, Pred, SetRVal(St, Ex, V));
     else
@@ -850,22 +851,16 @@ void GRExprEngine::VisitArraySubscriptExpr(ArraySubscriptExpr* A, NodeTy* Pred,
   
   Expr* Base = A->getBase()->IgnoreParens();
   Expr* Idx  = A->getIdx()->IgnoreParens();
-  
   NodeSet Tmp;
-
-  // Get Base's rvalue, which should be an LocVal.
-  Visit(Base, Pred, Tmp);
+  Visit(Base, Pred, Tmp);   // Get Base's rvalue, which should be an LocVal.
   
-  for (NodeSet::iterator I1=Tmp.begin(), E1=Tmp.end(); I1!=E1; ++I1) {
-    
-    // Evaluate the index.
+  for (NodeSet::iterator I1=Tmp.begin(), E1=Tmp.end(); I1!=E1; ++I1) {    
     NodeSet Tmp2;
-    Visit(Idx, *I1, Tmp2);
+    Visit(Idx, *I1, Tmp2);     // Evaluate the index.
       
     for (NodeSet::iterator I2=Tmp2.begin(), E2=Tmp2.end(); I2!=E2; ++I2) {
-
       const GRState* St = GetState(*I2);
-      RVal V = GetLValue(St, A);
+      RVal V = StateMgr.GetLValue(St, GetRVal(St, Base), GetRVal(St, Idx));
 
       if (asLValue)
         MakeNode(Dst, A, *I2, SetRVal(St, A, V));
@@ -880,15 +875,16 @@ void GRExprEngine::VisitMemberExpr(MemberExpr* M, NodeTy* Pred,
                                    NodeSet& Dst, bool asLValue) {
   
   Expr* Base = M->getBase()->IgnoreParens();
-
   NodeSet Tmp;
-
-  // Get Base expr's rvalue.
   Visit(Base, Pred, Tmp);
 
   for (NodeSet::iterator I = Tmp.begin(), E = Tmp.end(); I != E; ++I) {
     const GRState* St = GetState(*I);
-    RVal L = GetLValue(St, M);
+    // FIXME: Should we insert some assumption logic in here to determine
+    // if "Base" is a valid piece of memory?  Before we put this assumption
+    // later when using FieldOffset lvals (which we no longer have).    
+    RVal L = StateMgr.GetLValue(St, M->getMemberDecl(), GetRVal(St, Base));
+
     if (asLValue)
       MakeNode(Dst, M, *I, SetRVal(St, M, L));
     else
diff --git a/clang/lib/Analysis/GRSimpleVals.cpp b/clang/lib/Analysis/GRSimpleVals.cpp
index 44703163e06..07ec3e8bffc 100644
--- a/clang/lib/Analysis/GRSimpleVals.cpp
+++ b/clang/lib/Analysis/GRSimpleVals.cpp
@@ -282,11 +282,6 @@ RVal GRSimpleVals::EvalEQ(GRExprEngine& Eng, LVal L, LVal R) {
       return UnknownVal();      
     }
       
-      // FIXME: Different offsets can map to the same memory cell.
-    case lval::ArrayOffsetKind:
-    case lval::FieldOffsetKind:
-      // Fall-through.
-      
     case lval::MemRegionKind:
     case lval::FuncValKind:
     case lval::GotoLabelKind:
@@ -346,11 +341,6 @@ RVal GRSimpleVals::EvalNE(GRExprEngine& Eng, LVal L, LVal R) {
       break;
     }
       
-      // FIXME: Different offsets can map to the same memory cell.
-    case lval::ArrayOffsetKind:
-    case lval::FieldOffsetKind:
-      // Fall-through.
-      
     case lval::MemRegionKind:
     case lval::FuncValKind:
     case lval::GotoLabelKind:
diff --git a/clang/lib/Analysis/RValues.cpp b/clang/lib/Analysis/RValues.cpp
index 2b573681e82..551945ce26e 100644
--- a/clang/lib/Analysis/RValues.cpp
+++ b/clang/lib/Analysis/RValues.cpp
@@ -43,10 +43,9 @@ RVal::symbol_iterator RVal::symbol_begin() const {
     const nonlval::LValAsInteger& V = cast<nonlval::LValAsInteger>(*this);
     return  V.getPersistentLVal().symbol_begin();
   }
-  else if (isa<lval::FieldOffset>(this)) {
-    const lval::FieldOffset& V = cast<lval::FieldOffset>(*this);
-    return V.getPersistentBase().symbol_begin();
-  }
+  
+  // FIXME: We need to iterate over the symbols of regions.
+
   return NULL;
 }
 
@@ -267,37 +266,6 @@ LVal LVal::MakeVal(StringLiteral* S) {
 }
 
 //===----------------------------------------------------------------------===//
-// Utility methods for constructing RVals (both NonLVals and LVals).
-//===----------------------------------------------------------------------===//
-
-// Remove this method?
-RVal RVal::MakeVal(GRStateManager& SMgr, DeclRefExpr* E) {
-  
-  ValueDecl* D = cast<DeclRefExpr>(E)->getDecl();
-  
-  if (VarDecl* VD = dyn_cast<VarDecl>(D)) {
-    return SMgr.getLVal(VD);
-  }
-  else if (EnumConstantDecl* ED = dyn_cast<EnumConstantDecl>(D)) {
-    
-    // FIXME: Do we need to cache a copy of this enum, since it
-    // already has persistent storage?  We do this because we
-    // are comparing states using pointer equality.  Perhaps there is
-    // a better way, since APInts are fairly lightweight.
-    BasicValueFactory& BasicVals = SMgr.getBasicVals();
-    return nonlval::ConcreteInt(BasicVals.getValue(ED->getInitVal()));          
-  }
-  else if (FunctionDecl* FD = dyn_cast<FunctionDecl>(D)) {
-    return lval::FuncVal(FD);
-  }
-  
-  assert (false &&
-          "ValueDecl support for this ValueDecl not implemented.");
-  
-  return UnknownVal();
-}
-
-//===----------------------------------------------------------------------===//
 // Pretty-Printing.
 //===----------------------------------------------------------------------===//
 
@@ -424,22 +392,6 @@ void LVal::print(std::ostream& Out) const {
           << "\"";
       break;
       
-    case lval::FieldOffsetKind: {
-      const lval::FieldOffset& C = *cast<lval::FieldOffset>(this);
-      C.getBase().print(Out);
-      Out << "." << C.getFieldDecl()->getName() << " (field LVal)";
-      break;
-    }
-      
-    case lval::ArrayOffsetKind: {
-      const lval::ArrayOffset& C = *cast<lval::ArrayOffset>(this);
-      C.getBase().print(Out);
-      Out << "[";
-      C.getOffset().print(Out);
-      Out << "] (lval array entry)";
-      break;
-    }
-      
     default:
       assert (false && "Pretty-printing not implemented for this LVal.");
       break;