[cfe-commits] r138001 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticAnalyzer/Core/BugReporterVisitors.cpp lib/StaticAnalyzer/Core/CFRefCount.cpp

Anna Zaks ganna at apple.com
Thu Aug 18 15:37:56 PDT 2011


Author: zaks
Date: Thu Aug 18 17:37:56 2011
New Revision: 138001

URL: http://llvm.org/viewvc/llvm-project?rev=138001&view=rev
Log:
Static Analyzer Diagnostics: Move custom diagnostic visitors from BugReporterContext to BugReport. 

One API change: I added BugReporter as an additional parameter to the BugReporterVisitor::VisitNode() method to allow visitors register other visitors with the report on the fly (while processing a node). This functionality is used by NilReceiverVisitor, which registers TrackNullOrUndefValue when the receiver is null.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=138001&r1=138000&r2=138001&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Thu Aug 18 17:37:56 2011
@@ -37,6 +37,7 @@
 class PathDiagnosticClient;
 class ExplodedNode;
 class ExplodedGraph;
+class BugReport;
 class BugReporter;
 class BugReporterContext;
 class ExprEngine;
@@ -50,9 +51,16 @@
 class BugReporterVisitor : public llvm::FoldingSetNode {
 public:
   virtual ~BugReporterVisitor();
+
+  /// \brief Return a diagnostic piece which should be associated with the
+  /// given node.
+  ///
+  /// The last parameter can be used to register a new visitor with the given
+  /// BugReport while processing a node.
   virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                          const ExplodedNode *PrevN,
-                                         BugReporterContext &BRC) = 0;
+                                         BugReporterContext &BRC,
+                                         BugReport &BR) = 0;
 
   virtual bool isOwnedByReporterContext() { return true; }
   virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0;
@@ -69,9 +77,9 @@
             getOriginalNode(const ExplodedNode *N) = 0;
   };
 
-  typedef void (*VisitorCreator)(BugReporterContext &BRcC, const void *data,
-                                 const ExplodedNode *N);
+  typedef void (*VisitorCreator)(BugReport &BR, const void *data);
   typedef const SourceRange *ranges_iterator;
+  typedef llvm::ImmutableList<BugReporterVisitor*>::iterator visitor_iterator;
 
 protected:
   friend class BugReporter;
@@ -84,7 +92,12 @@
   FullSourceLoc Location;
   const ExplodedNode *ErrorNode;
   SmallVector<SourceRange, 4> Ranges;
-  Creators creators;
+
+  // Not the most efficient data structure, but we use an ImmutableList for the
+  // Callbacks because it is safe to make additions to list during iteration.
+  llvm::ImmutableList<BugReporterVisitor*>::Factory F;
+  llvm::ImmutableList<BugReporterVisitor*> Callbacks;
+  llvm::FoldingSet<BugReporterVisitor> CallbacksSet;
 
   /// Profile to identify equivalent bug reports for error report coalescing.
   /// Reports are uniqued to ensure that we do not emit multiple diagnostics
@@ -95,15 +108,23 @@
 
 public:
   BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode)
-    : BT(bt), Description(desc), ErrorNode(errornode) {}
+    : BT(bt), Description(desc), ErrorNode(errornode),
+      Callbacks(F.getEmptyList()) {
+      addVisitor(this);
+    }
 
   BugReport(BugType& bt, StringRef shortDesc, StringRef desc,
             const ExplodedNode *errornode)
-  : BT(bt), ShortDescription(shortDesc), Description(desc),
-    ErrorNode(errornode) {}
+    : BT(bt), ShortDescription(shortDesc), Description(desc),
+      ErrorNode(errornode), Callbacks(F.getEmptyList()) {
+      addVisitor(this);
+    }
 
   BugReport(BugType& bt, StringRef desc, FullSourceLoc l)
-    : BT(bt), Description(desc), Location(l), ErrorNode(0) {}
+    : BT(bt), Description(desc), Location(l), ErrorNode(0),
+      Callbacks(F.getEmptyList()) {
+      addVisitor(this);
+    }
 
   virtual ~BugReport();
 
@@ -158,18 +179,19 @@
   /// registerFindLastStore(), registerNilReceiverVisitor(), and
   /// registerVarDeclsLastStore().
   void addVisitorCreator(VisitorCreator creator, const void *data) {
-    creators.push_back(std::make_pair(creator, data));
+    creator(*this, data);
   }
 
+  void addVisitor(BugReporterVisitor* visitor);
+
+	/// Iterators through the custom diagnostic visitors.
+  visitor_iterator visitor_begin() { return Callbacks.begin(); }
+  visitor_iterator visitor_end() { return Callbacks.end(); }
+
   virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                          const ExplodedNode *PrevN,
-                                         BugReporterContext &BR);
-
-  virtual void registerInitialVisitors(BugReporterContext &BRC,
-                                       const ExplodedNode *N) {
-    for (Creators::iterator I = creators.begin(), E = creators.end(); I!=E; ++I)
-      I->first(BRC, I->second, N);
-  }
+                                         BugReporterContext &BRC,
+                                         BugReport &BR);
 };
 
 //===----------------------------------------------------------------------===//
@@ -383,20 +405,10 @@
 
 class BugReporterContext {
   GRBugReporter &BR;
-  // Not the most efficient data structure, but we use an ImmutableList for the
-  // Callbacks because it is safe to make additions to list during iteration.
-  llvm::ImmutableList<BugReporterVisitor*>::Factory F;
-  llvm::ImmutableList<BugReporterVisitor*> Callbacks;
-  llvm::FoldingSet<BugReporterVisitor> CallbacksSet;
 public:
-  BugReporterContext(GRBugReporter& br) : BR(br), Callbacks(F.getEmptyList()) {}
-  virtual ~BugReporterContext();
-
-  void addVisitor(BugReporterVisitor* visitor);
+  BugReporterContext(GRBugReporter& br) : BR(br) {}
 
-  typedef llvm::ImmutableList<BugReporterVisitor*>::iterator visitor_iterator;
-  visitor_iterator visitor_begin() { return Callbacks.begin(); }
-  visitor_iterator visitor_end() { return Callbacks.end(); }
+  virtual ~BugReporterContext() {}
 
   GRBugReporter& getBugReporter() { return BR; }
 
@@ -441,18 +453,15 @@
 const Stmt *GetCalleeExpr(const ExplodedNode *N);
 const Stmt *GetRetValExpr(const ExplodedNode *N);
 
-void registerConditionVisitor(BugReporterContext &BRC);
+void registerConditionVisitor(BugReport &BR);
 
-void registerTrackNullOrUndefValue(BugReporterContext &BRC, const void *stmt,
-                                   const ExplodedNode *N);
+void registerTrackNullOrUndefValue(BugReport &BR, const void *stmt);
 
-void registerFindLastStore(BugReporterContext &BRC, const void *memregion,
-                           const ExplodedNode *N);
+void registerFindLastStore(BugReport &BR, const void *memregion);
 
-void registerNilReceiverVisitor(BugReporterContext &BRC);  
+void registerNilReceiverVisitor(BugReport &BR);
 
-void registerVarDeclsLastStore(BugReporterContext &BRC, const void *stmt,
-                               const ExplodedNode *N);
+void registerVarDeclsLastStore(BugReport &BR, const void *stmt);
 
 } // end namespace clang::bugreporter
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=138001&r1=138000&r2=138001&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Thu Aug 18 17:37:56 2011
@@ -33,27 +33,6 @@
 using namespace ento;
 
 BugReporterVisitor::~BugReporterVisitor() {}
-BugReporterContext::~BugReporterContext() {
-  for (visitor_iterator I = visitor_begin(), E = visitor_end(); I != E; ++I)
-    if ((*I)->isOwnedByReporterContext()) delete *I;
-}
-
-void BugReporterContext::addVisitor(BugReporterVisitor* visitor) {
-  if (!visitor)
-    return;
-
-  llvm::FoldingSetNodeID ID;
-  visitor->Profile(ID);
-  void *InsertPos;
-
-  if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos)) {
-    delete visitor;
-    return;
-  }
-
-  CallbacksSet.InsertNode(visitor, InsertPos);
-  Callbacks = F.add(visitor, Callbacks);
-}
 
 //===----------------------------------------------------------------------===//
 // Helper routines for walking the ExplodedGraph and fetching statements.
@@ -161,15 +140,15 @@
                         BugReport *r, NodeBackMap *Backmap,
                         PathDiagnosticClient *pdc)
     : BugReporterContext(br),
-      R(r), PDC(pdc), NMC(Backmap) {
-    addVisitor(R);
-  }
+      R(r), PDC(pdc), NMC(Backmap) {}
 
   PathDiagnosticLocation ExecutionContinues(const ExplodedNode *N);
 
   PathDiagnosticLocation ExecutionContinues(llvm::raw_string_ostream &os,
                                             const ExplodedNode *N);
 
+  BugReport *getBugReport() { return R; }
+
   Decl const &getCodeDecl() { return R->getErrorNode()->getCodeDecl(); }
 
   ParentMap& getParentMap() { return R->getErrorNode()->getParentMap(); }
@@ -791,9 +770,11 @@
     }
 
     if (NextNode) {
-      for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
-           E = PDB.visitor_end(); I!=E; ++I) {
-        if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB))
+      // Add diagnostic pieces from custom visitors.
+      BugReport *R = PDB.getBugReport();
+      for (BugReport::visitor_iterator I = R->visitor_begin(),
+           E = R->visitor_end(); I!=E; ++I) {
+        if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R))
           PD.push_front(p);
       }
     }
@@ -1198,9 +1179,11 @@
     if (!NextNode)
       continue;
 
-    for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
-         E = PDB.visitor_end(); I!=E; ++I) {
-      if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB)) {
+    // Add pieces from custom visitors.
+    BugReport *R = PDB.getBugReport();
+    for (BugReport::visitor_iterator I = R->visitor_begin(),
+                                     E = R->visitor_end(); I!=E; ++I) {
+      if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R)) {
         const PathDiagnosticLocation &Loc = p->getLocation();
         EB.addEdge(Loc, true);
         PD.push_front(p);
@@ -1222,7 +1205,30 @@
 // Methods for BugReport and subclasses.
 //===----------------------------------------------------------------------===//
 
-BugReport::~BugReport() {}
+void BugReport::addVisitor(BugReporterVisitor* visitor) {
+  if (!visitor)
+    return;
+
+  llvm::FoldingSetNodeID ID;
+  visitor->Profile(ID);
+  void *InsertPos;
+
+  if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos)) {
+    delete visitor;
+    return;
+  }
+
+  CallbacksSet.InsertNode(visitor, InsertPos);
+  Callbacks = F.add(visitor, Callbacks);
+}
+
+BugReport::~BugReport() {
+  for (visitor_iterator I = visitor_begin(), E = visitor_end(); I != E; ++I) {
+    // TODO: Remove the isOwned method; use reference counting to track visitors?.
+    assert((*I)->isOwnedByReporterContext());
+    delete *I;
+  }
+}
 
 void BugReport::Profile(llvm::FoldingSetNodeID& hash) const {
   hash.AddPointer(&BT);
@@ -1336,7 +1342,8 @@
 
 PathDiagnosticPiece *BugReport::VisitNode(const ExplodedNode *N,
                                           const ExplodedNode *PrevN,
-                                          BugReporterContext &BRC) {
+                                          BugReporterContext &BRC,
+                                          BugReport &BR) {
   return NULL;
 }
 
@@ -1655,10 +1662,9 @@
   else
     return;
 
-  // Register node visitors.
-  R->registerInitialVisitors(PDB, N);
-  bugreporter::registerNilReceiverVisitor(PDB);
-  bugreporter::registerConditionVisitor(PDB);
+  // Register additional node visitors.
+  bugreporter::registerNilReceiverVisitor(*R);
+  bugreporter::registerConditionVisitor(*R);
 
   switch (PDB.getGenerationScheme()) {
     case PathDiagnosticClient::Extensive:

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=138001&r1=138000&r2=138001&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Thu Aug 18 17:37:56 2011
@@ -91,7 +91,8 @@
 
   PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                  const ExplodedNode *PrevN,
-                                 BugReporterContext &BRC) {
+                                 BugReporterContext &BRC,
+                                 BugReport &BR) {
 
     if (satisfied)
       return NULL;
@@ -221,9 +222,9 @@
 };
 
 
-static void registerFindLastStore(BugReporterContext &BRC, const MemRegion *R,
+static void registerFindLastStore(BugReport &BR, const MemRegion *R,
                                   SVal V) {
-  BRC.addVisitor(new FindLastStoreBRVisitor(V, R));
+  BR.addVisitor(new FindLastStoreBRVisitor(V, R));
 }
 
 class TrackConstraintBRVisitor : public BugReporterVisitor {
@@ -243,7 +244,8 @@
 
   PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                  const ExplodedNode *PrevN,
-                                 BugReporterContext &BRC) {
+                                 BugReporterContext &BRC,
+                                 BugReport &BR) {
     if (isSatisfied)
       return NULL;
 
@@ -297,23 +299,23 @@
 };
 } // end anonymous namespace
 
-static void registerTrackConstraint(BugReporterContext &BRC,
+static void registerTrackConstraint(BugReport &BR,
                                     DefinedSVal Constraint,
                                     bool Assumption) {
-  BRC.addVisitor(new TrackConstraintBRVisitor(Constraint, Assumption));
+  BR.addVisitor(new TrackConstraintBRVisitor(Constraint, Assumption));
 }
 
-void bugreporter::registerTrackNullOrUndefValue(BugReporterContext &BRC,
-                                                const void *data,
-                                                const ExplodedNode *N) {
+void bugreporter::registerTrackNullOrUndefValue(BugReport &BR,
+                                                const void *data) {
 
   const Stmt *S = static_cast<const Stmt*>(data);
+  const ExplodedNode *N = BR.getErrorNode();
 
   if (!S)
     return;
-
-  ProgramStateManager &StateMgr = BRC.getStateManager();
   
+  ProgramStateManager &StateMgr = N->getState()->getStateManager();
+
   // Walk through nodes until we get one that matches the statement
   // exactly.
   while (N) {
@@ -341,7 +343,7 @@
 
       if (isa<loc::ConcreteInt>(V) || isa<nonloc::ConcreteInt>(V)
           || V.isUndef()) {
-        ::registerFindLastStore(BRC, R, V);
+        ::registerFindLastStore(BR, R, V);
       }
     }
   }
@@ -361,16 +363,16 @@
 
     if (R) {
       assert(isa<SymbolicRegion>(R));
-      registerTrackConstraint(BRC, loc::MemRegionVal(R), false);
+      registerTrackConstraint(BR, loc::MemRegionVal(R), false);
     }
   }
 }
 
-void bugreporter::registerFindLastStore(BugReporterContext &BRC,
-                                        const void *data,
-                                        const ExplodedNode *N) {
+void bugreporter::registerFindLastStore(BugReport &BR,
+                                        const void *data) {
 
   const MemRegion *R = static_cast<const MemRegion*>(data);
+  const ExplodedNode *N = BR.getErrorNode();
 
   if (!R)
     return;
@@ -381,7 +383,7 @@
   if (V.isUnknown())
     return;
 
-  BRC.addVisitor(new FindLastStoreBRVisitor(V, R));
+  BR.addVisitor(new FindLastStoreBRVisitor(V, R));
 }
 
 
@@ -397,7 +399,8 @@
 
   PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                  const ExplodedNode *PrevN,
-                                 BugReporterContext &BRC) {
+                                 BugReporterContext &BRC,
+                                 BugReport &BR) {
 
     const PostStmt *P = N->getLocationAs<PostStmt>();
     if (!P)
@@ -420,7 +423,7 @@
     // The receiver was nil, and hence the method was skipped.
     // Register a BugReporterVisitor to issue a message telling us how
     // the receiver was null.
-    bugreporter::registerTrackNullOrUndefValue(BRC, Receiver, N);
+    bugreporter::registerTrackNullOrUndefValue(BR, Receiver);
     // Issue a message saying that the method was skipped.
     PathDiagnosticLocation L(Receiver, BRC.getSourceManager());
     return new PathDiagnosticEventPiece(L, "No method actually called "
@@ -429,15 +432,15 @@
 };
 } // end anonymous namespace
 
-void bugreporter::registerNilReceiverVisitor(BugReporterContext &BRC) {
-  BRC.addVisitor(new NilReceiverVisitor());
+void bugreporter::registerNilReceiverVisitor(BugReport &BR) {
+  BR.addVisitor(new NilReceiverVisitor());
 }
 
-// Registers every VarDecl inside a Stmt with a last store vistor.
-void bugreporter::registerVarDeclsLastStore(BugReporterContext &BRC,
-                                                   const void *stmt,
-                                                   const ExplodedNode *N) {
+// Registers every VarDecl inside a Stmt with a last store visitor.
+void bugreporter::registerVarDeclsLastStore(BugReport &BR,
+                                            const void *stmt) {
   const Stmt *S = static_cast<const Stmt *>(stmt);
+  const ExplodedNode *N = BR.getErrorNode();
 
   std::deque<const Stmt *> WorkList;
 
@@ -447,8 +450,8 @@
     const Stmt *Head = WorkList.front();
     WorkList.pop_front();
 
-    ProgramStateManager &StateMgr = BRC.getStateManager();
     const ProgramState *state = N->getState();
+    ProgramStateManager &StateMgr = state->getStateManager();
 
     if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Head)) {
       if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
@@ -459,7 +462,7 @@
         SVal V = state->getSVal(S);
 
         if (isa<loc::ConcreteInt>(V) || isa<nonloc::ConcreteInt>(V)) {
-          ::registerFindLastStore(BRC, R, V);
+          ::registerFindLastStore(BR, R, V);
         }
       }
     }
@@ -484,7 +487,8 @@
 
   virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                          const ExplodedNode *Prev,
-                                         BugReporterContext &BRC);
+                                         BugReporterContext &BRC,
+                                         BugReport &BR);
   
   PathDiagnosticPiece *VisitTerminator(const Stmt *Term,
                                        const ProgramState *CurrentState,
@@ -515,7 +519,8 @@
 
 PathDiagnosticPiece *ConditionVisitor::VisitNode(const ExplodedNode *N,
                                                  const ExplodedNode *Prev,
-                                                 BugReporterContext &BRC) {
+                                                 BugReporterContext &BRC,
+                                                 BugReport &BR) {
   
   const ProgramPoint &progPoint = N->getLocation();
 
@@ -752,7 +757,7 @@
   return new PathDiagnosticEventPiece(Loc, Out.str());
 }
 
-void bugreporter::registerConditionVisitor(BugReporterContext &BRC) {
-  BRC.addVisitor(new ConditionVisitor());
+void bugreporter::registerConditionVisitor(BugReport &BR) {
+  BR.addVisitor(new ConditionVisitor());
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp?rev=138001&r1=138000&r2=138001&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp Thu Aug 18 17:37:56 2011
@@ -1994,7 +1994,8 @@
 
     PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
                                    const ExplodedNode *PrevN,
-                                   BugReporterContext &BRC);
+                                   BugReporterContext &BRC,
+                                   BugReport &BR);
   };
 
   class CFRefLeakReport : public CFRefReport {
@@ -2061,7 +2062,8 @@
 
 PathDiagnosticPiece *CFRefReport::VisitNode(const ExplodedNode *N,
                                             const ExplodedNode *PrevN,
-                                            BugReporterContext &BRC) {
+                                            BugReporterContext &BRC,
+                                            BugReport &BR) {
 
   if (!isa<PostStmt>(N->getLocation()))
     return NULL;





More information about the cfe-commits mailing list