Hooked up the dead-store checker to the BugReporter interface. Now dead-store

warnings are emitted as part of the warnings registered by GRSimpleVals.

llvm-svn: 49658

5 files changed, 179 insertions, 111 deletions

diff --git a/clang/lib/Analysis/BasicObjCFoundationChecks.cpp b/clang/lib/Analysis/BasicObjCFoundationChecks.cpp
index e5a31bf50df..717f5b8d802 100644
--- a/clang/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/clang/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -68,7 +68,9 @@ public:
     SourceRange R;
   public:
     
-    Report(NilArg& Desc, ObjCMessageExpr* ME, unsigned Arg) : BugReport(Desc) {
+    Report(NilArg& Desc, ExplodedNode<ValueState>* N, 
+           ObjCMessageExpr* ME, unsigned Arg)
+      : BugReport(Desc, N) {
       
       Expr* E = ME->getArg(Arg);
       R = E->getSourceRange();
@@ -90,22 +92,6 @@ public:
       B = &R;
       E = B+1;
     }
-    
-    virtual PathDiagnosticPiece* getEndPath(ASTContext& Ctx,
-                                            ExplodedNode<ValueState> *N) const {
-      
-      Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
-      
-      if (!S)
-        return NULL;
-      
-      FullSourceLoc L(S->getLocStart(), Ctx.getSourceManager());
-      PathDiagnosticPiece* P = new PathDiagnosticPiece(L, s);
-      
-      P->addRange(R);
-      
-      return P;
-    }
   };  
 };
 
@@ -115,7 +101,7 @@ class VISIBILITY_HIDDEN BasicObjCFoundationChecks : public GRSimpleAPICheck {
   ASTContext &Ctx;
   ValueStateManager* VMgr;
   
-  typedef std::vector<std::pair<NodeTy*,BugReport*> > ErrorsTy;
+  typedef std::vector<BugReport*> ErrorsTy;
   ErrorsTy Errors;
       
   RVal GetRVal(ValueState* St, Expr* E) { return VMgr->GetRVal(St, E); }
@@ -134,7 +120,7 @@ public:
       
   virtual ~BasicObjCFoundationChecks() {
     for (ErrorsTy::iterator I = Errors.begin(), E = Errors.end(); I!=E; ++I)
-      delete I->second;    
+      delete *I;    
   }
   
   virtual bool Audit(ExplodedNode<ValueState>* N);
@@ -143,12 +129,12 @@ public:
   
 private:
   
-  void AddError(NodeTy* N, BugReport* R) {
-    Errors.push_back(std::make_pair(N, R));
+  void AddError(BugReport* R) {
+    Errors.push_back(R);
   }
   
   void WarnNilArg(NodeTy* N, ObjCMessageExpr* ME, unsigned Arg) {
-    AddError(N, new NilArg::Report(Desc, ME, Arg));
+    AddError(new NilArg::Report(Desc, N, ME, Arg));
   }
 };
   
@@ -203,9 +189,8 @@ static inline bool isNil(RVal X) {
 
 void BasicObjCFoundationChecks::EmitWarnings(BugReporter& BR) {    
                  
-  for (ErrorsTy::iterator I=Errors.begin(), E=Errors.end(); I!=E; ++I)
-    
-    BR.EmitPathWarning(*I->second, I->first);
+  for (ErrorsTy::iterator I=Errors.begin(), E=Errors.end(); I!=E; ++I)    
+    BR.EmitPathWarning(**I);
 }
 
 bool BasicObjCFoundationChecks::CheckNilArg(NodeTy* N, unsigned Arg) {
diff --git a/clang/lib/Analysis/BugReporter.cpp b/clang/lib/Analysis/BugReporter.cpp
index 148d2582dc9..310fdc9accb 100644
--- a/clang/lib/Analysis/BugReporter.cpp
+++ b/clang/lib/Analysis/BugReporter.cpp
@@ -52,11 +52,13 @@ static inline Stmt* GetStmt(const CFGBlock* B) {
   return (*B)[0];
 }
 
-
-PathDiagnosticPiece*
-BugReport::getEndPath(ASTContext& Ctx, ExplodedNode<ValueState> *N) const {
+Stmt* BugReport::getStmt() const {
+  return N ? GetStmt(N->getLocation()) : NULL;
+}
   
-  Stmt* S = GetStmt(N->getLocation());
+PathDiagnosticPiece* BugReport::getEndPath(ASTContext& Ctx) const {
+  
+  Stmt* S = getStmt();
   
   if (!S)
     return NULL;
@@ -83,11 +85,24 @@ BugReport::getEndPath(ASTContext& Ctx, ExplodedNode<ValueState> *N) const {
 }
 
 void BugReport::getRanges(const SourceRange*& beg,
-                          const SourceRange*& end) const {
+                          const SourceRange*& end) const {  
   beg = NULL;
   end = NULL;
 }
 
+FullSourceLoc BugReport::getLocation(SourceManager& Mgr) {
+  
+  if (!N)
+    return FullSourceLoc();
+  
+  Stmt* S = GetStmt(N->getLocation());
+  
+  if (!S)
+    return FullSourceLoc();
+  
+  return FullSourceLoc(S->getLocStart(), Mgr);
+}
+
 PathDiagnosticPiece* BugReport::VisitNode(ExplodedNode<ValueState>* N,
                                           ExplodedNode<ValueState>* PrevN,
                                           ExplodedGraph<ValueState>& G,
@@ -96,10 +111,13 @@ PathDiagnosticPiece* BugReport::VisitNode(ExplodedNode<ValueState>* N,
 }
 
 void BugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
-                                         BugReport& R,
-                                         ExplodedNode<ValueState>* N) {
+                                         BugReport& R) {
 
-  if (PathDiagnosticPiece* Piece = R.getEndPath(Ctx,N))
+  ExplodedNode<ValueState>* N = R.getEndNode();
+  
+  assert (N && "Path diagnostic requires a ExplodedNode.");
+  
+  if (PathDiagnosticPiece* Piece = R.getEndPath(Ctx))
     PD.push_back(Piece);
   else
     return;
@@ -325,10 +343,12 @@ bool BugReporter::IsCached(ExplodedNode<ValueState>* N) {
   return false;
 }
 
-void BugReporter::EmitPathWarning(BugReport& R, ExplodedNode<ValueState>* N) {
+void BugReporter::EmitPathWarning(BugReport& R) {
+  
+  ExplodedNode<ValueState>* N = R.getEndNode();
   
-  if (!PD) {
-    EmitWarning(R, N);
+  if (!PD || !N) {
+    EmitWarning(R);
     return;
   }
   
@@ -336,15 +356,17 @@ void BugReporter::EmitPathWarning(BugReport& R, ExplodedNode<ValueState>* N) {
     return;
   
   PathDiagnostic D(R.getName());  
-  GeneratePathDiagnostic(D, R, N);
+  GeneratePathDiagnostic(D, R);
   
   if (!D.empty())  
     PD->HandlePathDiagnostic(D);
 }
 
+void BugReporter::EmitWarning(BugReport& R) {  
 
-void BugReporter::EmitWarning(BugReport& R, ExplodedNode<ValueState>* N) {  
-  if (IsCached(N))
+  ExplodedNode<ValueState>* N = R.getEndNode();
+  
+  if (N && IsCached(N))
     return;
   
   std::ostringstream os;
@@ -355,23 +377,9 @@ void BugReporter::EmitWarning(BugReport& R, ExplodedNode<ValueState>* N) {
 
   // FIXME: Add support for multiple ranges.
   
-  Stmt* S = GetStmt(N->getLocation());
-  
-  if (!S)
-    return;
-  
+  FullSourceLoc L = R.getLocation(Ctx.getSourceManager());
+
   const SourceRange *Beg, *End;
   R.getRanges(Beg, End);
-  
-  if (Beg == End) {
-    SourceRange Range = S->getSourceRange();
-    
-    Diag.Report(FullSourceLoc(S->getLocStart(), Ctx.getSourceManager()),
-                ErrorDiag, NULL, 0, &Range, 1);   
-    
-  }
-  else
-    Diag.Report(FullSourceLoc(S->getLocStart(), Ctx.getSourceManager()),
-                ErrorDiag, NULL, 0, Beg, End - Beg);
-  
+  Diag.Report(L, ErrorDiag, NULL, 0, Beg, End - Beg);
 }
diff --git a/clang/lib/Analysis/CFRefCount.cpp b/clang/lib/Analysis/CFRefCount.cpp
index ff78fd749ec..df479884ed5 100644
--- a/clang/lib/Analysis/CFRefCount.cpp
+++ b/clang/lib/Analysis/CFRefCount.cpp
@@ -911,9 +911,9 @@ void UseAfterRelease::EmitWarnings(BugReporter& BR) {
   for (CFRefCount::use_after_iterator I = TF.use_after_begin(),
         E = TF.use_after_end(); I != E; ++I) {
     
-    RangedBugReport report(*this);
+    RangedBugReport report(*this, I->first);
     report.addRange(I->second->getSourceRange());    
-    BR.EmitPathWarning(report, I->first);    
+    BR.EmitPathWarning(report);    
   }
 }
 
@@ -922,9 +922,9 @@ void BadRelease::EmitWarnings(BugReporter& BR) {
   for (CFRefCount::bad_release_iterator I = TF.bad_release_begin(),
        E = TF.bad_release_end(); I != E; ++I) {
     
-    RangedBugReport report(*this);
+    RangedBugReport report(*this, I->first);
     report.addRange(I->second->getSourceRange());    
-    BR.EmitPathWarning(report, I->first); 
+    BR.EmitPathWarning(report); 
 
   }  
 }
diff --git a/clang/lib/Analysis/DeadStores.cpp b/clang/lib/Analysis/DeadStores.cpp
index 0848336e586..2e57757f5c2 100644
--- a/clang/lib/Analysis/DeadStores.cpp
+++ b/clang/lib/Analysis/DeadStores.cpp
@@ -15,6 +15,8 @@
 #include "clang/Analysis/LocalCheckers.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/Visitors/CFGRecStmtVisitor.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/Analysis/PathSensitive/GRExprEngine.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/AST/ASTContext.h"
 #include "llvm/Support/Compiler.h"
@@ -26,8 +28,11 @@ namespace {
 class VISIBILITY_HIDDEN DeadStoreObs : public LiveVariables::ObserverTy {
   ASTContext &Ctx;
   Diagnostic &Diags;
+  DiagnosticClient &Client;
 public:
-  DeadStoreObs(ASTContext &ctx,Diagnostic &diags) : Ctx(ctx), Diags(diags){}    
+  DeadStoreObs(ASTContext &ctx, Diagnostic &diags, DiagnosticClient &client)
+    : Ctx(ctx), Diags(diags), Client(client) {}    
+  
   virtual ~DeadStoreObs() {}
   
   virtual void ObserveStmt(Stmt* S,
@@ -41,7 +46,8 @@ public:
         if (VarDecl* VD = dyn_cast<VarDecl>(DR->getDecl()))
           if (VD->hasLocalStorage() && !Live(VD,AD)) {
             SourceRange R = B->getRHS()->getSourceRange();
-            Diags.Report(Ctx.getFullLoc(DR->getSourceRange().getBegin()),
+            Diags.Report(&Client,
+                         Ctx.getFullLoc(DR->getSourceRange().getBegin()),
                          diag::warn_dead_store, 0, 0, &R, 1);                                                                        
         }
     }
@@ -63,7 +69,8 @@ public:
               if (!E->isConstantExpr(Ctx,NULL)) {
                 // Flag a warning.
                 SourceRange R = E->getSourceRange();
-                Diags.Report(Ctx.getFullLoc(V->getLocation()),
+                Diags.Report(&Client,
+                             Ctx.getFullLoc(V->getLocation()),
                              diag::warn_dead_store, 0, 0, &R, 1);
               }
             }
@@ -74,14 +81,103 @@ public:
   
 } // end anonymous namespace
 
-namespace clang {
+//===----------------------------------------------------------------------===//
+// Driver function to invoke the Dead-Stores checker on a CFG.
+//===----------------------------------------------------------------------===//
 
-void CheckDeadStores(CFG& cfg, ASTContext &Ctx, Diagnostic &Diags) {
-  
+void clang::CheckDeadStores(CFG& cfg, ASTContext &Ctx, Diagnostic &Diags) {  
   LiveVariables L(cfg);
   L.runOnCFG(cfg);
-  DeadStoreObs A(Ctx, Diags);
+  DeadStoreObs A(Ctx, Diags, Diags.getClient());
   L.runOnAllBlocks(cfg, &A);
 }
 
-} // end namespace clang
+//===----------------------------------------------------------------------===//
+// BugReporter-based invocation of the Dead-Stores checker.
+//===----------------------------------------------------------------------===//
+  
+namespace {
+
+class VISIBILITY_HIDDEN DiagBugReport : public RangedBugReport {
+  std::list<std::string> Strs;
+  FullSourceLoc L;
+public:
+  DiagBugReport(const BugType& D, FullSourceLoc l) :
+    RangedBugReport(D, NULL), L(l) {}
+  
+  virtual ~DiagBugReport() {}
+  virtual FullSourceLoc getLocation(SourceManager&) { return L; }
+  
+  void addString(const std::string& s) { Strs.push_back(s); }  
+  
+  typedef std::list<std::string>::const_iterator str_iterator;
+  str_iterator str_begin() const { return Strs.begin(); }
+  str_iterator str_end() const { return Strs.end(); }
+};
+  
+class VISIBILITY_HIDDEN DiagCollector : public DiagnosticClient {
+  std::list<DiagBugReport> Reports;
+  const BugType& D;
+public:
+  DiagCollector(BugType& d) : D(d) {}
+  
+  virtual ~DiagCollector() {}
+  
+  virtual void HandleDiagnostic(Diagnostic &Diags, 
+                                Diagnostic::Level DiagLevel,
+                                FullSourceLoc Pos,
+                                diag::kind ID,
+                                const std::string *Strs,
+                                unsigned NumStrs,
+                                const SourceRange *Ranges, 
+                                unsigned NumRanges) {
+    
+    // FIXME: Use a map from diag::kind to BugType, instead of having just
+    //  one BugType.
+    
+    Reports.push_back(DiagBugReport(D, Pos));
+    DiagBugReport& R = Reports.back();
+    
+    for ( ; NumRanges ; --NumRanges, ++Ranges)
+      R.addRange(*Ranges);
+    
+    for ( ; NumStrs ; --NumStrs, ++Strs)
+      R.addString(*Strs);    
+  }
+  
+  // Iterators.
+  
+  typedef std::list<DiagBugReport>::iterator iterator;
+  iterator begin() { return Reports.begin(); }
+  iterator end() { return Reports.end(); }
+};
+  
+class VISIBILITY_HIDDEN DeadStoresChecker : public BugType {
+public:
+  virtual const char* getName() const {
+    return "dead store";
+  }
+  
+  virtual const char* getDescription() const {
+    return "Value stored to variable is never used.";
+  }
+  
+  virtual void EmitWarnings(BugReporter& BR) {
+    
+    // Run the dead store checker and collect the diagnostics.
+    DiagCollector C(*this);    
+    DeadStoreObs A(BR.getContext(), BR.getDiagnostic(), C);
+    GRExprEngine& Eng = BR.getEngine();
+    Eng.getLiveness().runOnAllBlocks(Eng.getCFG(), &A);
+    
+    // Emit the bug reports.
+    
+    for (DiagCollector::iterator I = C.begin(), E = C.end(); I != E; ++I)
+      BR.EmitWarning(*I);    
+  }
+};
+} // end anonymous namespace
+
+BugType* clang::MakeDeadStoresChecker() {
+  return new DeadStoresChecker();
+}
diff --git a/clang/lib/Analysis/GRSimpleVals.cpp b/clang/lib/Analysis/GRSimpleVals.cpp
index c350ab9752a..b37ccf5f802 100644
--- a/clang/lib/Analysis/GRSimpleVals.cpp
+++ b/clang/lib/Analysis/GRSimpleVals.cpp
@@ -44,8 +44,8 @@ void GenericEmitWarnings(BugReporter& BR, const BugType& D,
                          ITER I, ITER E) {
   
   for (; I != E; ++I) {
-    BugReport R(D);    
-    BR.EmitPathWarning(R, GetNode(I));
+    BugReport R(D, GetNode(I));    
+    BR.EmitPathWarning(R);
   }
 }
 
@@ -159,20 +159,6 @@ public:
   
   
 class VISIBILITY_HIDDEN BadArg : public BugType {
-protected:
-  
-  class Report : public BugReport {
-    const SourceRange R;
-  public:
-    Report(BugType& D, Expr* E) : BugReport(D), R(E->getSourceRange()) {}
-    virtual ~Report() {}
-
-    virtual void getRanges(const SourceRange*& B, const SourceRange*& E) const {
-      B = &R;
-      E = B+1;
-    }
-  };
-    
 public:
   
   virtual ~BadArg() {}
@@ -192,10 +178,11 @@ public:
          E = Eng.undef_arg_end(); I!=E; ++I) {
       
       // Generate a report for this bug.
-      Report report(*this, I->second);
+      RangedBugReport report(*this, I->first);
+      report.addRange(I->second->getSourceRange());
 
       // Emit the warning.
-      BR.EmitPathWarning(report, I->first);
+      BR.EmitPathWarning(report);
     }
 
   }
@@ -218,35 +205,16 @@ public:
          E = Eng.msg_expr_undef_arg_end(); I!=E; ++I) {
       
       // Generate a report for this bug.
-      Report report(*this, I->second);
+      RangedBugReport report(*this, I->first);
+      report.addRange(I->second->getSourceRange());
       
       // Emit the warning.
-      BR.EmitPathWarning(report, I->first);
-    }
-    
+      BR.EmitPathWarning(report);
+    }    
   }
 };
 
 class VISIBILITY_HIDDEN BadReceiver : public BugType {
-  
-  class Report : public BugReport {
-    SourceRange R;
-  public:
-    Report(BugType& D, ExplodedNode<ValueState> *N) : BugReport(D) {      
-      Stmt *S = cast<PostStmt>(N->getLocation()).getStmt();
-      Expr* E = cast<ObjCMessageExpr>(S)->getReceiver();
-      assert (E && "Receiver cannot be NULL");
-      R = E->getSourceRange();
-    }
-    
-    virtual ~Report() {}
-    
-    virtual void getRanges(const SourceRange*& B, const SourceRange*& E) const {
-      B = &R;
-      E = B+1;
-    }
-  };
-
 public:  
   virtual const char* getName() const {
     return "bad receiver";
@@ -263,10 +231,16 @@ public:
          E = Eng.undef_receivers_end(); I!=E; ++I) {
           
       // Generate a report for this bug.
-      Report report(*this, *I);
+      RangedBugReport report(*this, *I);
+      
+      ExplodedNode<ValueState>* N = *I;
+      Stmt *S = cast<PostStmt>(N->getLocation()).getStmt();
+      Expr* E = cast<ObjCMessageExpr>(S)->getReceiver();
+      assert (E && "Receiver cannot be NULL");
+      report.addRange(E->getSourceRange());
       
       // Emit the warning.
-      BR.EmitPathWarning(report, *I);
+      BR.EmitPathWarning(report);
     }    
   }
 };
@@ -291,6 +265,8 @@ public:
 } // end anonymous namespace
 
 void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) {
+  
+  // Path-sensitive checks.
   Eng.Register(new NullDeref());
   Eng.Register(new UndefDeref());
   Eng.Register(new UndefBranch());
@@ -302,6 +278,9 @@ void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) {
   Eng.Register(new BadMsgExprArg());
   Eng.Register(new BadReceiver());
   
+  // Flow-sensitive checks.
+  Eng.Register(MakeDeadStoresChecker());  
+  
   // Add extra checkers.
 
   GRSimpleAPICheck* FoundationCheck =