From: Anna Zaks Date: Thu, 18 Aug 2011 22:37:56 +0000 (+0000) Subject: Static Analyzer Diagnostics: Move custom diagnostic visitors from BugReporterContext... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8e6431adab313e283a992698f6fc7afe62420999;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@138001 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 356d7cab23..2b9d1a89f5 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -37,6 +37,7 @@ class PathDiagnosticPiece; class PathDiagnosticClient; class ExplodedNode; class ExplodedGraph; +class BugReport; class BugReporter; class BugReporterContext; class ExprEngine; @@ -50,9 +51,16 @@ class BugType; 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 @@ public: 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::iterator visitor_iterator; protected: friend class BugReporter; @@ -84,7 +92,12 @@ protected: FullSourceLoc Location; const ExplodedNode *ErrorNode; SmallVector 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::Factory F; + llvm::ImmutableList Callbacks; + llvm::FoldingSet 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 @@ protected: 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 @@ public: /// 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 @@ public: 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::Factory F; - llvm::ImmutableList Callbacks; - llvm::FoldingSet CallbacksSet; public: - BugReporterContext(GRBugReporter& br) : BR(br), Callbacks(F.getEmptyList()) {} - virtual ~BugReporterContext(); - - void addVisitor(BugReporterVisitor* visitor); + BugReporterContext(GRBugReporter& br) : BR(br) {} - typedef llvm::ImmutableList::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 *GetDenomExpr(const ExplodedNode *N); 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 diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 95303a034d..6624b93d2b 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -33,27 +33,6 @@ using namespace clang; 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 @@ public: 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 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, } 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 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, 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 @@ void BugType::FlushReports(BugReporter &BR) {} // 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 @@ SourceLocation BugReport::getLocation() const { PathDiagnosticPiece *BugReport::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, - BugReporterContext &BRC) { + BugReporterContext &BRC, + BugReport &BR) { return NULL; } @@ -1655,10 +1662,9 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, 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: diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 5cd9e4d15a..b6e726fd0b 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -91,7 +91,8 @@ public: PathDiagnosticPiece *VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, - BugReporterContext &BRC) { + BugReporterContext &BRC, + BugReport &BR) { if (satisfied) return NULL; @@ -221,9 +222,9 @@ public: }; -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 @@ public: PathDiagnosticPiece *VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, - BugReporterContext &BRC) { + BugReporterContext &BRC, + BugReport &BR) { if (isSatisfied) return NULL; @@ -297,23 +299,23 @@ public: }; } // 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(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 @@ void bugreporter::registerTrackNullOrUndefValue(BugReporterContext &BRC, if (isa(V) || isa(V) || V.isUndef()) { - ::registerFindLastStore(BRC, R, V); + ::registerFindLastStore(BR, R, V); } } } @@ -361,16 +363,16 @@ void bugreporter::registerTrackNullOrUndefValue(BugReporterContext &BRC, if (R) { assert(isa(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(data); + const ExplodedNode *N = BR.getErrorNode(); if (!R) return; @@ -381,7 +383,7 @@ void bugreporter::registerFindLastStore(BugReporterContext &BRC, if (V.isUnknown()) return; - BRC.addVisitor(new FindLastStoreBRVisitor(V, R)); + BR.addVisitor(new FindLastStoreBRVisitor(V, R)); } @@ -397,7 +399,8 @@ public: PathDiagnosticPiece *VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, - BugReporterContext &BRC) { + BugReporterContext &BRC, + BugReport &BR) { const PostStmt *P = N->getLocationAs(); if (!P) @@ -420,7 +423,7 @@ public: // 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 @@ public: }; } // 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(stmt); + const ExplodedNode *N = BR.getErrorNode(); std::deque WorkList; @@ -447,8 +450,8 @@ void bugreporter::registerVarDeclsLastStore(BugReporterContext &BRC, 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(Head)) { if (const VarDecl *VD = dyn_cast(DR->getDecl())) { @@ -459,7 +462,7 @@ void bugreporter::registerVarDeclsLastStore(BugReporterContext &BRC, SVal V = state->getSVal(S); if (isa(V) || isa(V)) { - ::registerFindLastStore(BRC, R, V); + ::registerFindLastStore(BR, R, V); } } } @@ -484,7 +487,8 @@ public: 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 @@ public: PathDiagnosticPiece *ConditionVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *Prev, - BugReporterContext &BRC) { + BugReporterContext &BRC, + BugReport &BR) { const ProgramPoint &progPoint = N->getLocation(); @@ -752,7 +757,7 @@ ConditionVisitor::VisitTrueTest(const Expr *Cond, return new PathDiagnosticEventPiece(Loc, Out.str()); } -void bugreporter::registerConditionVisitor(BugReporterContext &BRC) { - BRC.addVisitor(new ConditionVisitor()); +void bugreporter::registerConditionVisitor(BugReport &BR) { + BR.addVisitor(new ConditionVisitor()); } diff --git a/lib/StaticAnalyzer/Core/CFRefCount.cpp b/lib/StaticAnalyzer/Core/CFRefCount.cpp index 3c395fe282..78325f366c 100644 --- a/lib/StaticAnalyzer/Core/CFRefCount.cpp +++ b/lib/StaticAnalyzer/Core/CFRefCount.cpp @@ -1994,7 +1994,8 @@ namespace { PathDiagnosticPiece *VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, - BugReporterContext &BRC); + BugReporterContext &BRC, + BugReport &BR); }; class CFRefLeakReport : public CFRefReport { @@ -2061,7 +2062,8 @@ static inline bool contains(const SmallVectorImpl& V, PathDiagnosticPiece *CFRefReport::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, - BugReporterContext &BRC) { + BugReporterContext &BRC, + BugReport &BR) { if (!isa(N->getLocation())) return NULL;