Refactored auditor interface within GRExprEngine and GRCoreEngine to use a "batch auditor" to dispatch to specialized auditors instead of having a separate vector for each audited Expr*. This not only provides a much cleaner implementation, but also allows us to install auditors for any expression.

llvm-svn: 53464

clang/include/clang/Analysis/PathSensitive/GRCoreEngine.h

@@ -165,36 +165,26 @@ public:
   
 template<typename STATE>
 class GRStmtNodeBuilder  {
+public:
   typedef STATE                   StateTy;
   typedef ExplodedNode<StateTy>   NodeTy;
   
+private:
   GRStmtNodeBuilderImpl& NB;
-  const StateTy* CleanedState;
-  
-  GRAuditor<StateTy> **CallExprAuditBeg, **CallExprAuditEnd;
-  GRAuditor<StateTy> **ObjCMsgExprAuditBeg, **ObjCMsgExprAuditEnd;
+  const StateTy* CleanedState;  
+  GRAuditor<StateTy>* Auditor;
   
 public:
   GRStmtNodeBuilder(GRStmtNodeBuilderImpl& nb) : NB(nb),
-    CallExprAuditBeg(0), CallExprAuditEnd(0),
-    ObjCMsgExprAuditBeg(0), ObjCMsgExprAuditEnd(0),
-    PurgingDeadSymbols(false),
+    Auditor(0), PurgingDeadSymbols(false),
     BuildSinks(false), HasGeneratedNode(false) {
       
     CleanedState = getLastNode()->getState();
   }
-  
-  void setObjCMsgExprAuditors(GRAuditor<StateTy> **B,
-                              GRAuditor<StateTy> **E) {
-    ObjCMsgExprAuditBeg = B;
-    ObjCMsgExprAuditEnd = E;
+
+  void setAuditor(GRAuditor<StateTy>* A) {
+    Auditor = A;
   }
-  
-  void setCallExprAuditors(GRAuditor<StateTy> **B,
-                           GRAuditor<StateTy> **E) {
-    CallExprAuditBeg = B;
-    CallExprAuditEnd = E;
-  }  
     
   NodeTy* getLastNode() const {
     return static_cast<NodeTy*>(NB.getLastNode());
@@ -241,23 +231,9 @@ public:
                    NodeTy* Pred, const StateTy* St) {    
     
     const StateTy* PredState = GetState(Pred);
-    
-    GRAuditor<StateTy> **AB = NULL, **AE = NULL;
-    
-    switch (S->getStmtClass()) {
-      default: break;
-      case Stmt::CallExprClass:
-        AB = CallExprAuditBeg;
-        AE = CallExprAuditEnd;
-        break;
-      case Stmt::ObjCMessageExprClass:
-        AB = ObjCMsgExprAuditBeg;
-        AE = ObjCMsgExprAuditEnd;
-        break;
-    }
-    
+        
     // If the state hasn't changed, don't generate a new node.
-    if (!BuildSinks && St == PredState && AB == NULL) {
+    if (!BuildSinks && St == PredState && Auditor == 0) {
       Dst.Add(Pred);
       return NULL;
     }
@@ -268,9 +244,8 @@ public:
       if (BuildSinks)
         N->markAsSink();
       else {
-        for ( ; AB != AE; ++AB)
-          if ((*AB)->Audit(N))
-            N->markAsSink();
+        if (Auditor && Auditor->Audit(N))
+          N->markAsSink();
         
         Dst.Add(N);
       }

clang/include/clang/Analysis/PathSensitive/GRExprEngine.h

@@ -98,10 +98,7 @@ protected:
   Selector* NSExceptionInstanceRaiseSelectors;
   Selector RaiseSel;
   
-  typedef llvm::SmallVector<GRSimpleAPICheck*,2> SimpleChecksTy;
-  
-  SimpleChecksTy CallChecks;
-  SimpleChecksTy MsgExprChecks;
+  llvm::OwningPtr<GRSimpleAPICheck> BatchAuditor;
   
 public:
   typedef llvm::SmallPtrSet<NodeTy*,2> UndefBranchesTy;  
@@ -347,21 +344,7 @@ public:
     return UndefReceivers.end();
   }
   
-  typedef SimpleChecksTy::iterator simple_checks_iterator;
-  
-  simple_checks_iterator call_auditors_begin() { return CallChecks.begin(); }
-  simple_checks_iterator call_auditors_end() { return CallChecks.end(); }
-  
-  simple_checks_iterator msgexpr_auditors_begin() {
-    return MsgExprChecks.begin();
-  }
-  simple_checks_iterator msgexpr_auditors_end() {
-    return MsgExprChecks.end();
-  }
-  
-  void AddCallCheck(GRSimpleAPICheck* A);
-  
-  void AddObjCMessageExprCheck(GRSimpleAPICheck* A);
+  void AddCheck(GRSimpleAPICheck* A, Stmt::StmtClass C);
   
   /// ProcessStmt - Called by GRCoreEngine. Used to generate new successor
   ///  nodes by processing the 'effects' of a block-level statement.  

clang/lib/Analysis/GRExprEngine.cpp

@@ -13,12 +13,13 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Analysis/PathSensitive/BasicStore.h"
 #include "clang/Analysis/PathSensitive/GRExprEngine.h"
 #include "clang/Analysis/PathSensitive/BugReporter.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/Streams.h"
-
-#include "clang/Analysis/PathSensitive/BasicStore.h"
+#include "llvm/ADT/ImmutableList.h"
+#include "llvm/Support/Compiler.h"
 
 #ifndef NDEBUG
 #include "llvm/Support/GraphWriter.h"
@@ -34,6 +35,79 @@ using llvm::APSInt;
 // Engine construction and deletion.
 //===----------------------------------------------------------------------===//
 
+namespace {
+
+class VISIBILITY_HIDDEN MappedBatchAuditor : public GRSimpleAPICheck {
+  typedef llvm::ImmutableList<GRSimpleAPICheck*> Checks;
+  typedef llvm::DenseMap<void*,Checks> MapTy;
+  
+  MapTy M;
+  Checks::Factory F;
+
+public:
+  MappedBatchAuditor(llvm::BumpPtrAllocator& Alloc) : F(Alloc) {}
+  
+  virtual ~MappedBatchAuditor() {
+    llvm::DenseSet<GRSimpleAPICheck*> AlreadyVisited;
+    
+    for (MapTy::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI)
+      for (Checks::iterator I=MI->second.begin(), E=MI->second.end(); I!=E;++I){
+
+        GRSimpleAPICheck* check = *I;
+        
+        if (AlreadyVisited.count(check))
+          continue;
+        
+        AlreadyVisited.insert(check);
+        delete check;
+      }
+  }
+
+  void AddCheck(GRSimpleAPICheck* A, Stmt::StmtClass C) {
+    assert (A && "Check cannot be null.");
+    void* key = reinterpret_cast<void*>((uintptr_t) C);
+    MapTy::iterator I = M.find(key);
+    M[key] = F.Concat(A, I == M.end() ? F.GetEmptyList() : I->second);
+  }
+  
+  virtual void EmitWarnings(BugReporter& BR) {
+    llvm::DenseSet<GRSimpleAPICheck*> AlreadyVisited;
+    
+    for (MapTy::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI)
+      for (Checks::iterator I=MI->second.begin(), E=MI->second.end(); I!=E;++I){
+        
+        GRSimpleAPICheck* check = *I;
+        
+        if (AlreadyVisited.count(check))
+          continue;
+        
+        check->EmitWarnings(BR);
+      }
+  }
+  
+  virtual bool Audit(NodeTy* N) {
+    Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
+    void* key = reinterpret_cast<void*>((uintptr_t) S->getStmtClass());
+    MapTy::iterator MI = M.find(key);
+
+    if (MI == M.end())
+      return false;
+    
+    bool isSink = false;
+    
+    for (Checks::iterator I=MI->second.begin(), E=MI->second.end(); I!=E; ++I)
+      isSink |= (*I)->Audit(N);
+
+    return isSink;    
+  }
+};
+
+} // end anonymous namespace
+
+//===----------------------------------------------------------------------===//
+// Engine construction and deletion.
+//===----------------------------------------------------------------------===//
+
 static inline Selector GetNullarySelector(const char* name, ASTContext& Ctx) {
   IdentifierInfo* II = &Ctx.Idents.get(name);
   return Ctx.Selectors.getSelector(0, &II);
@@ -58,14 +132,7 @@ GRExprEngine::GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx,
 GRExprEngine::~GRExprEngine() {    
   for (BugTypeSet::iterator I = BugTypes.begin(), E = BugTypes.end(); I!=E; ++I)
     delete *I;
-    
-  for (SimpleChecksTy::iterator I = CallChecks.begin(), E = CallChecks.end();
-       I != E; ++I)
-    delete *I;
-  
-  for (SimpleChecksTy::iterator I=MsgExprChecks.begin(), E=MsgExprChecks.end();
-       I != E; ++I)
-    delete *I;  
+
   
   delete [] NSExceptionInstanceRaiseSelectors;
 }
@@ -104,16 +171,9 @@ void GRExprEngine::EmitWarnings(BugReporterData& BRData) {
     (*I)->EmitWarnings(BR);
   }
   
-  for (SimpleChecksTy::iterator I = CallChecks.begin(), E = CallChecks.end();
-       I != E; ++I) {
+  if (BatchAuditor) {
     GRBugReporter BR(BRData, *this);
-    (*I)->EmitWarnings(BR);
-  }
-  
-  for (SimpleChecksTy::iterator I=MsgExprChecks.begin(), E=MsgExprChecks.end();
-       I != E; ++I) {
-    GRBugReporter BR(BRData, *this);
-    (*I)->EmitWarnings(BR);
+    BatchAuditor->EmitWarnings(BR);
   }
 }
 
@@ -122,12 +182,11 @@ void GRExprEngine::setTransferFunctions(GRTransferFuncs* tf) {
   TF->RegisterChecks(*this);
 }
 
-void GRExprEngine::AddCallCheck(GRSimpleAPICheck* A) {
-  CallChecks.push_back(A);
-}
-
-void GRExprEngine::AddObjCMessageExprCheck(GRSimpleAPICheck* A) {
-  MsgExprChecks.push_back(A);
+void GRExprEngine::AddCheck(GRSimpleAPICheck* A, Stmt::StmtClass C) {
+  if (!BatchAuditor)
+    BatchAuditor.reset(new MappedBatchAuditor(getGraph().getAllocator()));
+  
+  ((MappedBatchAuditor*) BatchAuditor.get())->AddCheck(A, C);
 }
 
 const ValueState* GRExprEngine::getInitialState() {
@@ -186,26 +245,14 @@ void GRExprEngine::ProcessStmt(Stmt* S, StmtNodeBuilder& builder) {
   CurrentStmt = S;
   
   // Set up our simple checks.
+  if (BatchAuditor)
+    Builder->setAuditor(BatchAuditor.get());
   
-  // FIXME: This can probably be installed directly in GRCoreEngine, obviating
-  //  the need to do a copy every time we hit a block-level statement.
-  
-  if (!MsgExprChecks.empty())
-    Builder->setObjCMsgExprAuditors((GRAuditor<ValueState>**) &MsgExprChecks[0],
-         (GRAuditor<ValueState>**) (&MsgExprChecks[0] + MsgExprChecks.size()));
-  
-  
-  if (!CallChecks.empty())
-    Builder->setCallExprAuditors((GRAuditor<ValueState>**) &CallChecks[0],
-         (GRAuditor<ValueState>**) (&CallChecks[0] + CallChecks.size()));
-  
-  // Create the cleaned state.
-  
+  // Create the cleaned state.  
   CleanedState = StateMgr.RemoveDeadBindings(EntryNode->getState(), CurrentStmt,
                                              Liveness, DeadSymbols);
   
   // Process any special transfer function for dead symbols.
-  
   NodeSet Tmp;
   
   if (DeadSymbols.empty())

clang/lib/Analysis/GRSimpleVals.cpp

@@ -354,11 +354,11 @@ void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) {
   ASTContext& Ctx = Eng.getContext();
   ValueStateManager* VMgr = &Eng.getStateManager();
 
-  GRSimpleAPICheck* Check = CreateBasicObjCFoundationChecks(Ctx, VMgr);  
-  Eng.AddObjCMessageExprCheck(Check);
+  GRSimpleAPICheck* Check = CreateBasicObjCFoundationChecks(Ctx, VMgr);
+  Eng.AddCheck(Check, Stmt::ObjCMessageExprClass);
   
-  Check = CreateAuditCFNumberCreate(Ctx, VMgr);  
-  Eng.AddCallCheck(Check);
+  Check = CreateAuditCFNumberCreate(Ctx, VMgr);
+  Eng.AddCheck(Check, Stmt::CallExprClass);
   
 }