Major refactoring/cleanup of GRExprEngine, ExplodedGraph, and BugReporter.

Bugs are now reported using a combination of "BugType" (previously
BugDescription) and Bug "BugReport" objects, which are fed to BugReporter (which
generates PathDiagnostics). This provides a far more modular way of registering
bug types and plugging in diagnostics.

GRExprEngine now owns its copy of GRCoreEngine, and is not owned by the
ExplodedGraph.

ExplodedGraph is no longer templated on the "checker", but instead on the state
contained in the nodes.


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

6 files changed, 128 insertions, 113 deletions

diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h
index 7ff79fc53d..9f8b17ac00 100644
--- a/include/clang/Analysis/PathSensitive/BugReporter.h
+++ b/include/clang/Analysis/PathSensitive/BugReporter.h
@@ -19,9 +19,6 @@
 #include "clang/Analysis/PathSensitive/ExplodedGraph.h"
 #include "llvm/ADT/SmallPtrSet.h"
 
-// FIXME: ExplodedGraph<> should be templated on state, not the checker engine.
-#include "clang/Analysis/PathSensitive/GRExprEngine.h"
-
 namespace clang {
   
 class PathDiagnostic;
@@ -29,61 +26,79 @@ class PathDiagnosticPiece;
 class PathDiagnosticClient;
 class ASTContext;
 class Diagnostic;
-
-class BugDescription {
+class BugReporter;
+class GRExprEngine;
+class ValueState;
+  
+class BugType {
 public:
-  BugDescription() {}
-  virtual ~BugDescription() {}
+  BugType() {}
+  virtual ~BugType();
   
   virtual const char* getName() const = 0;
+  virtual const char* getDescription() const { return getName(); }
+      
+  virtual void EmitWarnings(BugReporter& BR) {}
+};
   
-  virtual const char* getDescription() const = 0;
+class BugReport {
+  const BugType& Desc;  
+  
+public:
+  BugReport(const BugType& D) : Desc(D) {}
+  virtual ~BugReport();
+  
+  const BugType& getBugType() const { return Desc; }
+    
+  const char* getName() const { return getBugType().getName(); }
+  const char* getDescription() const { return getBugType().getDescription(); }
   
   virtual PathDiagnosticPiece* getEndPath(ASTContext& Ctx,
                                           ExplodedNode<ValueState> *N) const;
   
   virtual void getRanges(const SourceRange*& beg,
                          const SourceRange*& end) const;
-};
-  
-class BugReporterHelper {
-public:
-  virtual ~BugReporterHelper() {}
   
   virtual PathDiagnosticPiece* VisitNode(ExplodedNode<ValueState>* N,
                                          ExplodedNode<ValueState>* PrevN,
-                                         ExplodedGraph<GRExprEngine>& G,
-                                         ASTContext& Ctx) = 0;
+                                         ExplodedGraph<ValueState>& G,
+                                         ASTContext& Ctx);
 };
   
 class BugReporter {
   llvm::SmallPtrSet<void*,10> CachedErrors;
+  Diagnostic& Diag;
+  PathDiagnosticClient* PD;
+  ASTContext& Ctx;
+  GRExprEngine& Eng;
   
 public:
-  BugReporter() {}
+  BugReporter(Diagnostic& diag, PathDiagnosticClient* pd,
+              ASTContext& ctx, GRExprEngine& eng)
+  : Diag(diag), PD(pd), Ctx(ctx), Eng(eng) {}
+  
   ~BugReporter();
   
-  void EmitPathWarning(Diagnostic& Diag, PathDiagnosticClient* PDC,
-                       ASTContext& Ctx, const BugDescription& B,
-                       ExplodedGraph<GRExprEngine>& G,
-                       ExplodedNode<ValueState>* N,
-                       BugReporterHelper** BegHelpers = NULL,
-                       BugReporterHelper** EndHelpers = NULL);
+  Diagnostic& getDiagnostic() { return Diag; }
+  
+  PathDiagnosticClient* getDiagnosticClient() { return PD; }
   
-  void EmitWarning(Diagnostic& Diag, ASTContext& Ctx,
-                   const BugDescription& B,
-                   ExplodedNode<ValueState>* N);
+  ASTContext& getContext() { return Ctx; }
   
+  ExplodedGraph<ValueState>& getGraph();
 
-private:
+  GRExprEngine& getEngine() { return Eng; }
+  
+  void EmitPathWarning(BugReport& R, ExplodedNode<ValueState>* N);
+  
+  void EmitWarning(BugReport& R, ExplodedNode<ValueState>* N);
+  
+  void clearCache() { CachedErrors.clear(); }
+  
   bool IsCached(ExplodedNode<ValueState>* N);
   
-  void GeneratePathDiagnostic(PathDiagnostic& PD, ASTContext& Ctx,
-                              const BugDescription& B,
-                              ExplodedGraph<GRExprEngine>& G,
-                              ExplodedNode<ValueState>* N,
-                              BugReporterHelper** BegHelpers = NULL,
-                              BugReporterHelper** EndHelpers = NULL);
+  void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R,
+                              ExplodedNode<ValueState>* N);
 };
   
 } // end clang namespace
diff --git a/include/clang/Analysis/PathSensitive/ExplodedGraph.h b/include/clang/Analysis/PathSensitive/ExplodedGraph.h
index 30d95605ea..8db069639b 100644
--- a/include/clang/Analysis/PathSensitive/ExplodedGraph.h
+++ b/include/clang/Analysis/PathSensitive/ExplodedGraph.h
@@ -278,17 +278,14 @@ public:
   
 };
   
-template <typename CHECKER>
+template <typename STATE>
 class ExplodedGraph : public ExplodedGraphImpl {
 public:
-  typedef CHECKER                     CheckerTy;
-  typedef typename CHECKER::StateTy   StateTy;
+  typedef STATE                       StateTy;
   typedef ExplodedNode<StateTy>       NodeTy;  
   typedef llvm::FoldingSet<NodeTy>    AllNodesTy;
   
 protected:  
-  llvm::OwningPtr<CheckerTy> CheckerState;
-
   /// Nodes - The nodes in the graph.
   AllNodesTy Nodes;
   
@@ -305,12 +302,7 @@ protected:
     
 public:
   ExplodedGraph(CFG& c, Decl& cd, ASTContext& ctx)
-    : ExplodedGraphImpl(c, cd, ctx), CheckerState(new CheckerTy(*this)) {}
-  
-  /// getCheckerState - Returns the internal checker state associated
-  ///  with the exploded graph.  Ownership remains with the ExplodedGraph
-  ///  objecct.
-  CheckerTy* getCheckerState() const { return CheckerState.get(); }
+    : ExplodedGraphImpl(c, cd, ctx) {}
   
   /// getNode - Retrieve the node associated with a (Location,State) pair,
   ///  where the 'Location' is a ProgramPoint in the CFG.  If no node for
diff --git a/include/clang/Analysis/PathSensitive/GRCoreEngine.h b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
index 35fddadf33..5b7aeb5975 100644
--- a/include/clang/Analysis/PathSensitive/GRCoreEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
@@ -111,7 +111,7 @@ protected:
 public:
   /// ExecuteWorkList - Run the worklist algorithm for a maximum number of
   ///  steps.  Returns true if there is still simulation state on the worklist.
-  bool ExecuteWorkList(unsigned Steps = 1000000);
+  bool ExecuteWorkList(unsigned Steps);
   
   virtual ~GRCoreEngineImpl() {}
   
@@ -309,11 +309,10 @@ public:
   }
 };
 
-template<typename CHECKER>
+template<typename STATE>
 class GRBranchNodeBuilder {
-  typedef CHECKER                                CheckerTy; 
-  typedef typename CheckerTy::StateTy            StateTy;
-  typedef ExplodedGraph<CheckerTy>               GraphTy;
+  typedef STATE                                  StateTy;
+  typedef ExplodedGraph<StateTy>                 GraphTy;
   typedef typename GraphTy::NodeTy               NodeTy;
   
   GRBranchNodeBuilderImpl& NB;
@@ -392,11 +391,10 @@ public:
   inline void* getState() const { return Pred->State; }
 };
   
-template<typename CHECKER>
+template<typename STATE>
 class GRIndirectGotoNodeBuilder {
-  typedef CHECKER                                CheckerTy; 
-  typedef typename CheckerTy::StateTy            StateTy;
-  typedef ExplodedGraph<CheckerTy>               GraphTy;
+  typedef STATE                                  StateTy;
+  typedef ExplodedGraph<StateTy>                 GraphTy;
   typedef typename GraphTy::NodeTy               NodeTy;
 
   GRIndirectGotoNodeBuilderImpl& NB;
@@ -459,11 +457,10 @@ public:
   inline void* getState() const { return Pred->State; }
 };
 
-template<typename CHECKER>
+template<typename STATE>
 class GRSwitchNodeBuilder {
-  typedef CHECKER                                CheckerTy; 
-  typedef typename CheckerTy::StateTy            StateTy;
-  typedef ExplodedGraph<CheckerTy>               GraphTy;
+  typedef STATE                                  StateTy;
+  typedef ExplodedGraph<StateTy>                 GraphTy;
   typedef typename GraphTy::NodeTy               NodeTy;
   
   GRSwitchNodeBuilderImpl& NB;
@@ -492,21 +489,19 @@ public:
 };
 
   
-template<typename CHECKER>
+template<typename SUBENGINE>
 class GRCoreEngine : public GRCoreEngineImpl {
 public:
-  typedef CHECKER                                CheckerTy; 
-  typedef typename CheckerTy::StateTy            StateTy;
-  typedef ExplodedGraph<CheckerTy>               GraphTy;
+  typedef SUBENGINE                              SubEngineTy; 
+  typedef typename SubEngineTy::StateTy          StateTy;
+  typedef ExplodedGraph<StateTy>                 GraphTy;
   typedef typename GraphTy::NodeTy               NodeTy;
 
 protected:
-  // A local reference to the checker that avoids an indirect access
-  // via the Graph.
-  CheckerTy* Checker;  
+  SubEngineTy& SubEngine;
   
   virtual void* getInitialState() {
-    return getCheckerState().getInitialState();
+    return SubEngine.getInitialState();
   }
   
   virtual void* ProcessEOP(CFGBlock* Blk, void* State) {
@@ -516,44 +511,44 @@ protected:
   
   virtual void ProcessStmt(Stmt* S, GRStmtNodeBuilderImpl& BuilderImpl) {
     GRStmtNodeBuilder<StateTy> Builder(BuilderImpl);
-    Checker->ProcessStmt(S, Builder);
+    SubEngine.ProcessStmt(S, Builder);
   }
   
   virtual bool ProcessBlockEntrance(CFGBlock* Blk, void* State,
                                     GRBlockCounter BC) {    
-    return Checker->ProcessBlockEntrance(Blk,
-                                         static_cast<StateTy*>(State), BC);
+    return SubEngine.ProcessBlockEntrance(Blk, static_cast<StateTy*>(State),BC);
   }
 
   virtual void ProcessBranch(Expr* Condition, Stmt* Terminator,
                              GRBranchNodeBuilderImpl& BuilderImpl) {
-    GRBranchNodeBuilder<CHECKER> Builder(BuilderImpl);
-    Checker->ProcessBranch(Condition, Terminator, Builder);    
+    GRBranchNodeBuilder<StateTy> Builder(BuilderImpl);
+    SubEngine.ProcessBranch(Condition, Terminator, Builder);    
   }
   
   virtual void ProcessIndirectGoto(GRIndirectGotoNodeBuilderImpl& BuilderImpl) {
-    GRIndirectGotoNodeBuilder<CHECKER> Builder(BuilderImpl);
-    Checker->ProcessIndirectGoto(Builder);
+    GRIndirectGotoNodeBuilder<StateTy> Builder(BuilderImpl);
+    SubEngine.ProcessIndirectGoto(Builder);
   }
   
   virtual void ProcessSwitch(GRSwitchNodeBuilderImpl& BuilderImpl) {
-    GRSwitchNodeBuilder<CHECKER> Builder(BuilderImpl);
-    Checker->ProcessSwitch(Builder);
+    GRSwitchNodeBuilder<StateTy> Builder(BuilderImpl);
+    SubEngine.ProcessSwitch(Builder);
   }
   
 public:  
   /// Construct a GRCoreEngine object to analyze the provided CFG using
   ///  a DFS exploration of the exploded graph.
-  GRCoreEngine(CFG& cfg, Decl& cd, ASTContext& ctx)
+  GRCoreEngine(CFG& cfg, Decl& cd, ASTContext& ctx, SubEngineTy& subengine)
     : GRCoreEngineImpl(new GraphTy(cfg, cd, ctx), GRWorkList::MakeDFS()),
-      Checker(static_cast<GraphTy*>(G.get())->getCheckerState()) {}
+      SubEngine(subengine) {}
   
   /// Construct a GRCoreEngine object to analyze the provided CFG and to
   ///  use the provided worklist object to execute the worklist algorithm.
   ///  The GRCoreEngine object assumes ownership of 'wlist'.
-  GRCoreEngine(CFG& cfg, Decl& cd, ASTContext& ctx, GRWorkList* wlist)
+  GRCoreEngine(CFG& cfg, Decl& cd, ASTContext& ctx, GRWorkList* wlist,
+               SubEngineTy& subengine)
     : GRCoreEngineImpl(new GraphTy(cfg, cd, ctx), wlist),
-      Checker(static_cast<GraphTy*>(G.get())->getCheckerState()) {}
+      SubEngine(subengine) {}
   
   virtual ~GRCoreEngine() {}
   
@@ -562,11 +557,6 @@ public:
     return *static_cast<GraphTy*>(G.get());
   }
   
-  /// getCheckerState - Returns the internal checker state.
-  CheckerTy& getCheckerState() {
-    return *Checker;
-  }  
-  
   /// takeGraph - Returns the exploded graph.  Ownership of the graph is
   ///  transfered to the caller.
   GraphTy* takeGraph() { 
diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index e88b377a97..473b658b67 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -21,26 +21,30 @@
 #include "clang/Analysis/PathSensitive/GRSimpleAPICheck.h"
 #include "clang/Analysis/PathSensitive/GRTransferFuncs.h"
 
-namespace clang {
+namespace clang {  
   
-  class StatelessChecks;
+  class BugType;
+  class PathDiagnosticClient;
+  class Diagnostic;
   
 class GRExprEngine {
   
 public:
   typedef ValueState                  StateTy;
-  typedef ExplodedGraph<GRExprEngine> GraphTy;
+  typedef ExplodedGraph<StateTy>      GraphTy;
   typedef GraphTy::NodeTy             NodeTy;
   
   // Builders.
-  typedef GRStmtNodeBuilder<StateTy>               StmtNodeBuilder;
-  typedef GRBranchNodeBuilder<GRExprEngine>        BranchNodeBuilder;
-  typedef GRIndirectGotoNodeBuilder<GRExprEngine>  IndirectGotoNodeBuilder;
-  typedef GRSwitchNodeBuilder<GRExprEngine>        SwitchNodeBuilder;
-  typedef ExplodedNodeSet<StateTy>                 NodeSet;
+  typedef GRStmtNodeBuilder<StateTy>          StmtNodeBuilder;
+  typedef GRBranchNodeBuilder<StateTy>        BranchNodeBuilder;
+  typedef GRIndirectGotoNodeBuilder<StateTy>  IndirectGotoNodeBuilder;
+  typedef GRSwitchNodeBuilder<StateTy>        SwitchNodeBuilder;
+  typedef ExplodedNodeSet<StateTy>            NodeSet;
   
     
 protected:
+  GRCoreEngine<GRExprEngine> CoreEngine;
+  
   /// G - the simulation graph.
   GraphTy& G;
   
@@ -62,6 +66,10 @@ protected:
   ///  for manipulating and creating RVals.
   GRTransferFuncs* TF;
   
+  /// BugTypes - Objects used for reporting bugs.
+  typedef std::vector<BugType*> BugTypeSet;
+  BugTypeSet BugTypes;
+  
   /// SymMgr - Object that manages the symbol information.
   SymbolManager& SymMgr;
   
@@ -154,18 +162,11 @@ protected:
   UndefArgsTy MsgExprUndefArgs;
   
 public:
-  GRExprEngine(GraphTy& g) : 
-    G(g), Liveness(G.getCFG()),
-    Builder(NULL),
-    StateMgr(G.getContext(), G.getAllocator()),
-    BasicVals(StateMgr.getBasicValueFactory()),
-    TF(NULL), // FIXME.
-    SymMgr(StateMgr.getSymbolManager()),
-    StmtEntryNode(NULL), CleanedState(NULL), CurrentStmt(NULL) {
-    
-    // Compute liveness information.
-    Liveness.runOnCFG(G.getCFG());
-    Liveness.runOnAllBlocks(G.getCFG(), NULL, true);
+  GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx);
+  ~GRExprEngine();
+  
+  void ExecuteWorkList(unsigned Steps = 150000) {
+    CoreEngine.ExecuteWorkList(Steps);
   }
   
   /// getContext - Return the ASTContext associated with this analysis.
@@ -175,8 +176,11 @@ public:
   CFG& getCFG() { return G.getCFG(); }
   
   /// setTransferFunctions
-  void setTransferFunctions(GRTransferFuncs* tf) { TF = tf; }
-  void setTransferFunctions(GRTransferFuncs& tf) { TF = &tf; }
+  void setTransferFunctions(GRTransferFuncs* tf);
+
+  void setTransferFunctions(GRTransferFuncs& tf) {
+    setTransferFunctions(&tf);
+  }
   
   /// ViewGraph - Visualize the ExplodedGraph created by executing the
   ///  simulation.
@@ -188,6 +192,24 @@ public:
   ///  in the ExplodedGraph.
   ValueState* getInitialState();
   
+  GraphTy& getGraph() { return G; }
+  const GraphTy& getGraph() const { return G; }
+  
+  typedef BugTypeSet::iterator bug_type_iterator;
+  typedef BugTypeSet::const_iterator const_bug_type_iterator;
+  
+  bug_type_iterator bug_types_begin() { return BugTypes.begin(); }
+  bug_type_iterator bug_types_end() { return BugTypes.end(); }
+
+  const_bug_type_iterator bug_types_begin() const { return BugTypes.begin(); }
+  const_bug_type_iterator bug_types_end() const { return BugTypes.end(); }
+  
+  void Register(BugType* B) {
+    BugTypes.push_back(B);
+  }
+  
+  void EmitWarnings(Diagnostic& Diag, PathDiagnosticClient* PD);  
+  
   bool isRetStackAddr(const NodeTy* N) const {
     return N->isSink() && RetsStackAddr.count(const_cast<NodeTy*>(N)) != 0;
   }
@@ -317,13 +339,9 @@ public:
     return MsgExprChecks.end();
   }
   
-  void AddCallCheck(GRSimpleAPICheck* A) {
-    CallChecks.push_back(A);
-  }
+  void AddCallCheck(GRSimpleAPICheck* A);
   
-  void AddObjCMessageExprCheck(GRSimpleAPICheck* A) {
-    MsgExprChecks.push_back(A);
-  }
+  void AddObjCMessageExprCheck(GRSimpleAPICheck* A);
   
   /// ProcessStmt - Called by GRCoreEngine. Used to generate new successor
   ///  nodes by processing the 'effects' of a block-level statement.  
diff --git a/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h b/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h
index 16b23c558c..782ddcfffe 100644
--- a/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h
+++ b/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h
@@ -33,9 +33,7 @@ class GRSimpleAPICheck : public GRAuditor<ValueState> {
 public:
   GRSimpleAPICheck() {}
   virtual ~GRSimpleAPICheck() {}
-  virtual void ReportResults(Diagnostic& Diag, PathDiagnosticClient* PD,
-                             ASTContext& Ctx, BugReporter& BR,
-                             ExplodedGraph<GRExprEngine>& G) = 0;
+  virtual void EmitWarnings(BugReporter& BR) = 0;
 };
 
 } // end namespace clang
diff --git a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
index 55a0fae273..9c68f20e74 100644
--- a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
+++ b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
@@ -32,6 +32,8 @@ public:
     return NULL;
   }
   
+  virtual void RegisterChecks(GRExprEngine& Eng) {}
+  
   // Casts.
   
   virtual RVal EvalCast(GRExprEngine& Engine, NonLVal V, QualType CastT) =0;