From 88255cc533a094f1044062088921d2d79221d73c Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Sat, 20 Aug 2011 01:27:22 +0000 Subject: [PATCH] Static Analyzer Diagnostics: Move the responsibility for generating the endOfPath diagnostic piece from BugReport to BugReporterVisitor. Switch CFRefCount to use visitors in order to generate the endOfPath piece. llvm-svn: 138184 --- .../Core/BugReporter/BugReporter.h | 8 +-- .../Core/BugReporter/BugReporterVisitor.h | 16 +++++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 62 ++++++------------- .../Core/BugReporterVisitors.cpp | 48 ++++++++++++++ clang/lib/StaticAnalyzer/Core/CFRefCount.cpp | 55 +++++++++------- 5 files changed, 118 insertions(+), 71 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 07cc467521bf..cfe7048a7761 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -85,8 +85,6 @@ protected: /// for each bug. virtual void Profile(llvm::FoldingSetNodeID& hash) const; - const Stmt *getStmt() const; - public: BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode) : BT(bt), Description(desc), ErrorNode(errornode), @@ -121,10 +119,6 @@ public: return std::make_pair((const char**)0,(const char**)0); } - /// Provide custom definition for the last diagnostic piece on the path. - virtual PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, - const ExplodedNode *N); - /// \brief Return the "definitive" location of the reported bug. /// /// While a bug can span an entire path, usually there is a specific @@ -132,6 +126,8 @@ public: /// This location is used by clients rendering diagnostics. virtual SourceLocation getLocation() const; + const Stmt *getStmt() const; + /// \brief Add a range to a bug report. /// /// Ranges are used to highlight regions of interest in the source code. diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h index 98943d6c41c7..85ac9820db87 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h @@ -42,7 +42,23 @@ public: BugReporterContext &BRC, BugReport &BR) = 0; + /// \brief Provide custom definition for the final diagnostic piece on the + /// path - the piece, which is displayed before the path is expanded. + /// + /// If returns NULL the default implementation will be used. + /// Also note that at most one visitor of a BugReport should generate a + /// non-NULL end of path diagnostic piece. + virtual PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + BugReport &BR); + virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0; + + /// \brief Generates the default final diagnostic piece. + static PathDiagnosticPiece *getDefaultEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + BugReport &BR); + }; class FindLastStoreBRVisitor : public BugReporterVisitor { diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index df3ebb860d99..b82d12310e62 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -1261,45 +1261,6 @@ const Stmt *BugReport::getStmt() const { return S; } -PathDiagnosticPiece* -BugReport::getEndPath(BugReporterContext &BRC, - const ExplodedNode *EndPathNode) { - - const ProgramPoint &PP = EndPathNode->getLocation(); - PathDiagnosticLocation L; - - if (const BlockEntrance *BE = dyn_cast(&PP)) { - const CFGBlock *block = BE->getBlock(); - if (block->getBlockID() == 0) { - L = PathDiagnosticLocation( - EndPathNode->getLocationContext()->getDecl()->getBodyRBrace(), - BRC.getSourceManager()); - } - } - - if (!L.isValid()) { - const Stmt *S = getStmt(); - - if (!S) - return NULL; - - L = PathDiagnosticLocation(S, BRC.getSourceManager()); - } - - BugReport::ranges_iterator Beg, End; - llvm::tie(Beg, End) = getRanges(); - - // Only add the statement itself as a range if we didn't specify any - // special ranges for this report. - PathDiagnosticPiece *P = new PathDiagnosticEventPiece(L, getDescription(), - Beg == End); - - for (; Beg != End; ++Beg) - P->addRange(*Beg); - - return P; -} - std::pair BugReport::getRanges() { // If no custom ranges, add the range of the statement corresponding to @@ -1657,15 +1618,28 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, // 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; - // Register additional node visitors. R->addVisitor(new NilReceiverBRVisitor()); R->addVisitor(new ConditionBRVisitor()); + // Generate the very last diagnostic piece - the piece is visible before + // the trace is expanded. + PathDiagnosticPiece *LastPiece = 0; + for (BugReport::visitor_iterator I = R->visitor_begin(), + E = R->visitor_end(); I!=E; ++I) { + if (PathDiagnosticPiece *Piece = (*I)->getEndPath(PDB, N, *R)) { + assert (!LastPiece && + "There can only be one final piece in a diagnostic."); + LastPiece = Piece; + } + } + if (!LastPiece) + LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R); + if (LastPiece) + PD.push_back(LastPiece); + else + return; + switch (PDB.getGenerationScheme()) { case PathDiagnosticClient::Extensive: GenerateExtensivePathDiagnostic(PD, PDB, N); diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 9fbccf8f8136..9b5a60c0d6f1 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -72,6 +72,54 @@ const Stmt *bugreporter::GetRetValExpr(const ExplodedNode *N) { //===----------------------------------------------------------------------===// // Definitions for bug reporter visitors. //===----------------------------------------------------------------------===// + +PathDiagnosticPiece* +BugReporterVisitor::getEndPath(BugReporterContext &BRC, + const ExplodedNode *EndPathNode, + BugReport &BR) { + return 0; +} + +PathDiagnosticPiece* +BugReporterVisitor::getDefaultEndPath(BugReporterContext &BRC, + const ExplodedNode *EndPathNode, + BugReport &BR) { + const ProgramPoint &PP = EndPathNode->getLocation(); + PathDiagnosticLocation L; + + if (const BlockEntrance *BE = dyn_cast(&PP)) { + const CFGBlock *block = BE->getBlock(); + if (block->getBlockID() == 0) { + L = PathDiagnosticLocation( + EndPathNode->getLocationContext()->getDecl()->getBodyRBrace(), + BRC.getSourceManager()); + } + } + + if (!L.isValid()) { + const Stmt *S = BR.getStmt(); + + if (!S) + return NULL; + + L = PathDiagnosticLocation(S, BRC.getSourceManager()); + } + + BugReport::ranges_iterator Beg, End; + llvm::tie(Beg, End) = BR.getRanges(); + + // Only add the statement itself as a range if we didn't specify any + // special ranges for this report. + PathDiagnosticPiece *P = new PathDiagnosticEventPiece(L, + BR.getDescription(), + Beg == End); + for (; Beg != End; ++Beg) + P->addRange(*Beg); + + return P; +} + + void FindLastStoreBRVisitor ::Profile(llvm::FoldingSetNodeID &ID) const { static int tag = 0; ID.AddPointer(&tag); diff --git a/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp b/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp index 9e6d829a259e..836cb1519617 100644 --- a/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp +++ b/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp @@ -1960,23 +1960,38 @@ namespace { //===---------===// class CFRefReportVisitor : public BugReporterVisitor { + protected: SymbolRef Sym; const CFRefCount &TF; + public: - CFRefReportVisitor(SymbolRef sym, const CFRefCount &tf) : Sym(sym), TF(tf) {} - void Profile(llvm::FoldingSetNodeID &ID) const { + virtual void Profile(llvm::FoldingSetNodeID &ID) const { static int x = 0; ID.AddPointer(&x); ID.AddPointer(Sym); } - PathDiagnosticPiece *VisitNode(const ExplodedNode *N, - const ExplodedNode *PrevN, - BugReporterContext &BRC, - BugReport &BR); + virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR); + + virtual PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + BugReport &BR); + }; + + class CFRefLeakReportVisitor : public CFRefReportVisitor { + public: + CFRefLeakReportVisitor(SymbolRef sym, const CFRefCount &tf) + : CFRefReportVisitor(sym, tf) {} + + PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + BugReport &BR); }; class CFRefReport : public BugReport { @@ -1985,9 +2000,10 @@ namespace { const CFRefCount &TF; public: CFRefReport(CFRefBug& D, const CFRefCount &tf, - ExplodedNode *n, SymbolRef sym) + ExplodedNode *n, SymbolRef sym, bool registerVisitor = true) : BugReport(D, D.getDescription(), n), Sym(sym), TF(tf) { - addVisitor(new CFRefReportVisitor(sym, tf)); + if (registerVisitor) + addVisitor(new CFRefReportVisitor(sym, tf)); } CFRefReport(CFRefBug& D, const CFRefCount &tf, @@ -2011,23 +2027,18 @@ namespace { SymbolRef getSymbol() const { return Sym; } - PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, - const ExplodedNode *N); - std::pair getExtraDescriptiveText(); }; class CFRefLeakReport : public CFRefReport { SourceLocation AllocSite; const MemRegion* AllocBinding; + public: CFRefLeakReport(CFRefBug& D, const CFRefCount &tf, ExplodedNode *n, SymbolRef sym, ExprEngine& Eng); - PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, - const ExplodedNode *N); - SourceLocation getLocation() const { return AllocSite; } }; } // end anonymous namespace @@ -2384,17 +2395,19 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, } PathDiagnosticPiece* -CFRefReport::getEndPath(BugReporterContext &BRC, - const ExplodedNode *EndN) { +CFRefReportVisitor::getEndPath(BugReporterContext &BRC, + const ExplodedNode *EndN, + BugReport &BR) { // Tell the BugReporterContext to report cases when the tracked symbol is // assigned to different variables, etc. BRC.addNotableSymbol(Sym); - return BugReport::getEndPath(BRC, EndN); + return BugReporterVisitor::getDefaultEndPath(BRC, EndN, BR); } PathDiagnosticPiece* -CFRefLeakReport::getEndPath(BugReporterContext &BRC, - const ExplodedNode *EndN){ +CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, + const ExplodedNode *EndN, + BugReport &BR) { // Tell the BugReporterContext to report cases when the tracked symbol is // assigned to different variables, etc. @@ -2493,7 +2506,7 @@ CFRefLeakReport::getEndPath(BugReporterContext &BRC, CFRefLeakReport::CFRefLeakReport(CFRefBug& D, const CFRefCount &tf, ExplodedNode *n, SymbolRef sym, ExprEngine& Eng) -: CFRefReport(D, tf, n, sym) { +: CFRefReport(D, tf, n, sym, false) { // Most bug reports are cached at the location where they occurred. // With leaks, we want to unique them by the location where they were @@ -2527,7 +2540,7 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug& D, const CFRefCount &tf, if (AllocBinding) os << " and stored into '" << AllocBinding->getString() << '\''; - addVisitor(new CFRefReportVisitor(sym, tf)); + addVisitor(new CFRefLeakReportVisitor(sym, tf)); } //===----------------------------------------------------------------------===//