[cfe-commits] r48036 - in /cfe/trunk: Analysis/ExplodedGraph.cpp Analysis/GRExprEngine.cpp Analysis/GRSimpleVals.cpp Driver/ASTConsumers.cpp Driver/ASTConsumers.h Driver/clang.cpp include/clang/Analysis/Analyses/GRSimpleVals.h include/clang/Analysis/PathSensitive/ExplodedGraph.h include/clang/Analysis/PathSensitive/GRExprEngine.h

Ted Kremenek kremenek at apple.com
Fri Mar 7 14:58:01 PST 2008


Author: kremenek
Date: Fri Mar  7 16:58:01 2008
New Revision: 48036

URL: http://llvm.org/viewvc/llvm-project?rev=48036&view=rev
Log:
Added --trim-path-graph to the driver to trim paths from the ExplodedGraph
that are not related to error nodes.

Fixed bug where we did not detect some NULL dereferences.

Added "ExplodedGraph::Trim" to trim all nodes that cannot transitively reach
a set of provided nodes.

Fixed subtle bug in ExplodedNodeImpl where we could create predecessor
iterators that included the mangled "sink" bit.  The better fix is to integrate
this bit into the void* for the wrapped State, not the NodeGroups representing
a node's predecessors and successors.

Modified:
    cfe/trunk/Analysis/ExplodedGraph.cpp
    cfe/trunk/Analysis/GRExprEngine.cpp
    cfe/trunk/Analysis/GRSimpleVals.cpp
    cfe/trunk/Driver/ASTConsumers.cpp
    cfe/trunk/Driver/ASTConsumers.h
    cfe/trunk/Driver/clang.cpp
    cfe/trunk/include/clang/Analysis/Analyses/GRSimpleVals.h
    cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h
    cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h

Modified: cfe/trunk/Analysis/ExplodedGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Analysis/ExplodedGraph.cpp?rev=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/Analysis/ExplodedGraph.cpp (original)
+++ cfe/trunk/Analysis/ExplodedGraph.cpp Fri Mar  7 16:58:01 2008
@@ -13,6 +13,9 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/PathSensitive/ExplodedGraph.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
 #include <vector>
 
 using namespace clang;
@@ -25,6 +28,7 @@
 void ExplodedNodeImpl::NodeGroup::addNode(ExplodedNodeImpl* N) {
   
   assert ((reinterpret_cast<uintptr_t>(N) & Mask) == 0x0);
+  assert (!getFlag());
   
   if (getKind() == Size1) {
     if (ExplodedNodeImpl* NOld = getNode()) {
@@ -47,14 +51,11 @@
   }
 }
 
-bool ExplodedNodeImpl::NodeGroup::empty() const {
-  if (getKind() == Size1)
-    return getNode() ? false : true;
-  else
-    return getVector(getPtr()).empty();
-}
 
 unsigned ExplodedNodeImpl::NodeGroup::size() const {
+  if (getFlag())
+    return 0;
+  
   if (getKind() == Size1)
     return getNode() ? 1 : 0;
   else
@@ -62,15 +63,21 @@
 }
 
 ExplodedNodeImpl** ExplodedNodeImpl::NodeGroup::begin() const {
+  if (getFlag())
+    return NULL;
+  
   if (getKind() == Size1)
-    return (ExplodedNodeImpl**) &P;
+    return (ExplodedNodeImpl**) (getPtr() ? &P : NULL);
   else
     return const_cast<ExplodedNodeImpl**>(&*(getVector(getPtr()).begin()));
 }
 
 ExplodedNodeImpl** ExplodedNodeImpl::NodeGroup::end() const {
+  if (getFlag())
+    return NULL;
+  
   if (getKind() == Size1)
-    return (ExplodedNodeImpl**) (P ? &P+1 : &P);
+    return (ExplodedNodeImpl**) (getPtr() ? &P+1 : NULL);
   else
     return const_cast<ExplodedNodeImpl**>(&*(getVector(getPtr()).end()));
 }
@@ -78,3 +85,110 @@
 ExplodedNodeImpl::NodeGroup::~NodeGroup() {
   if (getKind() == SizeOther) delete &getVector(getPtr());
 }
+
+ExplodedGraphImpl* ExplodedGraphImpl::Trim(ExplodedNodeImpl** BeginSources,
+                                           ExplodedNodeImpl** EndSources) const{
+  
+  typedef llvm::DenseSet<ExplodedNodeImpl*> Pass1Ty;
+  typedef llvm::DenseMap<ExplodedNodeImpl*, ExplodedNodeImpl*> Pass2Ty;
+  
+  Pass1Ty Pass1;
+  Pass2Ty Pass2;
+  
+  llvm::SmallVector<ExplodedNodeImpl*, 10> WL2;
+
+  { // ===- Pass 1 (reverse BFS) -===
+    
+    // Enqueue the source nodes to the first worklist. 
+    
+    llvm::SmallVector<ExplodedNodeImpl*, 10> WL1;
+  
+    for (ExplodedNodeImpl** I = BeginSources; I != EndSources; ++I)
+      WL1.push_back(*I);
+    
+    // Process the worklist.
+
+    while (!WL1.empty()) {
+      
+      ExplodedNodeImpl* N = WL1.back();
+      WL1.pop_back();
+      
+      if (Pass1.count(N))
+        continue;
+      
+      Pass1.insert(N);
+      
+      if (N->Preds.empty()) {
+        WL2.push_back(N);
+        continue;      
+      }
+      
+      for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I)
+        WL1.push_back(*I);
+    }
+  }
+  
+  if (WL2.empty())
+    return NULL;
+    
+  ExplodedGraphImpl* G = MakeEmptyGraph();
+  
+  // ===- Pass 2 (forward DFS to construct the new graph) -===
+  
+  while (!WL2.empty()) {
+    
+    ExplodedNodeImpl* N = WL2.back();
+    WL2.pop_back();
+    
+    // Skip this node if we have already processed it.
+    
+    if (Pass2.find(N) != Pass2.end())
+      continue;
+    
+    // Create the corresponding node in the new graph.
+    
+    ExplodedNodeImpl* NewN = G->getNodeImpl(N->getLocation(), N->State, NULL);
+    Pass2[N] = NewN;
+    
+    if (N->Preds.empty())
+      G->addRoot(NewN);
+    
+    // In the case that some of the intended predecessors of NewN have already
+    // been created, we should hook them up as predecessors.
+    
+    for (ExplodedNodeImpl **I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I) {
+        
+      Pass2Ty::iterator PI = Pass2.find(*I);
+
+      if (PI == Pass2.end())
+        continue;
+      
+      NewN->addPredecessor(PI->second);
+    }
+
+    // In the case that some of the intended successors of NewN have already
+    // been created, we should hook them up as successors.  Otherwise, enqueue
+    // the new nodes from the original graph that should have nodes created
+    // in the new graph.
+   
+    for (ExplodedNodeImpl **I=N->Succs.begin(), **E=N->Succs.end(); I!=E; ++I) {
+      
+      Pass2Ty::iterator PI = Pass2.find(*I);
+      
+      if (PI != Pass2.end()) {
+        PI->second->addPredecessor(NewN);
+        continue;
+      }
+
+      // Enqueue nodes to the worklist that were marked during pass 1.
+      
+      if (Pass1.count(*I))
+        WL2.push_back(*I);
+    }
+    
+    if (N->isSink())
+      NewN->markAsSink();
+  }
+    
+  return G;
+}

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

==============================================================================
--- cfe/trunk/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/Analysis/GRExprEngine.cpp Fri Mar  7 16:58:01 2008
@@ -732,7 +732,8 @@
   
 }
 
-void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst) {
+void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred,
+                              NodeSet& Dst, bool GetLVal) {
 
   Expr* Ex = U->getSubExpr()->IgnoreParens();
     
@@ -787,12 +788,16 @@
     ValueState* StNotNull = Assume(St, LV, true, isFeasibleNotNull);
     
     if (isFeasibleNotNull) {
+
+      if (GetLVal) Nodify(Dst, U, N, SetRVal(StNotNull, U, LV));
+      else {
+        
+        // FIXME: Currently symbolic analysis "generates" new symbols
+        //  for the contents of values.  We need a better approach.
       
-      // FIXME: Currently symbolic analysis "generates" new symbols
-      //  for the contents of values.  We need a better approach.
-      
-      Nodify(Dst, U, N, SetRVal(StNotNull, U,
-                                GetRVal(StNotNull, LV, U->getType())));
+        Nodify(Dst, U, N, SetRVal(StNotNull, U,
+                                  GetRVal(StNotNull, LV, U->getType())));
+      }
     }
     
     bool isFeasibleNull;
@@ -975,18 +980,11 @@
     return;
   }
   
-  if (UnaryOperator* U = dyn_cast<UnaryOperator>(Ex)) {
+  if (UnaryOperator* U = dyn_cast<UnaryOperator>(Ex))
     if (U->getOpcode() == UnaryOperator::Deref) {
-      Ex = U->getSubExpr()->IgnoreParens();
-      
-      if (isa<DeclRefExpr>(Ex))
-        Dst.Add(Pred);
-      else
-        Visit(Ex, Pred, Dst);
-      
+      VisitDeref(U, Pred, Dst, true);
       return;
     }
-  }
   
   Visit(Ex, Pred, Dst);
 }
@@ -1810,11 +1808,40 @@
 } // end llvm namespace    
 #endif
 
-void GRExprEngine::ViewGraph() {
+#ifndef NDEBUG
+template <typename ITERATOR>
+static void AddSources(llvm::SmallVector<GRExprEngine::NodeTy*, 10>& Sources,
+                              ITERATOR I, ITERATOR E) {
+  
+  for ( ; I != E; ++I )
+    Sources.push_back(*I);
+}
+#endif
+
+void GRExprEngine::ViewGraph(bool trim) {
 #ifndef NDEBUG
   GraphPrintCheckerState = this;
   GraphPrintSourceManager = &getContext().getSourceManager();
-  llvm::ViewGraph(*G.roots_begin(), "GRExprEngine");
+  
+  if (trim) {
+    llvm::SmallVector<NodeTy*, 10> Sources;
+    AddSources(Sources, null_derefs_begin(), null_derefs_end());
+    AddSources(Sources, undef_derefs_begin(), undef_derefs_end());
+    
+    GRExprEngine::GraphTy* TrimmedG = G.Trim(&Sources[0],
+                                             &Sources[0]+Sources.size());
+    
+    if (!TrimmedG)
+      llvm::cerr << "warning: Trimmed ExplodedGraph is empty.\n";
+    else {
+      llvm::ViewGraph(*TrimmedG->roots_begin(), "TrimmedGRExprEngine");    
+      delete TrimmedG;
+    }
+  }
+  else
+    llvm::ViewGraph(*G.roots_begin(), "GRExprEngine");
+  
+  
   GraphPrintCheckerState = NULL;
   GraphPrintSourceManager = NULL;
 #endif

Modified: cfe/trunk/Analysis/GRSimpleVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Analysis/GRSimpleVals.cpp?rev=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/Analysis/GRSimpleVals.cpp (original)
+++ cfe/trunk/Analysis/GRSimpleVals.cpp Fri Mar  7 16:58:01 2008
@@ -90,7 +90,7 @@
 }
   
 unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx,
-                         Diagnostic& Diag, bool Visualize) {
+                         Diagnostic& Diag, bool Visualize, bool TrimGraph) {
   
   if (Diag.hasErrorOccurred())
     return 0;
@@ -141,7 +141,7 @@
       "Pass-by-value argument in function or message expression is undefined.");
       
 #ifndef NDEBUG
-  if (Visualize) CheckerState->ViewGraph();
+  if (Visualize) CheckerState->ViewGraph(TrimGraph);
 #endif
   
   return Engine.getGraph().size();

Modified: cfe/trunk/Driver/ASTConsumers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Driver/ASTConsumers.cpp?rev=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/Driver/ASTConsumers.cpp (original)
+++ cfe/trunk/Driver/ASTConsumers.cpp Fri Mar  7 16:58:01 2008
@@ -593,10 +593,11 @@
     Diagnostic &Diags;
     ASTContext* Ctx;
     bool Visualize;
+    bool TrimGraph;
   public:
     GRSimpleValsVisitor(Diagnostic &diags, const std::string& fname,
-                        bool visualize)
-      : CFGVisitor(fname), Diags(diags), Visualize(visualize) {}
+                        bool visualize, bool trim)
+      : CFGVisitor(fname), Diags(diags), Visualize(visualize), TrimGraph(trim){}
     
     virtual void Initialize(ASTContext &Context) { Ctx = &Context; }    
     virtual void VisitCFG(CFG& C, FunctionDecl&);
@@ -606,9 +607,9 @@
 
 ASTConsumer* clang::CreateGRSimpleVals(Diagnostic &Diags,
                                        const std::string& FunctionName,
-                                       bool Visualize) {
+                                       bool Visualize, bool TrimGraph) {
   
-  return new GRSimpleValsVisitor(Diags, FunctionName, Visualize);
+  return new GRSimpleValsVisitor(Diags, FunctionName, Visualize, TrimGraph);
 }
 
 void GRSimpleValsVisitor::VisitCFG(CFG& C, FunctionDecl& FD) {
@@ -626,13 +627,13 @@
 
     llvm::Timer T("GRSimpleVals");
     T.startTimer();
-    unsigned size = RunGRSimpleVals(C, FD, *Ctx, Diags, Visualize);
+    unsigned size = RunGRSimpleVals(C, FD, *Ctx, Diags, false, false);
     T.stopTimer();    
     llvm::cerr << size << ' ' << T.getWallTime() << '\n';
   }
   else {  
     llvm::cerr << '\n';    
-    RunGRSimpleVals(C, FD, *Ctx, Diags, Visualize);
+    RunGRSimpleVals(C, FD, *Ctx, Diags, Visualize, TrimGraph);
   }    
 }
 

Modified: cfe/trunk/Driver/ASTConsumers.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Driver/ASTConsumers.h?rev=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/Driver/ASTConsumers.h (original)
+++ cfe/trunk/Driver/ASTConsumers.h Fri Mar  7 16:58:01 2008
@@ -44,7 +44,7 @@
   
 ASTConsumer *CreateGRSimpleVals(Diagnostic &Diags,
                                 const std::string& Function,
-                                bool Visualize = false);
+                                bool Visualize = false, bool TrimGraph = false);
   
 ASTConsumer* CreateCFRefChecker(Diagnostic &Diags,
                                 const std::string& FunctionName); 

Modified: cfe/trunk/Driver/clang.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Driver/clang.cpp?rev=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/Driver/clang.cpp (original)
+++ cfe/trunk/Driver/clang.cpp Fri Mar  7 16:58:01 2008
@@ -461,6 +461,11 @@
 AnalyzeSpecificFunction("analyze-function",
                 llvm::cl::desc("Run analysis on specific function."));
 
+static llvm::cl::opt<bool>
+TrimGraph("trim-path-graph",
+      llvm::cl::desc("Only show error-related paths in the analysis graph."));
+
+
 //===----------------------------------------------------------------------===//
 // Target Triple Processing.
 //===----------------------------------------------------------------------===//
@@ -1039,7 +1044,7 @@
       return CreateGRSimpleVals(Diag, AnalyzeSpecificFunction);
       
     case AnalysisGRSimpleValsView:
-      return CreateGRSimpleVals(Diag, AnalyzeSpecificFunction, true);
+      return CreateGRSimpleVals(Diag, AnalyzeSpecificFunction, true, TrimGraph);
       
     case CheckerCFRef:
       return CreateCFRefChecker(Diag, AnalyzeSpecificFunction);

Modified: cfe/trunk/include/clang/Analysis/Analyses/GRSimpleVals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/GRSimpleVals.h?rev=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/GRSimpleVals.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/GRSimpleVals.h Fri Mar  7 16:58:01 2008
@@ -25,7 +25,7 @@
   ///  something more elaborate as the requirements on the interface become
   ///  clearer.  The value returned is the number of nodes in the ExplodedGraph.
   unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx,
-                           Diagnostic& Diag, bool Visualize);
+                           Diagnostic& Diag, bool Visualize, bool TrimGraph);
   
 } // end clang namespace
 

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

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h Fri Mar  7 16:58:01 2008
@@ -55,6 +55,7 @@
     }
     
     void* getPtr() const {
+      assert (!getFlag());
       return reinterpret_cast<void*>(P & ~Mask);
     }
 
@@ -73,12 +74,13 @@
     
     unsigned size() const;
     
-    bool empty() const;
+    bool empty() const { return size() == 0; }
     
     void addNode(ExplodedNodeImpl* N);
     
     void setFlag() {
-      P |= AuxFlag;
+      assert (P == 0);
+      P = AuxFlag;
     }
     
     bool getFlag() const {
@@ -123,8 +125,8 @@
   bool succ_empty() const { return Succs.empty(); }
   bool pred_empty() const { return Preds.size(); }
   
-  bool isSink() const { return Preds.getFlag(); }
-  void markAsSink() { Preds.setFlag(); }  
+  bool isSink() const { return Succs.getFlag(); }
+  void markAsSink() { Succs.setFlag(); }  
 };
 
 
@@ -232,6 +234,8 @@
   ///  is intended to be used only by GRCoreEngineImpl.
   virtual ExplodedNodeImpl* getNodeImpl(const ProgramPoint& L, void* State,
                                         bool* IsNew) = 0;
+  
+  virtual ExplodedGraphImpl* MakeEmptyGraph() const = 0;
 
   /// addRoot - Add an untyped node to the set of roots.
   ExplodedNodeImpl* addRoot(ExplodedNodeImpl* V) {
@@ -262,6 +266,10 @@
   CFG& getCFG() { return cfg; }
   ASTContext& getContext() { return Ctx; }
   FunctionDecl& getFunctionDecl() { return FD; }
+  
+  ExplodedGraphImpl* Trim(ExplodedNodeImpl** NBeg,
+                          ExplodedNodeImpl** NEnd) const;
+  
 };
   
 template <typename CHECKER>
@@ -279,10 +287,15 @@
   AllNodesTy Nodes;
   
 protected:
-  virtual ExplodedNodeImpl*
-  getNodeImpl(const ProgramPoint& L, void* State, bool* IsNew) {
+  virtual ExplodedNodeImpl* getNodeImpl(const ProgramPoint& L,
+                                        void* State, bool* IsNew) {
+    
     return getNode(L, static_cast<StateTy*>(State), IsNew);
   }
+  
+  virtual ExplodedGraphImpl* MakeEmptyGraph() const {
+    return new ExplodedGraph(cfg, FD, Ctx);
+  }  
     
 public:
   ExplodedGraph(CFG& c, FunctionDecl& fd, ASTContext& ctx)
@@ -362,6 +375,22 @@
   const_eop_iterator eop_end() const {
     return const_cast<ExplodedGraph>(this)->eop_end();
   }
+  
+  // Utility.
+  
+  ExplodedGraph* Trim(NodeTy** NBeg, NodeTy** NEnd) const {
+    
+    if (NBeg == NEnd)
+      return NULL;
+    
+    assert (NBeg < NEnd);
+    
+    ExplodedNodeImpl** NBegImpl = (ExplodedNodeImpl**) NBeg;
+    ExplodedNodeImpl** NEndImpl = (ExplodedNodeImpl**) NEnd;
+    
+    return static_cast<ExplodedGraph*>(ExplodedGraphImpl::Trim(NBegImpl,
+                                                               NEndImpl));
+  }
 };
   
   

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=48036&r1=48035&r2=48036&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Fri Mar  7 16:58:01 2008
@@ -149,7 +149,7 @@
   
   /// ViewGraph - Visualize the ExplodedGraph created by executing the
   ///  simulation.
-  void ViewGraph();
+  void ViewGraph(bool trim = false);
   
   /// getInitialState - Return the initial state used for the root vertex
   ///  in the ExplodedGraph.
@@ -392,7 +392,8 @@
   /// VisitUnaryOperator - Transfer function logic for unary operators.
   void VisitUnaryOperator(UnaryOperator* B, NodeTy* Pred, NodeSet& Dst);
   
-  void VisitDeref(UnaryOperator* B, NodeTy* Pred, NodeSet& Dst);
+  void VisitDeref(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst,
+                  bool GetLVal = false);
   
   RVal EvalCast(RVal X, QualType CastT) {
     if (X.isUnknownOrUndef())





More information about the cfe-commits mailing list