[cfe-commits] r47851 - in /cfe/trunk: Analysis/ExplodedGraph.cpp include/clang/Analysis/PathSensitive/ExplodedGraph.h include/clang/Analysis/ProgramPoint.h

[Ted Kremenek]

Author: kremenek
Date: Mon Mar  3 13:29:58 2008
New Revision: 47851

URL: http://llvm.org/viewvc/llvm-project?rev=47851&view=rev
Log:
Fixed subtle caching bug in ExplodedGraph that would cause some nodes to
be incorrectly merged together.

Modified:
    cfe/trunk/Analysis/ExplodedGraph.cpp
    cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h
    cfe/trunk/include/clang/Analysis/ProgramPoint.h

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

==============================================================================
--- cfe/trunk/Analysis/ExplodedGraph.cpp (original)
+++ cfe/trunk/Analysis/ExplodedGraph.cpp Mon Mar  3 13:29:58 2008
@@ -68,19 +68,3 @@
 ExplodedNodeImpl::NodeGroup::~NodeGroup() {
   if (getKind() == SizeOther) delete &getVector(getPtr());
 }
-
-
-ExplodedGraphImpl::~ExplodedGraphImpl() {
-  // Delete the FoldingSet's in Nodes.  Note that the contents
-  // of the FoldingSets are nodes allocated from the BumpPtrAllocator,
-  // so all of those will get nuked when that object is destroyed.
-  for (EdgeNodeSetMap::iterator I=Nodes.begin(), E=Nodes.end(); I!=E; ++I) {
-    llvm::FoldingSet<ExplodedNodeImpl>* ENodes = I->second;
-    
-    for (llvm::FoldingSet<ExplodedNodeImpl>::iterator
-         I=ENodes->begin(), E=ENodes->end(); I!=E; ++I)
-      (*I).~ExplodedNodeImpl();
-    
-    delete ENodes;
-  }
-}

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=47851&r1=47850&r2=47851&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h Mon Mar  3 13:29:58 2008
@@ -203,10 +203,6 @@
   friend class GRSwitchNodeBuilderImpl;
   
   // Type definitions.
-  typedef llvm::DenseMap<ProgramPoint, 
-                         llvm::FoldingSet<clang::ExplodedNodeImpl>*> 
-          EdgeNodeSetMap;
-  
   typedef llvm::SmallVector<ExplodedNodeImpl*,2>    RootsTy;
   typedef llvm::SmallVector<ExplodedNodeImpl*,10>   EndNodesTy;
     
@@ -219,9 +215,6 @@
   /// EndNodes - The nodes in the simulation graph which have been
   ///  specially marked as the endpoint of an abstract simulation path.
   EndNodesTy EndNodes;
-    
-  /// Nodes - A mapping from edges to nodes.
-  EdgeNodeSetMap Nodes;
   
   /// Allocator - BumpPtrAllocator to create nodes.
   llvm::BumpPtrAllocator Allocator;
@@ -261,7 +254,7 @@
     : cfg(c), FD(f), Ctx(ctx), NumNodes(0) {}
 
 public:
-  virtual ~ExplodedGraphImpl();
+  virtual ~ExplodedGraphImpl() {}
 
   unsigned num_roots() const { return Roots.size(); }
   unsigned num_eops() const { return EndNodes.size(); }
@@ -280,11 +273,15 @@
 public:
   typedef CHECKER                     CheckerTy;
   typedef typename CHECKER::StateTy   StateTy;
-  typedef ExplodedNode<StateTy>       NodeTy;
+  typedef ExplodedNode<StateTy>       NodeTy;  
+  typedef llvm::FoldingSet<NodeTy>    AllNodesTy;
   
 protected:  
   llvm::OwningPtr<CheckerTy> CheckerState;
 
+  /// Nodes - The nodes in the graph.
+  AllNodesTy Nodes;
+  
 protected:
   virtual ExplodedNodeImpl*
   getNodeImpl(const ProgramPoint& L, void* State, bool* IsNew) {
@@ -306,19 +303,13 @@
   ///  the node was freshly created.
   NodeTy* getNode(const ProgramPoint& L, StateTy State, bool* IsNew = NULL) {
     
-    // Retrieve the node set associated with Loc.
-    llvm::FoldingSet<NodeTy>*& VSet =
-       reinterpret_cast<llvm::FoldingSet<NodeTy>*&>(Nodes[L]);
-    
-    // Create the FoldingSet for the nodes if it does not exist yet.
-    if (!VSet) VSet = new llvm::FoldingSet<NodeTy>();
-    
     // Profile 'State' to determine if we already have an existing node.
     llvm::FoldingSetNodeID profile;    
     void* InsertPos = 0;
     
     GRTrait<StateTy>::Profile(profile, State);
-    NodeTy* V = VSet->FindNodeOrInsertPos(profile, InsertPos);
+    profile.Add(L);
+    NodeTy* V = Nodes.FindNodeOrInsertPos(profile, InsertPos);
 
     if (!V) {
       // Allocate a new node.
@@ -326,7 +317,7 @@
       new (V) NodeTy(L, State);
       
       // Insert the node into the node set and return it.
-      VSet->InsertNode(V, InsertPos);
+      Nodes.InsertNode(V, InsertPos);
       
       ++NumNodes;
       

Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=47851&r1=47850&r2=47851&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
+++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Mon Mar  3 13:29:58 2008
@@ -18,6 +18,7 @@
 #include "clang/AST/CFG.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/FoldingSet.h"
 #include <cassert>
 
 namespace clang {
@@ -45,7 +46,12 @@
   
   static bool classof(const ProgramPoint*) { return true; }
   bool operator==(const ProgramPoint & RHS) const { return Data == RHS.Data; }
-  bool operator!=(const ProgramPoint& RHS) const { return Data != RHS.Data; }  
+  bool operator!=(const ProgramPoint& RHS) const { return Data != RHS.Data; }
+  
+  void Profile(llvm::FoldingSetNodeID& ID) const {
+    ID.AddInteger(getKind());
+    ID.AddPointer(getRawPtr());
+  }    
 };
                
 class BlockEntrance : public ProgramPoint {