More hacking on static analyzer diagnostics. When emitting summary diagnostics the code paths for diagnostics involving paths or single locations are now unified. This patch also constifies many arguments/methods that are touched by this logic, leading to a nice overall code cleanup.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@62903 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index 9d4d059..b08f5cb 100644
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -64,27 +64,30 @@
return (*B)[0];
}
-static inline ExplodedNode<GRState>*
-GetNextNode(ExplodedNode<GRState>* N) {
+static inline const ExplodedNode<GRState>*
+GetNextNode(const ExplodedNode<GRState>* N) {
return N->pred_empty() ? NULL : *(N->pred_begin());
}
-static Stmt* GetLastStmt(ExplodedNode<GRState>* N) {
+static Stmt* GetLastStmt(const ExplodedNode<GRState>* N) {
assert (isa<BlockEntrance>(N->getLocation()));
for (N = GetNextNode(N); N; N = GetNextNode(N)) {
-
ProgramPoint P = N->getLocation();
-
- if (PostStmt* PS = dyn_cast<PostStmt>(&P))
- return PS->getStmt();
+ if (PostStmt* PS = dyn_cast<PostStmt>(&P)) return PS->getStmt();
}
return NULL;
}
+static inline Stmt* GetStmt(const ExplodedNode<GRState>* N) {
+ ProgramPoint ProgP = N->getLocation();
+ return isa<BlockEntrance>(ProgP) ? GetLastStmt(N) : GetStmt(ProgP);
+}
+
+
static void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr,
- Stmt* S) {
+ const Stmt* S) {
if (!S)
return;
@@ -100,7 +103,7 @@
static inline void ExecutionContinues(std::ostringstream& os,
SourceManager& SMgr,
- ExplodedNode<GRState>* N) {
+ const ExplodedNode<GRState>* N) {
ExecutionContinues(os, SMgr, GetStmt(N->getLocation()));
}
@@ -129,7 +132,7 @@
PathDiagnosticPiece*
BugReport::getEndPath(BugReporter& BR,
- ExplodedNode<GRState>* EndPathNode) {
+ const ExplodedNode<GRState>* EndPathNode) {
Stmt* S = getStmt(BR);
@@ -165,7 +168,7 @@
if (!EndNode)
return FullSourceLoc();
- Stmt* S = GetStmt(EndNode->getLocation());
+ Stmt* S = GetStmt(EndNode);
if (!S)
return FullSourceLoc();
@@ -173,21 +176,22 @@
return FullSourceLoc(S->getLocStart(), Mgr);
}
-PathDiagnosticPiece* BugReport::VisitNode(ExplodedNode<GRState>* N,
- ExplodedNode<GRState>* PrevN,
- ExplodedGraph<GRState>& G,
+PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N,
+ const ExplodedNode<GRState>* PrevN,
+ const ExplodedGraph<GRState>& G,
BugReporter& BR) {
return NULL;
}
static std::pair<ExplodedGraph<GRState>*, ExplodedNode<GRState>*>
-MakeReportGraph(ExplodedGraph<GRState>* G, ExplodedNode<GRState>* N) {
+MakeReportGraph(const ExplodedGraph<GRState>* G,
+ const ExplodedNode<GRState>* N) {
- llvm::OwningPtr<ExplodedGraph<GRState> > GTrim(G->Trim(&N, &N+1));
+ llvm::OwningPtr< ExplodedGraph<GRState> > GTrim(G->Trim(&N, &N+1));
// Find the error node in the trimmed graph.
- ExplodedNode<GRState>* NOld = N;
+ const ExplodedNode<GRState>* NOld = N;
N = 0;
for (ExplodedGraph<GRState>::node_iterator
@@ -203,21 +207,21 @@
assert(N);
// Create a new graph with a single path.
-
- G = new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
- GTrim->getContext());
+ ExplodedGraph<GRState> *GNew =
+ new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
+ GTrim->getContext());
// Sometimes TrimGraph can contain a cycle. Perform a reverse DFS
// to the root node, and then construct a new graph that contains only
// a single path.
- llvm::DenseMap<void*,unsigned> Visited;
- llvm::SmallVector<ExplodedNode<GRState>*, 10> WS;
+ llvm::DenseMap<const void*,unsigned> Visited;
+ llvm::SmallVector<const ExplodedNode<GRState>*, 10> WS;
WS.push_back(N);
unsigned cnt = 0;
- ExplodedNode<GRState>* Root = 0;
+ const ExplodedNode<GRState>* Root = 0;
while (!WS.empty()) {
- ExplodedNode<GRState>* Node = WS.back();
+ const ExplodedNode<GRState>* Node = WS.back();
WS.pop_back();
if (Visited.find(Node) != Visited.end())
@@ -230,7 +234,7 @@
break;
}
- for (ExplodedNode<GRState>::pred_iterator I=Node->pred_begin(),
+ for (ExplodedNode<GRState>::const_pred_iterator I=Node->pred_begin(),
E=Node->pred_end(); I!=E; ++I)
WS.push_back(*I);
}
@@ -242,15 +246,14 @@
ExplodedNode<GRState> *Last = 0, *First = 0;
for ( N = Root ;;) {
-
// Lookup the number associated with the current node.
- llvm::DenseMap<void*,unsigned>::iterator I=Visited.find(N);
+ llvm::DenseMap<const void*,unsigned>::iterator I = Visited.find(N);
assert (I != Visited.end());
// Create the equivalent node in the new graph with the same state
// and location.
ExplodedNode<GRState>* NewN =
- G->getNode(N->getLocation(), N->getState());
+ GNew->getNode(N->getLocation(), N->getState());
// Link up the new node with the previous node.
if (Last)
@@ -266,8 +269,8 @@
// Find the next successor node. We choose the node that is marked
// with the lowest DFS number.
- ExplodedNode<GRState>::succ_iterator SI = N->succ_begin();
- ExplodedNode<GRState>::succ_iterator SE = N->succ_end();
+ ExplodedNode<GRState>::const_succ_iterator SI = N->succ_begin();
+ ExplodedNode<GRState>::const_succ_iterator SE = N->succ_end();
N = 0;
for (unsigned MinVal = 0; SI != SE; ++SI) {
@@ -287,12 +290,12 @@
}
assert (First);
- return std::make_pair(G, First);
+ return std::make_pair(GNew, First);
}
-static VarDecl* GetMostRecentVarDeclBinding(ExplodedNode<GRState>* N,
- GRStateManager& VMgr,
- SVal X) {
+static const VarDecl*
+GetMostRecentVarDeclBinding(const ExplodedNode<GRState>* N,
+ GRStateManager& VMgr, SVal X) {
for ( ; N ; N = N->pred_empty() ? 0 : *N->pred_begin()) {
@@ -328,16 +331,16 @@
SymbolRef Sym;
const GRState* PrevSt;
- Stmt* S;
+ const Stmt* S;
GRStateManager& VMgr;
- ExplodedNode<GRState>* Pred;
+ const ExplodedNode<GRState>* Pred;
PathDiagnostic& PD;
BugReporter& BR;
public:
- NotableSymbolHandler(SymbolRef sym, const GRState* prevst, Stmt* s,
- GRStateManager& vmgr, ExplodedNode<GRState>* pred,
+ NotableSymbolHandler(SymbolRef sym, const GRState* prevst, const Stmt* s,
+ GRStateManager& vmgr, const ExplodedNode<GRState>* pred,
PathDiagnostic& pd, BugReporter& br)
: Sym(sym), PrevSt(prevst), S(s), VMgr(vmgr), Pred(pred), PD(pd), BR(br) {}
@@ -367,7 +370,7 @@
VarDecl *VD = 0;
- if (BinaryOperator* B = dyn_cast<BinaryOperator>(S)) {
+ if (const BinaryOperator* B = dyn_cast<BinaryOperator>(S)) {
if (!B->isAssignmentOp())
return true;
@@ -379,7 +382,7 @@
VD = dyn_cast<VarDecl>(DR->getDecl());
}
- else if (DeclStmt* DS = dyn_cast<DeclStmt>(S)) {
+ else if (const DeclStmt* DS = dyn_cast<DeclStmt>(S)) {
// FIXME: Eventually CFGs won't have DeclStmts. Right now we
// assume that each DeclStmt has a single Decl. This invariant
// holds by contruction in the CFG.
@@ -390,7 +393,7 @@
return true;
// What is the most recently referenced variable with this binding?
- VarDecl* MostRecent = GetMostRecentVarDeclBinding(Pred, VMgr, V);
+ const VarDecl* MostRecent = GetMostRecentVarDeclBinding(Pred, VMgr, V);
if (!MostRecent)
return true;
@@ -411,11 +414,12 @@
};
}
-static void HandleNotableSymbol(ExplodedNode<GRState>* N, Stmt* S,
+static void HandleNotableSymbol(const ExplodedNode<GRState>* N,
+ const Stmt* S,
SymbolRef Sym, BugReporter& BR,
PathDiagnostic& PD) {
- ExplodedNode<GRState>* Pred = N->pred_empty() ? 0 : *N->pred_begin();
+ const ExplodedNode<GRState>* Pred = N->pred_empty() ? 0 : *N->pred_begin();
const GRState* PrevSt = Pred ? Pred->getState() : 0;
if (!PrevSt)
@@ -433,13 +437,13 @@
: public StoreManager::BindingsHandler {
llvm::SmallSet<SymbolRef, 10> AlreadyProcessed;
- ExplodedNode<GRState>* N;
+ const ExplodedNode<GRState>* N;
Stmt* S;
GRBugReporter& BR;
PathDiagnostic& PD;
public:
- ScanNotableSymbols(ExplodedNode<GRState>* n, Stmt* s, GRBugReporter& br,
+ ScanNotableSymbols(const ExplodedNode<GRState>* n, Stmt* s, GRBugReporter& br,
PathDiagnostic& pd)
: N(n), S(s), BR(br), PD(pd) {}
@@ -472,14 +476,14 @@
void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
BugReport& R) {
- ExplodedNode<GRState>* N = R.getEndNode();
+ const ExplodedNode<GRState>* N = R.getEndNode();
if (!N) return;
// Construct a new graph that contains only a single path from the error
// node to a root.
- const std::pair<ExplodedGraph<GRState>*,ExplodedNode<GRState>*>
+ const std::pair<ExplodedGraph<GRState>*, ExplodedNode<GRState>*>
GPair = MakeReportGraph(&getGraph(), N);
llvm::OwningPtr<ExplodedGraph<GRState> > ReportGraph(GPair.first);
@@ -493,15 +497,15 @@
else
return;
- ExplodedNode<GRState>* NextNode = N->pred_empty()
- ? NULL : *(N->pred_begin());
+ const ExplodedNode<GRState>* NextNode = N->pred_empty()
+ ? NULL : *(N->pred_begin());
ASTContext& Ctx = getContext();
SourceManager& SMgr = Ctx.getSourceManager();
while (NextNode) {
- ExplodedNode<GRState>* LastNode = N;
+ const ExplodedNode<GRState>* LastNode = N;
N = NextNode;
NextNode = GetNextNode(N);
@@ -697,7 +701,7 @@
bool BugTypeCacheLocation::isCached(BugReport& R) {
- ExplodedNode<GRState>* N = R.getEndNode();
+ const ExplodedNode<GRState>* N = R.getEndNode();
if (!N)
return false;
@@ -727,73 +731,37 @@
GeneratePathDiagnostic(*D.get(), R);
// Get the meta data.
-
std::pair<const char**, const char**> Meta = R.getExtraDescriptiveText();
-
- for (const char** s = Meta.first; s != Meta.second; ++s)
- D->addMeta(*s);
+ for (const char** s = Meta.first; s != Meta.second; ++s) D->addMeta(*s);
- // Emit a full diagnostic for the path if we have a PathDiagnosticClient.
-
+ // Emit a summary diagnostic to the regular Diagnostics engine.
PathDiagnosticClient* PD = getPathDiagnosticClient();
-
- if (PD && !D->empty()) {
- PD->HandlePathDiagnostic(D.take());
- // Output a diagnostic summarizing the report.
- Diagnostic& Diag = getDiagnostic();
- Diag.Report(R.getLocation(getSourceManager()),
- Diag.getCustomDiagID(Diagnostic::Warning,R.getDescription()));
- return;
- }
-
- // This isn't a bug with a path, but we can still emit a single
- // line diagnostic. Determine the location.
- FullSourceLoc L = D->empty() ? R.getLocation(getSourceManager())
- : D->back()->getLocation();
-
- // Determine the range.
- const SourceRange *Beg, *End;
+ const SourceRange *Beg = 0, *End = 0;
+ R.getRanges(*this, Beg, End);
+ Diagnostic& Diag = getDiagnostic();
+ FullSourceLoc L = R.getLocation(getSourceManager());
+ const char *msg = PD ? R.getBugType().getName() : R.getDescription();
+ unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning, msg);
- if (!D->empty()) {
- Beg = D->back()->ranges_begin();
- End = D->back()->ranges_end();
- }
- else
- R.getRanges(*this, Beg, End);
-
- if (PD) {
- PathDiagnosticPiece* piece = new PathDiagnosticPiece(L, R.getDescription());
-
- for ( ; Beg != End; ++Beg)
- piece->addRange(*Beg);
-
- D->push_back(piece);
- PD->HandlePathDiagnostic(D.take());
-
- // Output a diagnostic summarizing the report.
- Diagnostic& Diag = getDiagnostic();
- Diag.Report(L,Diag.getCustomDiagID(Diagnostic::Warning,R.getDescription()));
- return;
- }
- else {
- std::string str;
-
- if (D->empty())
- str = R.getDescription();
- else
- str = D->back()->getString();
-
- Diagnostic& Diag = getDiagnostic();
- unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning, str.c_str());
-
- switch (End-Beg) {
+ switch (End-Beg) {
default: assert(0 && "Don't handle this many ranges yet!");
case 0: Diag.Report(L, ErrorDiag); break;
case 1: Diag.Report(L, ErrorDiag) << Beg[0]; break;
case 2: Diag.Report(L, ErrorDiag) << Beg[0] << Beg[1]; break;
case 3: Diag.Report(L, ErrorDiag) << Beg[0] << Beg[1] << Beg[2]; break;
- }
}
+
+ // Emit a full diagnostic for the path if we have a PathDiagnosticClient.
+ if (!PD)
+ return;
+
+ if (D->empty()) {
+ PathDiagnosticPiece* piece = new PathDiagnosticPiece(L, R.getDescription());
+ for ( ; Beg != End; ++Beg) piece->addRange(*Beg);
+ D->push_back(piece);
+ }
+
+ PD->HandlePathDiagnostic(D.take());
}
void BugReporter::EmitBasicReport(const char* name, const char* str,
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 4731921..bfc9d90 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -2215,11 +2215,7 @@
return "[naming convention] leak of returned object";
}
}
-
- virtual const char* getDescription() const {
- return "Object leaked";
- }
-
+
virtual void EmitWarnings(BugReporter& BR);
virtual void GetErrorNodes(std::vector<ExplodedNode<GRState>*>& Nodes);
virtual bool isLeak() const { return true; }
@@ -2256,15 +2252,15 @@
SymbolRef getSymbol() const { return Sym; }
- virtual PathDiagnosticPiece* getEndPath(BugReporter& BR,
- ExplodedNode<GRState>* N);
+ PathDiagnosticPiece* getEndPath(BugReporter& BR,
+ const ExplodedNode<GRState>* N);
- virtual std::pair<const char**,const char**> getExtraDescriptiveText();
+ std::pair<const char**,const char**> getExtraDescriptiveText();
- virtual PathDiagnosticPiece* VisitNode(ExplodedNode<GRState>* N,
- ExplodedNode<GRState>* PrevN,
- ExplodedGraph<GRState>& G,
- BugReporter& BR);
+ PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
+ const ExplodedNode<GRState>* PrevN,
+ const ExplodedGraph<GRState>& G,
+ BugReporter& BR);
};
@@ -2313,9 +2309,9 @@
}
}
-PathDiagnosticPiece* CFRefReport::VisitNode(ExplodedNode<GRState>* N,
- ExplodedNode<GRState>* PrevN,
- ExplodedGraph<GRState>& G,
+PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
+ const ExplodedNode<GRState>* PrevN,
+ const ExplodedGraph<GRState>& G,
BugReporter& BR) {
// Check if the type state has changed.
@@ -2486,14 +2482,14 @@
};
}
-static std::pair<ExplodedNode<GRState>*,MemRegion*>
-GetAllocationSite(GRStateManager* StateMgr, ExplodedNode<GRState>* N,
+static std::pair<const ExplodedNode<GRState>*,const MemRegion*>
+GetAllocationSite(GRStateManager* StateMgr, const ExplodedNode<GRState>* N,
SymbolRef Sym) {
// Find both first node that referred to the tracked symbol and the
// memory location that value was store to.
- ExplodedNode<GRState>* Last = N;
- MemRegion* FirstBinding = 0;
+ const ExplodedNode<GRState>* Last = N;
+ const MemRegion* FirstBinding = 0;
while (N) {
const GRState* St = N->getState();
@@ -2515,8 +2511,8 @@
return std::make_pair(Last, FirstBinding);
}
-PathDiagnosticPiece* CFRefReport::getEndPath(BugReporter& br,
- ExplodedNode<GRState>* EndN) {
+PathDiagnosticPiece*
+CFRefReport::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN) {
GRBugReporter& BR = cast<GRBugReporter>(br);
@@ -2530,8 +2526,8 @@
// We are a leak. Walk up the graph to get to the first node where the
// symbol appeared, and also get the first VarDecl that tracked object
// is stored to.
- ExplodedNode<GRState>* AllocNode = 0;
- MemRegion* FirstBinding = 0;
+ const ExplodedNode<GRState>* AllocNode = 0;
+ const MemRegion* FirstBinding = 0;
llvm::tie(AllocNode, FirstBinding) =
GetAllocationSite(&BR.getStateManager(), EndN, Sym);
@@ -2566,7 +2562,7 @@
PathDiagnosticPiece::DisplayHint Hint = PathDiagnosticPiece::Above;
assert (!EndN->pred_empty()); // Not possible to have 0 predecessors.
- ExplodedNode<GRState> *Pred = *(EndN->pred_begin());
+ const ExplodedNode<GRState> *Pred = *(EndN->pred_begin());
ProgramPoint PredPos = Pred->getLocation();
if (PostStmt* PredPS = dyn_cast<PostStmt>(&PredPos)) {
@@ -2664,8 +2660,8 @@
SymbolRef Sym = static_cast<CFRefReport&>(R).getSymbol();
- ExplodedNode<GRState>* AllocNode =
- GetAllocationSite(0, R.getEndNode(), Sym).first;
+ const ExplodedNode<GRState>* AllocNode =
+ GetAllocationSite(0, R.getEndNode(), Sym).first;
if (!AllocNode)
return false;
diff --git a/lib/Analysis/ExplodedGraph.cpp b/lib/Analysis/ExplodedGraph.cpp
index 945416b..4e41b60 100644
--- a/lib/Analysis/ExplodedGraph.cpp
+++ b/lib/Analysis/ExplodedGraph.cpp
@@ -120,36 +120,35 @@
if (getKind() == SizeOther) delete &getVector(getPtr());
}
-ExplodedGraphImpl* ExplodedGraphImpl::Trim(ExplodedNodeImpl** BeginSources,
- ExplodedNodeImpl** EndSources) const{
+ExplodedGraphImpl*
+ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources,
+ const ExplodedNodeImpl* const* EndSources) const{
- typedef llvm::DenseMap<ExplodedNodeImpl*, ExplodedNodeImpl*> Pass1Ty;
- typedef llvm::DenseMap<ExplodedNodeImpl*, ExplodedNodeImpl*> Pass2Ty;
+ typedef llvm::DenseMap<const ExplodedNodeImpl*, const ExplodedNodeImpl*> Pass1Ty;
+ typedef llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*> Pass2Ty;
Pass1Ty Pass1;
Pass2Ty Pass2;
- llvm::SmallVector<ExplodedNodeImpl*, 10> WL2;
+ llvm::SmallVector<const ExplodedNodeImpl*, 10> WL2;
{ // ===- Pass 1 (reverse BFS) -===
// Enqueue the source nodes to the first worklist.
- std::list<std::pair<ExplodedNodeImpl*, ExplodedNodeImpl*> > WL1;
- std::list<std::pair<ExplodedNodeImpl*, ExplodedNodeImpl*> > WL1_Loops;
+ std::list<std::pair<const ExplodedNodeImpl*,
+ const ExplodedNodeImpl*> > WL1, WL1_Loops;
- for (ExplodedNodeImpl** I = BeginSources; I != EndSources; ++I)
+ for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources; ++I)
WL1.push_back(std::make_pair(*I, *I));
// Process the worklist.
while (! (WL1.empty() && WL1_Loops.empty())) {
-
- ExplodedNodeImpl *N, *Src;
-
// Only dequeue from the "loops" worklist if WL1 has no items.
// Thus we prioritize for paths that don't span loop boundaries.
-
+ const ExplodedNodeImpl *N, *Src;
+
if (WL1.empty()) {
N = WL1_Loops.back().first;
Src = WL1_Loops.back().second;
@@ -227,7 +226,7 @@
while (!WL2.empty()) {
- ExplodedNodeImpl* N = WL2.back();
+ const ExplodedNodeImpl* N = WL2.back();
WL2.pop_back();
// Skip this node if we have already processed it.