From: Ted Kremenek <kremenek@apple.com>
Date: Mon, 3 Mar 2008 19:29:58 +0000 (+0000)
Subject: Fixed subtle caching bug in ExplodedGraph that would cause some nodes to
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5226755ab5ce6346f98b5f41cdcffbe84c5bb484;p=clang

Fixed subtle caching bug in ExplodedGraph that would cause some nodes to
be incorrectly merged together.


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

diff --git a/Analysis/ExplodedGraph.cpp b/Analysis/ExplodedGraph.cpp
index ca5842656f..69d190d09f 100644
--- a/Analysis/ExplodedGraph.cpp
+++ b/Analysis/ExplodedGraph.cpp
@@ -68,19 +68,3 @@ ExplodedNodeImpl** ExplodedNodeImpl::NodeGroup::end() const {
 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;
-  }
-}
diff --git a/include/clang/Analysis/PathSensitive/ExplodedGraph.h b/include/clang/Analysis/PathSensitive/ExplodedGraph.h
index dfdfa5b780..a56e7d3d14 100644
--- a/include/clang/Analysis/PathSensitive/ExplodedGraph.h
+++ b/include/clang/Analysis/PathSensitive/ExplodedGraph.h
@@ -203,10 +203,6 @@ protected:
   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 @@ protected:
   /// 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 @@ protected:
     : 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 @@ class ExplodedGraph : public ExplodedGraphImpl {
 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 @@ public:
   ///  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 @@ public:
       new (V) NodeTy(L, State);
       
       // Insert the node into the node set and return it.
-      VSet->InsertNode(V, InsertPos);
+      Nodes.InsertNode(V, InsertPos);
       
       ++NumNodes;
       
diff --git a/include/clang/Analysis/ProgramPoint.h b/include/clang/Analysis/ProgramPoint.h
index 45ec936e9a..a046268cfd 100644
--- a/include/clang/Analysis/ProgramPoint.h
+++ b/include/clang/Analysis/ProgramPoint.h
@@ -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 @@ public:
   
   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 {