[cfe-commits] r64525 - in /cfe/trunk: include/clang/Analysis/PathSensitive/GRExprEngine.h include/clang/Analysis/PathSensitive/GRExprEngineBuilders.h include/clang/Analysis/PathSensitive/GRTransferFuncs.h lib/Analysis/CFRefCount.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/GRTransferFuncs.cpp

Ted Kremenek kremenek at apple.com
Fri Feb 13 17:43:44 PST 2009


Author: kremenek
Date: Fri Feb 13 19:43:44 2009
New Revision: 64525

URL: http://llvm.org/viewvc/llvm-project?rev=64525&view=rev
Log:
Static analyzer:
- Added a new 'node builder' class called GRStmtNodeBuilderRef (name may
  change). This is essentially a smart reference to a GRStmtNodeBuilder object
  that keeps track of the current context (predecessor node, GRExprEngine
  object, etc.) The idea is to gradually simplify the interface between
  GRExprEngine and GRTransferFuncs using this new builder (i.e., passing 1
  argument instead of 5). It also handles some of the "auto-transition" for node
  creation, simplifying some of the logic in GRExprEngine itself.

- Used GRStmtBuilderRef to replace GRTransferFuncs::EvalStore with
  GRTransferFuncs::EvalBind. The new EvalBind method will be used at any
  arbitrary places where a binding between a location and value takes place.
  Moreover, GRTransferFuncs no longer has the responsibility to request
  StoreManager to do the binding; this is now in GRExprEngine::EvalBind. All
  GRTransferFuncs::EvalBind does is checker-specific logic (which can be a
  no-op).

Added:
    cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngineBuilders.h
Modified:
    cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
    cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
    cfe/trunk/lib/Analysis/CFRefCount.cpp
    cfe/trunk/lib/Analysis/GRExprEngine.cpp
    cfe/trunk/lib/Analysis/GRTransferFuncs.cpp

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

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Fri Feb 13 19:43:44 2009
@@ -8,7 +8,7 @@
 //===----------------------------------------------------------------------===//
 //
 //  This file defines a meta-engine for path-sensitive dataflow analysis that
-//  is built on GREngine, but provides the boilerplate to execute transfer
+//  is built on GRCoreEngine, but provides the boilerplate to execute transfer
 //  functions and build the ExplodedGraph at the expression level.
 //
 //===----------------------------------------------------------------------===//

Added: cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngineBuilders.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngineBuilders.h?rev=64525&view=auto

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngineBuilders.h (added)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngineBuilders.h Fri Feb 13 19:43:44 2009
@@ -0,0 +1,100 @@
+//===-- GRExprEngineBuilders.h - "Builder" classes for GRExprEngine -*- C++ -*-=
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines smart builder "references" which are used to marshal
+//  builders between GRExprEngine objects and their related components.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_GREXPRENGINE_BUILDERS
+#define LLVM_CLANG_ANALYSIS_GREXPRENGINE_BUILDERS
+#include "clang/Analysis/PathSensitive/GRExprEngine.h"
+
+namespace clang {
+  
+
+// SaveAndRestore - A utility class that uses RAII to save and restore
+//  the value of a variable.
+template<typename T>
+struct SaveAndRestore {
+  SaveAndRestore(T& x) : X(x), old_value(x) {}
+  ~SaveAndRestore() { X = old_value; }
+  T get() { return old_value; }
+private:  
+  T& X;
+  T old_value;
+};
+
+// SaveOr - Similar to SaveAndRestore.  Operates only on bools; the old
+//  value of a variable is saved, and during the dstor the old value is
+//  or'ed with the new value.
+struct SaveOr {
+  SaveOr(bool& x) : X(x), old_value(x) { x = false; }
+  ~SaveOr() { X |= old_value; }
+private:
+  bool& X;
+  const bool old_value;
+};
+
+class GRStmtNodeBuilderRef {
+  GRExprEngine::NodeSet &Dst;
+  GRExprEngine::StmtNodeBuilder &B;
+  GRExprEngine& Eng;
+  GRExprEngine::NodeTy* Pred;
+  const GRState* state;
+  const Stmt* stmt;
+  const unsigned OldSize;
+  const bool AutoCreateNode;
+  SaveAndRestore<bool> OldSink;
+  SaveOr OldHasGen;
+
+private:
+  friend class GRExprEngine;
+  
+  GRStmtNodeBuilderRef(); // do not implement
+  void operator=(const GRStmtNodeBuilderRef&); // do not implement
+  
+  GRStmtNodeBuilderRef(GRExprEngine::NodeSet &dst,
+                       GRExprEngine::StmtNodeBuilder &builder,
+                       GRExprEngine& eng,
+                       GRExprEngine::NodeTy* pred,
+                       const GRState *st,
+                       const Stmt* s, bool auto_create_node)
+  : Dst(dst), B(builder), Eng(eng), Pred(pred),
+    state(st), stmt(s), OldSize(Dst.size()), AutoCreateNode(auto_create_node),
+    OldSink(B.BuildSinks), OldHasGen(B.HasGeneratedNode) {}
+  
+public:
+
+  ~GRStmtNodeBuilderRef() {
+    // Handle the case where no nodes where generated.  Auto-generate that
+    // contains the updated state if we aren't generating sinks.  
+    if (!B.BuildSinks && Dst.size() == OldSize && !B.HasGeneratedNode) {
+      if (AutoCreateNode)
+        B.MakeNode(Dst, const_cast<Stmt*>(stmt), Pred, state);
+      else
+        Dst.Add(Pred);
+    }
+  }
+  
+  GRStateRef getState() {
+    return GRStateRef(state, Eng.getStateManager());
+  }
+
+  GRStateManager& getStateManager() {
+    return Eng.getStateManager();
+  }
+  
+  GRExprEngine::NodeTy* MakeNode(const GRState* state) {
+    return B.MakeNode(Dst, const_cast<Stmt*>(stmt), Pred, state);    
+  }    
+};
+
+} // end clang namespace
+#endif

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

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h Fri Feb 13 19:43:44 2009
@@ -25,6 +25,7 @@
   class GRExprEngine;
   class BugReporter;
   class ObjCMessageExpr;
+  class GRStmtNodeBuilderRef;
   
 class GRTransferFuncs {
   friend class GRExprEngine;  
@@ -84,15 +85,7 @@
   
   // Stores.
   
-  /// EvalStore - Evaluate the effects of a store, creating a new node
-  ///  the represents the effect of binding 'Val' to the location 'TargetLV'.
-  //   TargetLV is guaranteed to either be an UnknownVal or an Loc.
-  virtual void EvalStore(ExplodedNodeSet<GRState>& Dst,
-                         GRExprEngine& Engine,
-                         GRStmtNodeBuilder<GRState>& Builder,
-                         Expr* E, ExplodedNode<GRState>* Pred,
-                         const GRState* St, SVal TargetLV, SVal Val);
-                         
+  virtual void EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val) {}
   
   // End-of-path and dead symbol notification.
   

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

==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Fri Feb 13 19:43:44 2009
@@ -15,7 +15,7 @@
 #include "GRSimpleVals.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Analysis/PathSensitive/GRState.h"
+#include "clang/Analysis/PathSensitive/GRExprEngineBuilders.h"
 #include "clang/Analysis/PathSensitive/GRStateTrait.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Analysis/LocalCheckers.h"
@@ -1360,13 +1360,9 @@
                               ObjCMessageExpr* ME,
                               ExplodedNode<GRState>* Pred);
 
-  // Stores.
-  
-  virtual void EvalStore(ExplodedNodeSet<GRState>& Dst,
-                         GRExprEngine& Engine,
-                         GRStmtNodeBuilder<GRState>& Builder,
-                         Expr* E, ExplodedNode<GRState>* Pred,
-                         const GRState* St, SVal TargetLV, SVal Val);
+  // Stores.  
+  virtual void EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val);
+
   // End-of-path.
   
   virtual void EvalEndPath(GRExprEngine& Engine,
@@ -1741,22 +1737,13 @@
               ME->arg_begin(), ME->arg_end(), Pred);
 }
   
-// Stores.
-
-void CFRefCount::EvalStore(ExplodedNodeSet<GRState>& Dst,
-                           GRExprEngine& Eng,
-                           GRStmtNodeBuilder<GRState>& Builder,
-                           Expr* E, ExplodedNode<GRState>* Pred,
-                           const GRState* St, SVal TargetLV, SVal Val) {
-  
+void CFRefCount::EvalBind(GRStmtNodeBuilderRef& B, SVal location, SVal val) {  
   // Check if we have a binding for "Val" and if we are storing it to something
-  // we don't understand or otherwise the value "escapes" the function.
-  
-  if (!isa<loc::SymbolVal>(Val))
+  // we don't understand or otherwise the value "escapes" the function.  
+  if (!isa<loc::SymbolVal>(val))
     return;
   
-  // Are we storing to something that causes the value to "escape"?
-  
+  // Are we storing to something that causes the value to "escape"?  
   bool escapes = false;
   
   // A value escapes in three possible cases (this may change):
@@ -1764,44 +1751,35 @@
   // (1) we are binding to something that is not a memory region.
   // (2) we are binding to a memregion that does not have stack storage
   // (3) we are binding to a memregion with stack storage that the store
-  //     does not understand.
-  
-  SymbolRef Sym = cast<loc::SymbolVal>(Val).getSymbol();
-  GRStateRef state(St, Eng.getStateManager());
+  //     does not understand.  
+  SymbolRef Sym = cast<loc::SymbolVal>(val).getSymbol();
+  GRStateRef state = B.getState();
 
-  if (!isa<loc::MemRegionVal>(TargetLV))
+  if (!isa<loc::MemRegionVal>(location))
     escapes = true;
   else {
-    const MemRegion* R = cast<loc::MemRegionVal>(TargetLV).getRegion();
-    escapes = !Eng.getStateManager().hasStackStorage(R);
+    const MemRegion* R = cast<loc::MemRegionVal>(location).getRegion();
+    escapes = !B.getStateManager().hasStackStorage(R);
     
     if (!escapes) {
       // To test (3), generate a new state with the binding removed.  If it is
       // the same state, then it escapes (since the store cannot represent
       // the binding).
-      GRStateRef stateNew = state.BindLoc(cast<Loc>(TargetLV), Val);
-      escapes = (stateNew == state);
+      escapes = (state == (state.BindLoc(cast<Loc>(location), UnknownVal())));
     }
   }
-  
-  if (!escapes)
-    return;
 
-  // Do we have a reference count binding?
-  // FIXME: Is this step even needed?  We do blow away the binding anyway.
-  if (!state.get<RefBindings>(Sym))
+  // Our store can represent the binding and we aren't storing to something
+  // that doesn't have local storage.  Just return and have the simulation
+  // state continue as is.  We should also just return if the tracked symbol
+  // is not associated with a reference count.
+  if (!escapes || !state.get<RefBindings>(Sym))
     return;
-  
-  // Nuke the binding.
-  state = state.remove<RefBindings>(Sym);
 
-  // Hand of the remaining logic to the parent implementation.
-  GRSimpleVals::EvalStore(Dst, Eng, Builder, E, Pred, state, TargetLV, Val);
+  // The tracked object excapes. Stop tracking the object.
+  B.MakeNode(state.remove<RefBindings>(Sym));
 }
 
-// End-of-path.
-
-
 std::pair<GRStateRef,bool>
 CFRefCount::HandleSymbolDeath(GRStateManager& VMgr,
                               const GRState* St, const Decl* CD,

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

==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Fri Feb 13 19:43:44 2009
@@ -14,6 +14,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/PathSensitive/GRExprEngine.h"
+#include "clang/Analysis/PathSensitive/GRExprEngineBuilders.h"
+
 #include "clang/Analysis/PathSensitive/BugReporter.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/Streams.h"
@@ -125,28 +127,6 @@
 // Utility methods.
 //===----------------------------------------------------------------------===//
 
-// SaveAndRestore - A utility class that uses RIIA to save and restore
-//  the value of a variable.
-template<typename T>
-struct VISIBILITY_HIDDEN SaveAndRestore {
-  SaveAndRestore(T& x) : X(x), old_value(x) {}
-  ~SaveAndRestore() { X = old_value; }
-  T get() { return old_value; }
-  
-  T& X;
-  T old_value;
-};
-
-// SaveOr - Similar to SaveAndRestore.  Operates only on bools; the old
-//  value of a variable is saved, and during the dstor the old value is
-//  or'ed with the new value.
-struct VISIBILITY_HIDDEN SaveOr {
-  SaveOr(bool& x) : X(x), old_value(x) { x = false; }
-  ~SaveOr() { X |= old_value; }
-  
-  bool& X;
-  bool old_value;
-};
 
 void GRExprEngine::setTransferFunctions(GRTransferFuncs* tf) {
   StateMgr.TF = tf;
@@ -920,17 +900,27 @@
 void GRExprEngine::EvalBind(NodeSet& Dst, Expr* Ex, NodeTy* Pred,
                              const GRState* state, SVal location, SVal Val) {
 
-  unsigned size = Dst.size();    
-  SaveAndRestore<bool> OldSink(Builder->BuildSinks);
-  SaveOr OldHasGen(Builder->HasGeneratedNode);
-
-  getTF().EvalStore(Dst, *this, *Builder, Ex, Pred, state, location, Val);
+  const GRState* newState = 0;
+  
+  if (location.isUnknown()) {
+    // We know that the new state will be the same as the old state since
+    // the location of the binding is "unknown".  Consequently, there
+    // is no reason to just create a new node.
+    newState = state;
+  }
+  else {
+    // We are binding to a value other than 'unknown'.  Perform the binding
+    // using the StoreManager.
+    newState = StateMgr.BindLoc(state, cast<Loc>(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)
-    getTF().GRTransferFuncs::EvalStore(Dst, *this, *Builder, Ex, Pred, state,
-                                       location, Val);
+  // The next thing to do is check if the GRTransferFuncs object wants to
+  // update the state based on the new binding.  If the GRTransferFunc object
+  // doesn't do anything, just auto-propagate the current state.
+  GRStmtNodeBuilderRef BuilderRef(Dst, *Builder, *this, Pred, newState, Ex,
+                                  newState != state);
+    
+  getTF().EvalBind(BuilderRef, location, Val);
 }
 
 /// EvalStore - Handle the semantics of a store via an assignment.

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

==============================================================================
--- cfe/trunk/lib/Analysis/GRTransferFuncs.cpp (original)
+++ cfe/trunk/lib/Analysis/GRTransferFuncs.cpp Fri Feb 13 19:43:44 2009
@@ -17,25 +17,6 @@
 
 using namespace clang;
 
-void GRTransferFuncs::EvalStore(ExplodedNodeSet<GRState>& Dst,
-                                GRExprEngine& Eng,
-                                GRStmtNodeBuilder<GRState>& Builder,
-                                Expr* E, ExplodedNode<GRState>* Pred,
-                                const GRState* St, SVal TargetLV, SVal Val) {
-  
-  // This code basically matches the "safety-net" logic of GRExprEngine:
-  //  bind Val to TargetLV, and create a new node.  We replicate it here
-  //  because subclasses of GRTransferFuncs may wish to call it.
-
-  assert (!TargetLV.isUndef());
-  
-  if (TargetLV.isUnknown())
-    Builder.MakeNode(Dst, E, Pred, St);
-  else
-    Builder.MakeNode(Dst, E, Pred,
-                   Eng.getStateManager().BindLoc(St, cast<Loc>(TargetLV), Val));
-}
-
 void GRTransferFuncs::EvalBinOpNN(GRStateSet& OStates,
                                   GRExprEngine& Eng,
                                   const GRState *St, Expr* Ex,





More information about the cfe-commits mailing list