Major rewrite/refactoring of static analysis engine. We now use

EvalStore/EvalLoad to handle all loads/stores from symbolic memory, allowing us
to do checks for null dereferences, etc., at any arbitrary load/store (these
were missed checks before). This also resulted in some major cleanups, some
conceptual, and others just in the structure of the code.

This temporarily introduces a regression in the test suite (null-deref-ps.c)
before I add a new LVal type for structure fields.


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

7 files changed, 601 insertions, 670 deletions

diff --git a/include/clang/Analysis/PathSensitive/GRCoreEngine.h b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
index d4074af5a9..bef2e2c0b6 100644
--- a/include/clang/Analysis/PathSensitive/GRCoreEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
@@ -151,12 +151,14 @@ public:
   }  
   
   ExplodedNodeImpl* generateNodeImpl(Stmt* S, void* State,
-                                     ExplodedNodeImpl* Pred);
+                                     ExplodedNodeImpl* Pred,
+                                     bool isLoad = false);
 
-  inline ExplodedNodeImpl* generateNodeImpl(Stmt* S, void* State) {
+  inline ExplodedNodeImpl* generateNodeImpl(Stmt* S, void* State,
+                                            bool isLoad = false) {
     ExplodedNodeImpl* N = getLastNode();
     assert (N && "Predecessor of new node is infeasible.");
-    return generateNodeImpl(S, State, N);
+    return generateNodeImpl(S, State, N, isLoad);
   }
   
   Stmt* getStmt() const { return B[Idx]; }
@@ -201,14 +203,14 @@ public:
     return static_cast<NodeTy*>(NB.getLastNode());
   }
   
-  NodeTy* generateNode(Stmt* S, StateTy* St, NodeTy* Pred) {
+  NodeTy* generateNode(Stmt* S, StateTy* St, NodeTy* Pred, bool isLoad = false){
     HasGeneratedNode = true;
-    return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, Pred));
+    return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, Pred, isLoad));
   }
   
-  NodeTy* generateNode(Stmt* S, StateTy* St) {
+  NodeTy* generateNode(Stmt* S, StateTy* St, bool isLoad = false) {
     HasGeneratedNode = true;
-    return static_cast<NodeTy*>(NB.generateNodeImpl(S, St));    
+    return static_cast<NodeTy*>(NB.generateNodeImpl(S, St, isLoad));    
   }
   
   GRBlockCounter getBlockCounter() const {
diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index de8807a201..7005263033 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -425,10 +425,6 @@ protected:
     return StateMgr.GetRVal(St, LV, T);
   }
   
-  RVal GetLVal(ValueState* St, Expr* Ex) {
-    return StateMgr.GetLVal(St, Ex);
-  }
-  
   inline NonLVal MakeConstantVal(uint64_t X, Expr* Ex) {
     return NonLVal::MakeVal(BasicVals, X, Ex->getType());
   }
@@ -474,11 +470,7 @@ protected:
     assert (Builder && "GRStmtNodeBuilder not present.");
     return Builder->MakeNode(Dst, S, Pred, St);
   }
-  
-  /// HandleUndefinedStore - Create the necessary sink node to represent
-  ///  a store to an "undefined" LVal.
-  void HandleUndefinedStore(Stmt* S, NodeTy* Pred);
-  
+    
   /// Visit - Transfer function logic for all statements.  Dispatches to
   ///  other functions that handle specific kinds of statements.
   void Visit(Stmt* S, NodeTy* Pred, NodeSet& Dst);
@@ -520,7 +512,8 @@ protected:
   void VisitCast(Expr* CastE, Expr* Ex, NodeTy* Pred, NodeSet& Dst);  
   
   /// VisitDeclRefExpr - Transfer function logic for DeclRefExprs.
-  void VisitDeclRefExpr(DeclRefExpr* DR, NodeTy* Pred, NodeSet& Dst); 
+  void VisitDeclRefExpr(DeclRefExpr* DR, NodeTy* Pred, NodeSet& Dst,
+                        bool asLval); 
   
   /// VisitDeclStmt - Transfer function logic for DeclStmts.
   void VisitDeclStmt(DeclStmt* DS, NodeTy* Pred, NodeSet& Dst); 
@@ -528,12 +521,6 @@ protected:
   void VisitDeclStmtAux(DeclStmt* DS, ScopedDecl* D,
                         NodeTy* Pred, NodeSet& Dst);
   
-  void VisitDeref(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst,
-                  bool GetLVal = false);
-  
-  void VisitDeref(Expr* Ex, RVal V, ValueState* St, NodeTy* Pred, NodeSet& Dst,
-                  bool GetLVal);
-  
   /// VisitGuardedExpr - Transfer function logic for ?, __builtin_choose
   void VisitGuardedExpr(Expr* Ex, Expr* L, Expr* R, NodeTy* Pred, NodeSet& Dst);
   
@@ -543,10 +530,6 @@ protected:
   /// VisitMemberExpr - Transfer function for member expressions.
   void VisitMemberExpr(MemberExpr* M, NodeTy* Pred, NodeSet& Dst, bool asLVal);
   
-  void VisitMemberExprField(MemberExpr* M, Expr* Base, NodeTy* Pred,
-                            NodeSet& Dst, bool asLVal);
-    
-  
   /// VisitObjCMessageExpr - Transfer function for ObjC message expressions.
   void VisitObjCMessageExpr(ObjCMessageExpr* ME, NodeTy* Pred, NodeSet& Dst);
   
@@ -564,15 +547,12 @@ protected:
   /// VisitSizeOfAlignOfTypeExpr - Transfer function for sizeof(type).
   void VisitSizeOfAlignOfTypeExpr(SizeOfAlignOfTypeExpr* Ex, NodeTy* Pred,
                                   NodeSet& Dst);
-  
-  // VisitSizeOfExpr - Transfer function for sizeof(expr).
-  void VisitSizeOfExpr(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst);
-  
+    
   /// VisitUnaryOperator - Transfer function logic for unary operators.
-  void VisitUnaryOperator(UnaryOperator* B, NodeTy* Pred, NodeSet& Dst);
-  
-  
-  
+  void VisitUnaryOperator(UnaryOperator* B, NodeTy* Pred, NodeSet& Dst,
+                          bool asLVal);
+ 
+  bool CheckDivideZero(Expr* Ex, ValueState* St, NodeTy* Pred, RVal Denom);  
   
   RVal EvalCast(RVal X, QualType CastT) {
     if (X.isUnknownOrUndef())
@@ -584,8 +564,6 @@ protected:
       return TF->EvalCast(*this, cast<NonLVal>(X), CastT);
   }
   
-
-  
   RVal EvalMinus(UnaryOperator* U, RVal X) {
     return X.isValid() ? TF->EvalMinus(*this, U, cast<NonLVal>(X)) : X;
   }
@@ -603,27 +581,27 @@ protected:
   }
   
   RVal EvalBinOp(BinaryOperator::Opcode Op, RVal L, RVal R) {
-    
+
     if (L.isUndef() || R.isUndef())
       return UndefinedVal();
-    
+
     if (L.isUnknown() || R.isUnknown())
       return UnknownVal();
-        
+
     if (isa<LVal>(L)) {
       if (isa<LVal>(R))
         return TF->EvalBinOp(*this, Op, cast<LVal>(L), cast<LVal>(R));
       else
         return TF->EvalBinOp(*this, Op, cast<LVal>(L), cast<NonLVal>(R));
     }
-    
+
     if (isa<LVal>(R)) {
       // Support pointer arithmetic where the increment/decrement operand
       // is on the left and the pointer on the right.
-      
+
       assert (Op == BinaryOperator::Add || Op == BinaryOperator::Sub);
 
-      // Commute the operands.      
+      // Commute the operands.
       return TF->EvalBinOp(*this, Op, cast<LVal>(R), cast<NonLVal>(L));
     }
     else
@@ -631,11 +609,11 @@ protected:
   }
   
   void EvalCall(NodeSet& Dst, CallExpr* CE, RVal L, NodeTy* Pred) {
-    assert (Builder && "GRStmtNodeBuilder must be defined.");    
+    assert (Builder && "GRStmtNodeBuilder must be defined.");
     TF->EvalCall(Dst, *this, *Builder, CE, L, Pred);
   }
   
-  void EvalObjCMessageExpr(NodeSet& Dst, ObjCMessageExpr* ME, NodeTy* Pred) {    
+  void EvalObjCMessageExpr(NodeSet& Dst, ObjCMessageExpr* ME, NodeTy* Pred) {
     assert (Builder && "GRStmtNodeBuilder must be defined.");
     TF->EvalObjCMessageExpr(Dst, *this, *Builder, ME, Pred);
   }
@@ -643,7 +621,16 @@ protected:
   void EvalStore(NodeSet& Dst, Expr* E, NodeTy* Pred, ValueState* St,
                  RVal TargetLV, RVal Val);
   
-  void EvalReturn(NodeSet& Dst, ReturnStmt* s, NodeTy* Pred);  
+  // FIXME: The "CheckOnly" option exists only because Array and Field
+  //  loads aren't fully implemented.  Eventually this option will go away.
+  
+  void EvalLoad(NodeSet& Dst, Expr* Ex, NodeTy* Pred,
+                ValueState* St, RVal location, bool CheckOnly = false);
+  
+  ValueState* EvalLocation(Expr* Ex, NodeTy* Pred,
+                           ValueState* St, RVal location, bool isLoad = false);
+  
+  void EvalReturn(NodeSet& Dst, ReturnStmt* s, NodeTy* Pred);
   
   ValueState* MarkBranch(ValueState* St, Stmt* Terminator, bool branchTaken);
 };
diff --git a/include/clang/Analysis/PathSensitive/ValueState.h b/include/clang/Analysis/PathSensitive/ValueState.h
index 5e287b23b0..a4133085a8 100644
--- a/include/clang/Analysis/PathSensitive/ValueState.h
+++ b/include/clang/Analysis/PathSensitive/ValueState.h
@@ -242,7 +242,6 @@ public:
 
   RVal GetRVal(ValueState* St, Expr* E);
   RVal GetRVal(ValueState* St, LVal LV, QualType T = QualType());    
-  RVal GetLVal(ValueState* St, Expr* E);
   
   RVal GetBlkExprRVal(ValueState* St, Expr* Ex);
   
diff --git a/include/clang/Analysis/ProgramPoint.h b/include/clang/Analysis/ProgramPoint.h
index 5fc1fb651b..efd16b416d 100644
--- a/include/clang/Analysis/ProgramPoint.h
+++ b/include/clang/Analysis/ProgramPoint.h
@@ -25,8 +25,9 @@ namespace clang {
     
 class ProgramPoint {
 public:
-  enum Kind { BlockEntranceKind=0, PostStmtKind=1, BlockExitKind=2,
-              BlockEdgeSrcKind=3, BlockEdgeDstKind=4, BlockEdgeAuxKind=5 }; 
+  enum Kind { BlockEntranceKind=0, PostStmtKind=1, PostLoadKind=2,
+              BlockExitKind=3, BlockEdgeSrcKind=4, BlockEdgeDstKind=5,
+              BlockEdgeAuxKind=6 }; 
 protected:
   uintptr_t Data;
 
@@ -100,13 +101,25 @@ public:
 
 
 class PostStmt : public ProgramPoint {
+protected:
+  PostStmt(const Stmt* S, Kind k) : ProgramPoint(S, k) {}    
 public:
   PostStmt(const Stmt* S) : ProgramPoint(S, PostStmtKind) {}
-  
+      
   Stmt* getStmt() const { return (Stmt*) getRawPtr(); }
 
   static bool classof(const ProgramPoint* Location) {
-    return Location->getKind() == PostStmtKind;
+    unsigned k = Location->getKind();
+    return k == PostStmtKind || k == PostLoadKind;
+  }
+};
+  
+class PostLoad : public PostStmt {
+public:
+  PostLoad(const Stmt* S) : PostStmt(S, PostLoadKind) {}
+  
+  static bool classof(const ProgramPoint* Location) {
+    return Location->getKind() == PostLoadKind;
   }
 };
   
diff --git a/lib/Analysis/GRCoreEngine.cpp b/lib/Analysis/GRCoreEngine.cpp
index c2b8c115fd..05f9303856 100644
--- a/lib/Analysis/GRCoreEngine.cpp
+++ b/lib/Analysis/GRCoreEngine.cpp
@@ -102,7 +102,8 @@ bool GRCoreEngineImpl::ExecuteWorkList(unsigned Steps) {
       case ProgramPoint::BlockExitKind:
         assert (false && "BlockExit location never occur in forward analysis.");
         break;
-        
+      
+      case ProgramPoint::PostLoadKind:
       case ProgramPoint::PostStmtKind:
         HandlePostStmt(cast<PostStmt>(Node->getLocation()), WU.getBlock(),
                        WU.getIndex(), Node);
@@ -316,11 +317,13 @@ void GRStmtNodeBuilderImpl::GenerateAutoTransition(ExplodedNodeImpl* N) {
     Eng.WList->Enqueue(Succ, B, Idx+1);
 }
 
-ExplodedNodeImpl* GRStmtNodeBuilderImpl::generateNodeImpl(Stmt* S, void* State,
-                                                      ExplodedNodeImpl* Pred) {
+ExplodedNodeImpl*
+GRStmtNodeBuilderImpl::generateNodeImpl(Stmt* S, void* State,
+                                        ExplodedNodeImpl* Pred, bool isLoad) {
   
   bool IsNew;
-  ExplodedNodeImpl* N = Eng.G->getNodeImpl(PostStmt(S), State, &IsNew);
+  ProgramPoint Loc = isLoad ? PostLoad(S) : PostStmt(S);
+  ExplodedNodeImpl* N = Eng.G->getNodeImpl(Loc, State, &IsNew);
   N->addPredecessor(Pred);
   Deferred.erase(Pred);
   
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 287c6e982a..2353058e76 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -312,7 +312,7 @@ void GRExprEngine::Visit(Stmt* S, NodeTy* Pred, NodeSet& Dst) {
     }
       
     case Stmt::DeclRefExprClass:
-      VisitDeclRefExpr(cast<DeclRefExpr>(S), Pred, Dst);
+      VisitDeclRefExpr(cast<DeclRefExpr>(S), Pred, Dst, false);
       break;
       
     case Stmt::DeclStmtClass:
@@ -339,6 +339,10 @@ void GRExprEngine::Visit(Stmt* S, NodeTy* Pred, NodeSet& Dst) {
       Visit(cast<ParenExpr>(S)->getSubExpr()->IgnoreParens(), Pred, Dst);
       break;
       
+    case Stmt::ReturnStmtClass:
+      VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst);
+      break;
+      
     case Stmt::SizeOfAlignOfTypeExprClass:
       VisitSizeOfAlignOfTypeExpr(cast<SizeOfAlignOfTypeExpr>(S), Pred, Dst);
       break;
@@ -360,25 +364,41 @@ void GRExprEngine::Visit(Stmt* S, NodeTy* Pred, NodeSet& Dst) {
       break;
     }
       
-      // FIXME: We may wish to always bind state to ReturnStmts so
-      //  that users can quickly query what was the state at the
-      //  exit points of a function.
+    case Stmt::UnaryOperatorClass:
+      VisitUnaryOperator(cast<UnaryOperator>(S), Pred, Dst, false);
+      break;
+  }
+}
+
+void GRExprEngine::VisitLVal(Expr* Ex, NodeTy* Pred, NodeSet& Dst) {
+  
+  Ex = Ex->IgnoreParens();
+  
+  if (Ex != CurrentStmt && getCFG().isBlkExpr(Ex)) {
+    Dst.Add(Pred);
+    return;
+  }
+  
+  switch (Ex->getStmtClass()) {
+    default:
+      Visit(Ex, Pred, Dst);
+      return;
       
-    case Stmt::ReturnStmtClass:
-      VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst); break;
+    case Stmt::ArraySubscriptExprClass:
+      VisitArraySubscriptExpr(cast<ArraySubscriptExpr>(Ex), Pred, Dst, true);
+      return;
       
-    case Stmt::UnaryOperatorClass: {
-      UnaryOperator* U = cast<UnaryOperator>(S);
+    case Stmt::DeclRefExprClass:
+      VisitDeclRefExpr(cast<DeclRefExpr>(Ex), Pred, Dst, true);
+      return;
       
-      switch (U->getOpcode()) {
-        case UnaryOperator::Deref: VisitDeref(U, Pred, Dst); break;
-        case UnaryOperator::Plus:  Visit(U->getSubExpr(), Pred, Dst); break;
-        case UnaryOperator::SizeOf: VisitSizeOfExpr(U, Pred, Dst); break;
-        default: VisitUnaryOperator(U, Pred, Dst); break;
-      }
+    case Stmt::UnaryOperatorClass:
+      VisitUnaryOperator(cast<UnaryOperator>(Ex), Pred, Dst, true);
+      return;
       
-      break;
-    }
+    case Stmt::MemberExprClass:
+      VisitMemberExpr(cast<MemberExpr>(Ex), Pred, Dst, true);
+      return;
   }
 }
 
@@ -741,20 +761,18 @@ void GRExprEngine::VisitLogicalExpr(BinaryOperator* B, NodeTy* Pred,
 // Transfer functions: Loads and stores.
 //===----------------------------------------------------------------------===//
 
-void GRExprEngine::VisitDeclRefExpr(DeclRefExpr* D, NodeTy* Pred, NodeSet& Dst){
-
-  if (D != CurrentStmt) {
-    Dst.Add(Pred); // No-op. Simply propagate the current state unchanged.
-    return;
-  }
-  
-  // If we are here, we are loading the value of the decl and binding
-  // it to the block-level expression.
+void GRExprEngine::VisitDeclRefExpr(DeclRefExpr* D, NodeTy* Pred, NodeSet& Dst,
+                                    bool asLVal) {
   
-  ValueState* St = GetState(Pred);  
+  ValueState* St = GetState(Pred);
   RVal X = RVal::MakeVal(BasicVals, D);
-  RVal Y = isa<lval::DeclVal>(X) ? GetRVal(St, cast<lval::DeclVal>(X)) : X;
-  MakeNode(Dst, D, Pred, SetBlkExprRVal(St, D, Y));
+  
+  if (asLVal)
+    MakeNode(Dst, D, Pred, SetRVal(St, D, cast<LVal>(X)));
+  else {
+    RVal V = isa<lval::DeclVal>(X) ? GetRVal(St, cast<LVal>(X)) : X;
+    MakeNode(Dst, D, Pred, SetRVal(St, D, V));
+  }
 }
 
 /// VisitArraySubscriptExpr - Transfer function for array accesses
@@ -763,24 +781,57 @@ void GRExprEngine::VisitArraySubscriptExpr(ArraySubscriptExpr* A, NodeTy* Pred,
   
   Expr* Base = A->getBase()->IgnoreParens();
   
-  // Evaluate the base.  
-  NodeSet Tmp1;
-  Visit(Base, Pred, Tmp1);
+  // Always visit the base as an LVal expression.  This computes the
+  // abstract address of the base object.
+  NodeSet Tmp;
   
-  // Dereference the base.
-  NodeSet Tmp2;
-
-  for (NodeSet::iterator I=Tmp1.begin(), E=Tmp1.end(); I!=E; ++I) {
-    ValueState* St = GetState(*I);
-    VisitDeref(Base, GetRVal(St, Base), St, *I, Tmp2, true);
+  if (Base->getType()->isPointerType()) // Base always is an LVal.
+    Visit(Base, Pred, Tmp);
+  else  
+    VisitLVal(Base, Pred, Tmp);
+  
+  // TODO: Compute the LVal for the entry.  This will enable array sensitivity
+  //  for the analysis.
+  
+  // Return the LVal of the array entry.
+  if (asLVal) {
+    
+    // This is a redunant copy; we do this as a placeholder for future logic.
+    for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
+      
+      ValueState* St = GetState(*I);
+      RVal V = GetRVal(St, Base);
+      
+      // TODO: Compute the LVal for the entry.  This will enable array sensitivity
+      //  for the analysis.
+      
+      if (!(V.isUndef() || V.isUnknown() || isa<lval::ConcreteInt>(V)))
+        V = UnknownVal();      
+      
+      MakeNode(Dst, A, *I, SetRVal(St, A, V)); 
+    }
+          
+    return;
   }
   
-  // Get the index.
-  Tmp1.clear();
-  Expr* Index = A->getIdx()->IgnoreParens();
+  // We are doing a load.  Check for a bad dereference.  In the future we
+  // will check the actual entry lval; for now, check the Base LVal.  For now
+  // the load will just return "UnknownVal" (since we don't have array
+  // sensitivity), but it will perform a null check.
   
-  for (NodeSet::iterator I=Tmp2.begin(), E=Tmp2.end(); I!=E; ++I)
-    Visit(Index, *I, Dst);
+  for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
+
+    ValueState* St = GetState(*I);    
+    RVal V = GetRVal(St, Base);
+    
+    // TODO: Compute the LVal for the entry.  This will enable array sensitivity
+    //  for the analysis.
+    
+    if (!(V.isUndef() || V.isUnknown() || isa<lval::ConcreteInt>(V)))
+      V = UnknownVal();
+    
+    EvalLoad(Dst, A, *I, St, GetRVal(St, Base),  true);
+  }
 }
 
 /// VisitMemberExpr - Transfer function for member expressions.
@@ -789,50 +840,165 @@ void GRExprEngine::VisitMemberExpr(MemberExpr* M, NodeTy* Pred,
   
   Expr* Base = M->getBase()->IgnoreParens();
 
+  // Always visit the base as an LVal expression.  This computes the
+  // abstract address of the base object.
   NodeSet Tmp;
-  VisitLVal(Base, Pred, Tmp);
   
-  if (Base->getType()->isPointerType()) {
-    NodeSet Tmp2;
+  if (Base->getType()->isPointerType()) // Base always is an LVal.
+    Visit(Base, Pred, Tmp);
+  else  
+    VisitLVal(Base, Pred, Tmp);
+  
+  
+  // Return the LVal of the field access.
+  if (asLVal) {
     
+    // This is a redunant copy; we do this as a placeholder for future logic.
     for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
-      ValueState* St = GetState(*I);      
-      VisitDeref(Base, GetRVal(St, Base), St, *I, Tmp2, true);
+      ValueState* St = GetState(*I);
+      RVal V = GetRVal(St, Base);
+
+      // TODO: Compute the LVal for the field.  This will enable field
+      //  sensitivity for the analysis.
+      
+      if (!(V.isUndef() || V.isUnknown() || isa<lval::ConcreteInt>(V)))
+        V = UnknownVal();      
+      
+      MakeNode(Dst, M, *I, SetRVal(St, M, V)); 
+      
     }
+
+    return;
+  }
+  
+  // We are doing a load.  Check for a bad dereference.  In the future we
+  // will check the actual field lval; for now, check the Base LVal.  For now
+  // the load will just return "UnknownVal" (since we don't have field
+  // sensitivity), but it will perform a null check.
+  
+  for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
+    ValueState* St = GetState(*I);
     
-    for (NodeSet::iterator I=Tmp2.begin(), E=Tmp2.end(); I!=E; ++I)
-      VisitMemberExprField(M, Base, *I, Dst, asLVal);
+    RVal V = GetRVal(St, Base);
+    
+    // TODO: Compute the LVal for the field.  This will enable field
+    //  sensitivity for the analysis.
+    
+    if (!(V.isUndef() || V.isUnknown() || isa<lval::ConcreteInt>(V)))
+      V = UnknownVal();
+    
+    EvalLoad(Dst, M, *I, St, V, true);
   }
-  else
-    for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I)
-      VisitMemberExprField(M, Base, *I, Dst, asLVal);
 }
 
-void GRExprEngine::VisitMemberExprField(MemberExpr* M, Expr* Base, NodeTy* Pred,
-                                        NodeSet& Dst, bool asLVal) {
-  Dst.Add(Pred);  
-}
-
-void GRExprEngine::EvalStore(NodeSet& Dst, Expr* E, NodeTy* Pred,
-                             ValueState* St, RVal TargetLV, RVal Val) {
+void GRExprEngine::EvalStore(NodeSet& Dst, Expr* Ex, NodeTy* Pred,
+                             ValueState* St, RVal location, RVal Val) {
   
   assert (Builder && "GRStmtNodeBuilder must be defined.");
   
+  // Evaluate the location (checks for bad dereferences).
+  St = EvalLocation(Ex, Pred, St, location);
+  
+  if (!St)
+    return;
+  
+  // Proceed with the store.
+  
   unsigned size = Dst.size();  
 
   SaveAndRestore<bool> OldSink(Builder->BuildSinks);
   SaveOr OldHasGen(Builder->HasGeneratedNode);
 
-  assert (!TargetLV.isUndef());
+  assert (!location.isUndef());
   
-  TF->EvalStore(Dst, *this, *Builder, E, Pred, St, TargetLV, Val);
+  TF->EvalStore(Dst, *this, *Builder, Ex, Pred, St, location, Val);
   
   // Handle the case where no nodes where generated.  Auto-generate that
   // contains the updated state if we aren't generating sinks.
   
   if (!Builder->BuildSinks && Dst.size() == size && !Builder->HasGeneratedNode)
-    TF->GRTransferFuncs::EvalStore(Dst, *this, *Builder, E, Pred, St,
-                                   TargetLV, Val);
+    TF->GRTransferFuncs::EvalStore(Dst, *this, *Builder, Ex, Pred, St,
+                                   location, Val);
+}
+
+void GRExprEngine::EvalLoad(NodeSet& Dst, Expr* Ex, NodeTy* Pred,
+                            ValueState* St, RVal location, bool CheckOnly) {
+
+  // Evaluate the location (checks for bad dereferences).
+  
+  St = EvalLocation(Ex, Pred, St, location, true);
+  
+  if (!St)
+    return;
+  
+  // Proceed with the load.
+
+  // FIXME: Currently symbolic analysis "generates" new symbols
+  //  for the contents of values.  We need a better approach.
+
+  // FIXME: The "CheckOnly" option exists only because Array and Field
+  //  loads aren't fully implemented.  Eventually this option will go away.
+  
+  if (location.isUnknown() || CheckOnly)
+    MakeNode(Dst, Ex, Pred, St);
+  else    
+    MakeNode(Dst, Ex, Pred, SetRVal(St, Ex, GetRVal(St, cast<LVal>(location),
+                                                    Ex->getType())));  
+}
+
+ValueState* GRExprEngine::EvalLocation(Expr* Ex, NodeTy* Pred,
+                                       ValueState* St, RVal location,
+                                       bool isLoad) {
+  
+  // Check for loads/stores from/to undefined values.  
+  if (location.isUndef()) {
+    if (NodeTy* Succ = Builder->generateNode(Ex, St, Pred, isLoad)) {
+      Succ->markAsSink();
+      UndefDeref.insert(Succ);
+    }
+    
+    return NULL;
+  }
+  
+  // Check for loads/stores from/to unknown locations.  Treat as No-Ops.
+  if (location.isUnknown())
+    return St;
+  
+  // During a load, one of two possible situations arise:
+  //  (1) A crash, because the location (pointer) was NULL.
+  //  (2) The location (pointer) is not NULL, and the dereference works.
+  // 
+  // We add these assumptions.
+  
+  LVal LV = cast<LVal>(location);    
+  
+  // "Assume" that the pointer is not NULL.
+  
+  bool isFeasibleNotNull = false;
+  ValueState* StNotNull = Assume(St, LV, true, isFeasibleNotNull);
+  
+  // "Assume" that the pointer is NULL.
+  
+  bool isFeasibleNull = false;
+  ValueState* StNull = Assume(St, LV, false, isFeasibleNull);
+  
+  if (isFeasibleNull) {
+    
+    // We don't use "MakeNode" here because the node will be a sink
+    // and we have no intention of processing it later.
+    
+    NodeTy* NullNode = Builder->generateNode(Ex, StNull, Pred, isLoad);
+    
+    if (NullNode) {
+      
+      NullNode->markAsSink();
+      
+      if (isFeasibleNotNull) ImplicitNullDeref.insert(NullNode);
+      else ExplicitNullDeref.insert(NullNode);
+    }
+  }
+  
+  return isFeasibleNotNull ? StNotNull : NULL;
 }
 
 //===----------------------------------------------------------------------===//
@@ -870,7 +1036,7 @@ void GRExprEngine::VisitCall(CallExpr* CE, NodeTy* Pred,
   for (NodeSet::iterator DI = DstTmp.begin(), DE = DstTmp.end(); DI!=DE; ++DI) {
 
     ValueState* St = GetState(*DI);
-    RVal L = GetLVal(St, Callee);
+    RVal L = GetRVal(St, Callee);
 
     // FIXME: Add support for symbolic function calls (calls involving
     //  function pointer values that are symbolic).
@@ -1127,7 +1293,7 @@ void GRExprEngine::VisitCast(Expr* CastE, Expr* Ex, NodeTy* Pred, NodeSet& Dst){
   for (NodeSet::iterator I1 = S1.begin(), E1 = S1.end(); I1 != E1; ++I1) {
     NodeTy* N = *I1;
     ValueState* St = GetState(N);
-    RVal V = T->isReferenceType() ? GetLVal(St, Ex) : GetRVal(St, Ex);
+    RVal V = GetRVal(St, Ex);
 
     // Unknown?
     
@@ -1275,7 +1441,6 @@ void GRExprEngine::VisitDeclStmtAux(DeclStmt* DS, ScopedDecl* D,
 void GRExprEngine::VisitSizeOfAlignOfTypeExpr(SizeOfAlignOfTypeExpr* Ex,
                                               NodeTy* Pred,
                                               NodeSet& Dst) {
-
   QualType T = Ex->getArgumentType();
   uint64_t amt;  
   
@@ -1295,284 +1460,191 @@ void GRExprEngine::VisitSizeOfAlignOfTypeExpr(SizeOfAlignOfTypeExpr* Ex,
     amt = getContext().getTypeAlign(T) / 8;
   
   MakeNode(Dst, Ex, Pred,
-         SetRVal(GetState(Pred), Ex,
-                 NonLVal::MakeVal(BasicVals, amt, Ex->getType())));  
-}
-
-void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred,
-                              NodeSet& Dst, bool GetLVal) {
-  
-  Expr* Ex = U->getSubExpr()->IgnoreParens();
-  
-  NodeSet DstTmp;
-  
-  if (isa<DeclRefExpr>(Ex))
-    DstTmp.Add(Pred);
-  else
-    Visit(Ex, Pred, DstTmp);
-  
-  for (NodeSet::iterator I = DstTmp.begin(), DE = DstTmp.end(); I != DE; ++I) {
-    ValueState* St = GetState(*I);
-    RVal V = GetRVal(St, Ex);
-    VisitDeref(U, V, St, *I, Dst, GetLVal);
-  }
+           SetRVal(GetState(Pred), Ex,
+                   NonLVal::MakeVal(BasicVals, amt, Ex->getType())));  
 }
 
-void GRExprEngine::VisitDeref(Expr* Ex, RVal V, ValueState* St, NodeTy* Pred,
-                              NodeSet& Dst, bool GetLVal) {
-  
-  // Check for dereferences of undefined values.
-  
-  if (V.isUndef()) {
-    if (NodeTy* Succ = Builder->generateNode(Ex, St, Pred)) {
-      Succ->markAsSink();
-      UndefDeref.insert(Succ);
-    }
-    
-    return;
-  }
-  
-  // Check for dereferences of unknown values.  Treat as No-Ops.
-  
-  if (V.isUnknown()) {
-    Dst.Add(Pred);
-    return;
-  }
-  
-  // After a dereference, one of two possible situations arise:
-  //  (1) A crash, because the pointer was NULL.
-  //  (2) The pointer is not NULL, and the dereference works.
-  // 
-  // We add these assumptions.
-  
-  LVal LV = cast<LVal>(V);    
-  bool isFeasibleNotNull;
-  
-  // "Assume" that the pointer is Not-NULL.
-  
-  ValueState* StNotNull = Assume(St, LV, true, isFeasibleNotNull);
-  
-  if (isFeasibleNotNull) {
-    
-    if (GetLVal)
-      MakeNode(Dst, Ex, Pred, SetRVal(StNotNull, Ex, LV));
-    else {
-      
-      // FIXME: Currently symbolic analysis "generates" new symbols
-      //  for the contents of values.  We need a better approach.
-      
-      MakeNode(Dst, Ex, Pred,
-               SetRVal(StNotNull, Ex, GetRVal(StNotNull, LV, Ex->getType())));
-    }
-  }
-  
-  bool isFeasibleNull;
-  
-  // Now "assume" that the pointer is NULL.
-  
-  ValueState* StNull = Assume(St, LV, false, isFeasibleNull);
-  
-  if (isFeasibleNull) {
-    
-    // We don't use "MakeNode" here because the node will be a sink
-    // and we have no intention of processing it later.
-    
-    NodeTy* NullNode = Builder->generateNode(Ex, StNull, Pred);
-    
-    if (NullNode) {
-      
-      NullNode->markAsSink();
-      
-      if (isFeasibleNotNull) ImplicitNullDeref.insert(NullNode);
-      else ExplicitNullDeref.insert(NullNode);
-    }
-  }
-}
 
 void GRExprEngine::VisitUnaryOperator(UnaryOperator* U, NodeTy* Pred,
-                                      NodeSet& Dst) {
-  
-  NodeSet S1;
-  
-  assert (U->getOpcode() != UnaryOperator::Deref);
-  assert (U->getOpcode() != UnaryOperator::SizeOf);
-  assert (U->getOpcode() != UnaryOperator::AlignOf);
-  
-  bool use_GetLVal = false;
-  
+                                      NodeSet& Dst, bool asLVal) {
+
   switch (U->getOpcode()) {
-    case UnaryOperator::PostInc:
-    case UnaryOperator::PostDec:
-    case UnaryOperator::PreInc:
-    case UnaryOperator::PreDec:
-    case UnaryOperator::AddrOf:
-      // Evalue subexpression as an LVal.
-      use_GetLVal = true;
-      VisitLVal(U->getSubExpr(), Pred, S1);
-      break;
       
     default:
-      Visit(U->getSubExpr(), Pred, S1);
       break;
-  }
-
-  for (NodeSet::iterator I1 = S1.begin(), E1 = S1.end(); I1 != E1; ++I1) {
-
-    NodeTy* N1 = *I1;
-    ValueState* St = GetState(N1);
+      
+    case UnaryOperator::Deref: {
+      
+      Expr* Ex = U->getSubExpr()->IgnoreParens();
+      NodeSet Tmp;
+      Visit(Ex, Pred, Tmp);
+      
+      for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
         
-    RVal SubV = use_GetLVal ? GetLVal(St, U->getSubExpr()) : 
-                              GetRVal(St, U->getSubExpr());
-    
-    if (SubV.isUnknown()) {
-      Dst.Add(N1);
-      continue;
-    }
+        ValueState* St = GetState(*I);
+        RVal location = GetRVal(St, Ex);
+        
+        if (asLVal)
+          MakeNode(Dst, U, *I, SetRVal(St, U, location));
+        else
+          EvalLoad(Dst, Ex, *I, St, location);
+      } 
 
-    if (SubV.isUndef()) {
-      MakeNode(Dst, U, N1, SetRVal(St, U, SubV));
-      continue;
+      return;
     }
-    
-    if (U->isIncrementDecrementOp()) {
+
+    case UnaryOperator::Plus: assert (!asLVal);  // FALL-THROUGH.
+    case UnaryOperator::Extension: {
       
-      // Handle ++ and -- (both pre- and post-increment).
+      // Unary "+" is a no-op, similar to a parentheses.  We still have places
+      // where it may be a block-level expression, so we need to
+      // generate an extra node that just propagates the value of the
+      // subexpression.
+
+      Expr* Ex = U->getSubExpr()->IgnoreParens();
+      NodeSet Tmp;
+      Visit(Ex, Pred, Tmp);
       
-      LVal SubLV = cast<LVal>(SubV); 
-      RVal V  = GetRVal(St, SubLV, U->getType());
+      for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {        
+        ValueState* St = GetState(*I);
+        MakeNode(Dst, U, *I, SetRVal(St, U, GetRVal(St, Ex)));
+      }
       
-      if (V.isUnknown()) {
-        Dst.Add(N1);
-        continue;
+      return;
+    }
+    
+    case UnaryOperator::AddrOf: {
+      
+      assert (!asLVal);
+      Expr* Ex = U->getSubExpr()->IgnoreParens();
+      NodeSet Tmp;
+      VisitLVal(Ex, Pred, Tmp);
+     
+      for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {        
+        ValueState* St = GetState(*I);
+        RVal V = GetRVal(St, Ex);
+        St = SetRVal(St, U, V);
+        MakeNode(Dst, U, *I, St);
       }
 
-      // Propagate undefined values.      
-      if (V.isUndef()) {
-        MakeNode(Dst, U, N1, SetRVal(St, U, V));
-        continue;
-      }
-      
-      // Handle all other values.
+      return; 
+    }
       
-      BinaryOperator::Opcode Op = U->isIncrementOp() ? BinaryOperator::Add
-                                                     : BinaryOperator::Sub;
+    case UnaryOperator::LNot:
+    case UnaryOperator::Minus:
+    case UnaryOperator::Not: {
       
-      RVal Result = EvalBinOp(Op, V, MakeConstantVal(1U, U));
+      assert (!asLVal);
+      Expr* Ex = U->getSubExpr()->IgnoreParens();
+      NodeSet Tmp;
+      Visit(Ex, Pred, Tmp);
       
-      if (U->isPostfix())
-        St = SetRVal(SetRVal(St, U, V), SubLV, Result);
-      else
-        St = SetRVal(SetRVal(St, U, Result), SubLV, Result);
+      for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {        
+        ValueState* St = GetState(*I);
+        RVal V = GetRVal(St, Ex);
         
-      MakeNode(Dst, U, N1, St);
-      continue;
-    }    
-    
-    // Handle all other unary operators.
-    
-    switch (U->getOpcode()) {
-        
-      case UnaryOperator::Extension:
-        St = SetRVal(St, U, SubV);
-        break;
-
-      case UnaryOperator::Minus:
-        St = SetRVal(St, U, EvalMinus(U, cast<NonLVal>(SubV)));
-        break;
+        if (V.isUnknownOrUndef()) {
+          MakeNode(Dst, U, *I, SetRVal(St, U, V));
+          continue;
+        }
         
-      case UnaryOperator::Not:
-        St = SetRVal(St, U, EvalComplement(cast<NonLVal>(SubV)));
-        break;
+        switch (U->getOpcode()) {
+          default:
+            assert(false && "Invalid Opcode.");
+            break;
+            
+          case UnaryOperator::Not:
+            St = SetRVal(St, U, EvalComplement(cast<NonLVal>(V)));
+            break;            
+            
+          case UnaryOperator::Minus:
+            St = SetRVal(St, U, EvalMinus(U, cast<NonLVal>(V)));
+            break;   
+            
+          case UnaryOperator::LNot:   
+            
+            // C99 6.5.3.3: "The expression !E is equivalent to (0==E)."
+            //
+            //  Note: technically we do "E == 0", but this is the same in the
+            //    transfer functions as "0 == E".
+            
+            if (isa<LVal>(V)) {
+              lval::ConcreteInt X(BasicVals.getZeroWithPtrWidth());
+              RVal Result = EvalBinOp(BinaryOperator::EQ, cast<LVal>(V), X);
+              St = SetRVal(St, U, Result);
+            }