[cfe-commits] r71122 - in /cfe/branches/Apple/Dib: include/clang/Analysis/PathSensitive/BugReporter.h lib/Analysis/BugReporter.cpp lib/Analysis/CFRefCount.cpp
Mike Stump
mrs at apple.com
Wed May 6 14:52:39 PDT 2009
Author: mrs
Date: Wed May 6 16:52:39 2009
New Revision: 71122
URL: http://llvm.org/viewvc/llvm-project?rev=71122&view=rev
Log:
Merge in 71121:
Refactor BugReporter interface to have a new 'BugReporterContext' and
'BugReporterVisitor'. This simplifies callbacks from BugReporter to BugReports
(via VisitNode). It also lays the foundation for arbitrary visitor "call backs"
that can be registered to a BugReporterContext as a PathDiagnostic is
constructed. These call backs can help operate as separate "experts" that can
work on constructed pieces of a PathDiagnostic for which they possess special
knowledge.
Modified:
cfe/branches/Apple/Dib/include/clang/Analysis/PathSensitive/BugReporter.h
cfe/branches/Apple/Dib/lib/Analysis/BugReporter.cpp
cfe/branches/Apple/Dib/lib/Analysis/CFRefCount.cpp
Modified: cfe/branches/Apple/Dib/include/clang/Analysis/PathSensitive/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/Dib/include/clang/Analysis/PathSensitive/BugReporter.h?rev=71122&r1=71121&r2=71122&view=diff
==============================================================================
--- cfe/branches/Apple/Dib/include/clang/Analysis/PathSensitive/BugReporter.h (original)
+++ cfe/branches/Apple/Dib/include/clang/Analysis/PathSensitive/BugReporter.h Wed May 6 16:52:39 2009
@@ -34,6 +34,7 @@
class ASTContext;
class Diagnostic;
class BugReporter;
+class BugReporterContext;
class GRExprEngine;
class GRState;
class Stmt;
@@ -43,9 +44,19 @@
//===----------------------------------------------------------------------===//
// Interface for individual bug reports.
//===----------------------------------------------------------------------===//
+
+class BugReporterVisitor {
+public:
+ virtual ~BugReporterVisitor();
+ virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
+ const ExplodedNode<GRState>* PrevN,
+ BugReporterContext& BR) = 0;
+
+ virtual bool isOwnedByReporterContext() { return true; }
+};
// FIXME: Combine this with RangedBugReport and remove RangedBugReport.
-class BugReport {
+class BugReport : public BugReporterVisitor {
protected:
BugType& BT;
std::string ShortDescription;
@@ -76,8 +87,9 @@
const ExplodedNode<GRState> *n)
: BT(bt), ShortDescription(shortDesc), Description(desc), EndNode(n) {}
-
virtual ~BugReport();
+
+ virtual bool isOwnedByReporterContext() { return false; }
const BugType& getBugType() const { return BT; }
BugType& getBugType() { return BT; }
@@ -87,6 +99,8 @@
// FIXME: Do we need this? Maybe getLocation() should return a ProgramPoint
// object.
+ // FIXME: If we do need it, we can probably just make it private to
+ // BugReporter.
Stmt* getStmt(BugReporter& BR) const;
const std::string& getDescription() const { return Description; }
@@ -101,7 +115,7 @@
}
// FIXME: Perhaps move this into a subclass.
- virtual PathDiagnosticPiece* getEndPath(BugReporter& BR,
+ virtual PathDiagnosticPiece* getEndPath(BugReporterContext& BR,
const ExplodedNode<GRState>* N);
/// getLocation - Return the "definitive" location of the reported bug.
@@ -114,12 +128,17 @@
virtual void getRanges(BugReporter& BR,const SourceRange*& beg,
const SourceRange*& end);
- // FIXME: Perhaps this should be moved into a subclass?
+ virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
+ const ExplodedNode<GRState>* PrevN,
+ BugReporterContext& BR);
+
+ /*
virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
const ExplodedGraph<GRState>& G,
- BugReporter& BR,
+ BugReporterContext& BR,
NodeResolver& NR);
+ */
};
//===----------------------------------------------------------------------===//
@@ -332,7 +351,7 @@
static bool classof(const BugReporter* R) { return true; }
};
-
+
// FIXME: Get rid of GRBugReporter. It's the wrong abstraction.
class GRBugReporter : public BugReporter {
GRExprEngine& Eng;
@@ -371,6 +390,58 @@
return R->getKind() == GRBugReporterKind;
}
};
+
+class BugReporterContext {
+ GRBugReporter &BR;
+ std::vector<BugReporterVisitor*> Callbacks;
+public:
+ BugReporterContext(GRBugReporter& br) : BR(br) {}
+ virtual ~BugReporterContext();
+
+ void addVisitor(BugReporterVisitor* visitor) {
+ if (visitor) Callbacks.push_back(visitor);
+ }
+
+ typedef std::vector<BugReporterVisitor*>::iterator visitor_iterator;
+ visitor_iterator visitor_begin() { return Callbacks.begin(); }
+ visitor_iterator visitor_end() { return Callbacks.end(); }
+
+ GRBugReporter& getBugReporter() { return BR; }
+
+ ExplodedGraph<GRState>& getGraph() { return BR.getGraph(); }
+
+ void addNotableSymbol(SymbolRef Sym) {
+ // FIXME: For now forward to GRBugReporter.
+ BR.addNotableSymbol(Sym);
+ }
+
+ bool isNotable(SymbolRef Sym) const {
+ // FIXME: For now forward to GRBugReporter.
+ return BR.isNotable(Sym);
+ }
+
+ GRStateManager& getStateManager() {
+ return BR.getStateManager();
+ }
+
+ ASTContext& getASTContext() {
+ return BR.getContext();
+ }
+
+ SourceManager& getSourceManager() {
+ return BR.getSourceManager();
+ }
+
+ const Decl& getCodeDecl() {
+ return getStateManager().getCodeDecl();
+ }
+
+ const CFG& getCFG() {
+ return *BR.getCFG();
+ }
+
+ virtual BugReport::NodeResolver& getNodeResolver() = 0;
+};
class DiagBugReport : public RangedBugReport {
std::list<std::string> Strs;
Modified: cfe/branches/Apple/Dib/lib/Analysis/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/Dib/lib/Analysis/BugReporter.cpp?rev=71122&r1=71121&r2=71122&view=diff
==============================================================================
--- cfe/branches/Apple/Dib/lib/Analysis/BugReporter.cpp (original)
+++ cfe/branches/Apple/Dib/lib/Analysis/BugReporter.cpp Wed May 6 16:52:39 2009
@@ -30,6 +30,12 @@
using namespace clang;
+BugReporterVisitor::~BugReporterVisitor() {}
+BugReporterContext::~BugReporterContext() {
+ for (visitor_iterator I = visitor_begin(), E = visitor_end(); I != E; ++I)
+ if ((*I)->isOwnedByReporterContext()) delete *I;
+}
+
//===----------------------------------------------------------------------===//
// Helper routines for walking the ExplodedGraph and fetching statements.
//===----------------------------------------------------------------------===//
@@ -118,21 +124,20 @@
}
};
-class VISIBILITY_HIDDEN PathDiagnosticBuilder {
- GRBugReporter &BR;
- SourceManager &SMgr;
- ExplodedGraph<GRState> *ReportGraph;
+class VISIBILITY_HIDDEN PathDiagnosticBuilder : public BugReporterContext {
BugReport *R;
- const Decl& CodeDecl;
PathDiagnosticClient *PDC;
llvm::OwningPtr<ParentMap> PM;
NodeMapClosure NMC;
public:
- PathDiagnosticBuilder(GRBugReporter &br, ExplodedGraph<GRState> *reportGraph,
+ PathDiagnosticBuilder(GRBugReporter &br,
BugReport *r, NodeBackMap *Backmap,
- const Decl& codedecl, PathDiagnosticClient *pdc)
- : BR(br), SMgr(BR.getSourceManager()), ReportGraph(reportGraph), R(r),
- CodeDecl(codedecl), PDC(pdc), NMC(Backmap) {}
+ PathDiagnosticClient *pdc)
+ : BugReporterContext(br),
+ R(r), PDC(pdc), NMC(Backmap)
+ {
+ addVisitor(R);
+ }
PathDiagnosticLocation ExecutionContinues(const ExplodedNode<GRState>* N);
@@ -140,29 +145,17 @@
const ExplodedNode<GRState>* N);
ParentMap& getParentMap() {
- if (PM.get() == 0) PM.reset(new ParentMap(CodeDecl.getBody(getContext())));
+ if (PM.get() == 0)
+ PM.reset(new ParentMap(getCodeDecl().getBody(getASTContext())));
return *PM.get();
}
const Stmt *getParent(const Stmt *S) {
return getParentMap().getParent(S);
}
-
- const CFG& getCFG() {
- return *BR.getCFG();
- }
-
- const Decl& getCodeDecl() {
- return BR.getStateManager().getCodeDecl();
- }
-
- ExplodedGraph<GRState>& getGraph() { return *ReportGraph; }
- NodeMapClosure& getNodeMapClosure() { return NMC; }
- ASTContext& getContext() { return BR.getContext(); }
- SourceManager& getSourceManager() { return SMgr; }
+
+ virtual NodeMapClosure& getNodeResolver() { return NMC; }
BugReport& getReport() { return *R; }
- GRBugReporter& getBugReporter() { return BR; }
- GRStateManager& getStateManager() { return BR.getStateManager(); }
PathDiagnosticLocation getEnclosingStmtLocation(const Stmt *S);
@@ -187,9 +180,10 @@
PathDiagnosticLocation
PathDiagnosticBuilder::ExecutionContinues(const ExplodedNode<GRState>* N) {
if (Stmt *S = GetNextStmt(N))
- return PathDiagnosticLocation(S, SMgr);
+ return PathDiagnosticLocation(S, getSourceManager());
- return FullSourceLoc(CodeDecl.getBodyRBrace(getContext()), SMgr);
+ return FullSourceLoc(getCodeDecl().getBodyRBrace(getASTContext()),
+ getSourceManager());
}
PathDiagnosticLocation
@@ -204,10 +198,11 @@
if (Loc.asStmt())
os << "Execution continues on line "
- << SMgr.getInstantiationLineNumber(Loc.asLocation()) << '.';
+ << getSourceManager().getInstantiationLineNumber(Loc.asLocation())
+ << '.';
else
os << "Execution jumps to the end of the "
- << (isa<ObjCMethodDecl>(CodeDecl) ? "method" : "function") << '.';
+ << (isa<ObjCMethodDecl>(getCodeDecl()) ? "method" : "function") << '.';
return Loc;
}
@@ -216,6 +211,7 @@
PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) {
assert(S && "Null Stmt* passed to getEnclosingStmtLocation");
ParentMap &P = getParentMap();
+ SourceManager &SMgr = getSourceManager();
while (isa<Expr>(S) && P.isConsumedExpr(cast<Expr>(S))) {
const Stmt *Parent = P.getParent(S);
@@ -459,7 +455,7 @@
static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB,
const ExplodedNode<GRState> *N) {
- ASTContext& Ctx = PDB.getContext();
+
SourceManager& SMgr = PDB.getSourceManager();
const ExplodedNode<GRState>* NextNode = N->pred_empty()
? NULL : *(N->pred_begin());
@@ -541,7 +537,7 @@
}
if (GetRawInt)
- os << LHS->EvaluateAsInt(Ctx);
+ os << LHS->EvaluateAsInt(PDB.getASTContext());
os << ":' at line "
<< End.asLocation().getInstantiationLineNumber();
@@ -714,12 +710,11 @@
}
}
- if (PathDiagnosticPiece* p =
- PDB.getReport().VisitNode(N, NextNode, PDB.getGraph(),
- PDB.getBugReporter(),
- PDB.getNodeMapClosure())) {
- PD.push_front(p);
- }
+ for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
+ E = PDB.visitor_end(); I!=E; ++I) {
+ if (PathDiagnosticPiece* p = (*I)->VisitNode(N, NextNode, PDB))
+ PD.push_front(p);
+ }
if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {
// Scan the region bindings, and see if a "notable" symbol has a new
@@ -834,7 +829,7 @@
// statement (if it doesn't already exist).
// FIXME: Should handle CXXTryStmt if analyser starts supporting C++.
if (const CompoundStmt *CS =
- PDB.getCodeDecl().getCompoundBody(PDB.getContext()))
+ PDB.getCodeDecl().getCompoundBody(PDB.getASTContext()))
if (!CS->body_empty()) {
SourceLocation Loc = (*CS->body_begin())->getLocStart();
rawAddEdge(PathDiagnosticLocation(Loc, PDB.getSourceManager()));
@@ -1105,17 +1100,16 @@
continue;
}
- PathDiagnosticPiece* p =
- PDB.getReport().VisitNode(N, NextNode, PDB.getGraph(),
- PDB.getBugReporter(), PDB.getNodeMapClosure());
-
- if (p) {
- const PathDiagnosticLocation &Loc = p->getLocation();
- EB.addEdge(Loc, true);
- PD.push_front(p);
- if (const Stmt *S = Loc.asStmt())
- EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
- }
+ for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
+ E = PDB.visitor_end(); I!=E; ++I) {
+ if (PathDiagnosticPiece* p = (*I)->VisitNode(N, NextNode, PDB)) {
+ const PathDiagnosticLocation &Loc = p->getLocation();
+ EB.addEdge(Loc, true);
+ PD.push_front(p);
+ if (const Stmt *S = Loc.asStmt())
+ EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
+ }
+ }
}
}
@@ -1144,19 +1138,19 @@
}
PathDiagnosticPiece*
-BugReport::getEndPath(BugReporter& BR,
+BugReport::getEndPath(BugReporterContext& BRC,
const ExplodedNode<GRState>* EndPathNode) {
- Stmt* S = getStmt(BR);
+ Stmt* S = getStmt(BRC.getBugReporter());
if (!S)
return NULL;
- FullSourceLoc L(S->getLocStart(), BR.getContext().getSourceManager());
+ FullSourceLoc L(S->getLocStart(), BRC.getSourceManager());
PathDiagnosticPiece* P = new PathDiagnosticEventPiece(L, getDescription());
const SourceRange *Beg, *End;
- getRanges(BR, Beg, End);
+ getRanges(BRC.getBugReporter(), Beg, End);
for (; Beg != End; ++Beg)
P->addRange(*Beg);
@@ -1192,9 +1186,7 @@
PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
- const ExplodedGraph<GRState>& G,
- BugReporter& BR,
- NodeResolver &NR) {
+ BugReporterContext &BRC) {
return NULL;
}
@@ -1203,7 +1195,7 @@
//===----------------------------------------------------------------------===//
BugReportEquivClass::~BugReportEquivClass() {
- for (iterator I=begin(), E=end(); I!=E; ++I) delete *I;
+ for (iterator I=begin(), E=end(); I!=E; ++I) delete *I;
}
GRBugReporter::~GRBugReporter() { FlushReports(); }
@@ -1477,7 +1469,7 @@
}
void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
- BugReportEquivClass& EQ) {
+ BugReportEquivClass& EQ) {
std::vector<const ExplodedNode<GRState>*> Nodes;
@@ -1507,19 +1499,18 @@
llvm::OwningPtr<NodeBackMap> BackMap(GPair.first.second);
const ExplodedNode<GRState> *N = GPair.second.first;
- // Start building the path diagnostic...
- if (PathDiagnosticPiece* Piece = R->getEndPath(*this, N))
+ // Start building the path diagnostic...
+ PathDiagnosticBuilder PDB(*this, R, BackMap.get(), getPathDiagnosticClient());
+
+ if (PathDiagnosticPiece* Piece = R->getEndPath(PDB, N))
PD.push_back(Piece);
else
return;
- PathDiagnosticBuilder PDB(*this, ReportGraph.get(), R, BackMap.get(),
- getStateManager().getCodeDecl(),
- getPathDiagnosticClient());
switch (PDB.getGenerationScheme()) {
case PathDiagnosticClient::Extensive:
- GenerateExtensivePathDiagnostic(PD,PDB, N);
+ GenerateExtensivePathDiagnostic(PD, PDB, N);
break;
case PathDiagnosticClient::Minimal:
GenerateMinimalPathDiagnostic(PD, PDB, N);
Modified: cfe/branches/Apple/Dib/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/Dib/lib/Analysis/CFRefCount.cpp?rev=71122&r1=71121&r2=71122&view=diff
==============================================================================
--- cfe/branches/Apple/Dib/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/branches/Apple/Dib/lib/Analysis/CFRefCount.cpp Wed May 6 16:52:39 2009
@@ -22,7 +22,7 @@
#include "clang/Analysis/PathDiagnostic.h"
#include "clang/Analysis/PathSensitive/BugReporter.h"
#include "clang/Analysis/PathSensitive/SymbolManager.h"
-#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclObjC.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/ImmutableMap.h"
@@ -1975,16 +1975,14 @@
SymbolRef getSymbol() const { return Sym; }
- PathDiagnosticPiece* getEndPath(BugReporter& BR,
+ PathDiagnosticPiece* getEndPath(BugReporterContext& BRC,
const ExplodedNode<GRState>* N);
std::pair<const char**,const char**> getExtraDescriptiveText();
PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
- const ExplodedGraph<GRState>& G,
- BugReporter& BR,
- NodeResolver& NR);
+ BugReporterContext& BRC);
};
class VISIBILITY_HIDDEN CFRefLeakReport : public CFRefReport {
@@ -1995,7 +1993,7 @@
ExplodedNode<GRState> *n, SymbolRef sym,
GRExprEngine& Eng);
- PathDiagnosticPiece* getEndPath(BugReporter& BR,
+ PathDiagnosticPiece* getEndPath(BugReporterContext& BRC,
const ExplodedNode<GRState>* N);
SourceLocation getLocation() const { return AllocSite; }
@@ -2096,12 +2094,10 @@
PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
- const ExplodedGraph<GRState>& G,
- BugReporter& BR,
- NodeResolver& NR) {
+ BugReporterContext& BRC) {
- // Check if the type state has changed.
- GRStateManager &StMgr = cast<GRBugReporter>(BR).getStateManager();
+ // Check if the type state has changed.
+ GRStateManager &StMgr = BRC.getStateManager();
GRStateRef PrevSt(PrevN->getState(), StMgr);
GRStateRef CurrSt(N->getState(), StMgr);
@@ -2156,7 +2152,7 @@
os << "+0 retain count (non-owning reference).";
}
- PathDiagnosticLocation Pos(S, BR.getContext().getSourceManager());
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager());
return new PathDiagnosticEventPiece(Pos, os.str());
}
@@ -2164,7 +2160,8 @@
// program point
llvm::SmallVector<ArgEffect, 2> AEffects;
- if (const RetainSummary *Summ = TF.getSummaryOfNode(NR.getOriginalNode(N))) {
+ if (const RetainSummary *Summ =
+ TF.getSummaryOfNode(BRC.getNodeResolver().getOriginalNode(N))) {
// We only have summaries attached to nodes after evaluating CallExpr and
// ObjCMessageExprs.
Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
@@ -2313,7 +2310,7 @@
return 0; // We have nothing to say!
Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
- PathDiagnosticLocation Pos(S, BR.getContext().getSourceManager());
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager());
PathDiagnosticPiece* P = new PathDiagnosticEventPiece(Pos, os.str());
// Add the range by scanning the children of the statement for any bindings
@@ -2388,21 +2385,21 @@
}
PathDiagnosticPiece*
-CFRefReport::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN) {
- // Tell the BugReporter to report cases when the tracked symbol is
+CFRefReport::getEndPath(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* EndN) {
+ // Tell the BugReporterContext to report cases when the tracked symbol is
// assigned to different variables, etc.
- GRBugReporter& BR = cast<GRBugReporter>(br);
- cast<GRBugReporter>(BR).addNotableSymbol(Sym);
- return RangedBugReport::getEndPath(BR, EndN);
+ BRC.addNotableSymbol(Sym);
+ return RangedBugReport::getEndPath(BRC, EndN);
}
PathDiagnosticPiece*
-CFRefLeakReport::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN){
+CFRefLeakReport::getEndPath(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* EndN){
- GRBugReporter& BR = cast<GRBugReporter>(br);
- // Tell the BugReporter to report cases when the tracked symbol is
+ // Tell the BugReporterContext to report cases when the tracked symbol is
// assigned to different variables, etc.
- cast<GRBugReporter>(BR).addNotableSymbol(Sym);
+ BRC.addNotableSymbol(Sym);
// We are reporting 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
@@ -2411,13 +2408,13 @@
const MemRegion* FirstBinding = 0;
llvm::tie(AllocNode, FirstBinding) =
- GetAllocationSite(BR.getStateManager(), EndN, Sym);
+ GetAllocationSite(BRC.getStateManager(), EndN, Sym);
// Get the allocate site.
assert(AllocNode);
Stmt* FirstStmt = cast<PostStmt>(AllocNode->getLocation()).getStmt();
- SourceManager& SMgr = BR.getContext().getSourceManager();
+ SourceManager& SMgr = BRC.getSourceManager();
unsigned AllocLine =SMgr.getInstantiationLineNumber(FirstStmt->getLocStart());
// Compute an actual location for the leak. Sometimes a leak doesn't
@@ -2444,8 +2441,8 @@
}
if (!L.isValid()) {
- const Decl &D = BR.getStateManager().getCodeDecl();
- L = PathDiagnosticLocation(D.getBodyRBrace(BR.getContext()), SMgr);
+ const Decl &D = BRC.getCodeDecl();
+ L = PathDiagnosticLocation(D.getBodyRBrace(BRC.getASTContext()), SMgr);
}
std::string sbuf;
@@ -2463,7 +2460,7 @@
// FIXME: Per comments in rdar://6320065, "create" only applies to CF
// ojbects. Only "copy", "alloc", "retain" and "new" transfer ownership
// to the caller for NS objects.
- ObjCMethodDecl& MD = cast<ObjCMethodDecl>(BR.getGraph().getCodeDecl());
+ ObjCMethodDecl& MD = cast<ObjCMethodDecl>(BRC.getCodeDecl());
os << " is returned from a method whose name ('"
<< MD.getSelector().getAsString()
<< "') does not contain 'copy' or otherwise starts with"
More information about the cfe-commits
mailing list