From: Ted Kremenek Date: Thu, 2 May 2013 23:56:33 +0000 (+0000) Subject: Change LocationContextMap to be a temporary instead of shared variable in BugReporter. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c70fac3c52092013b08163187f034b73c94bf3d0;p=clang Change LocationContextMap to be a temporary instead of shared variable in BugReporter. BugReporter is used to process ALL bug reports. By using a shared map, we are having mappings from different PathDiagnosticPieces to LocationContexts well beyond the point where we are processing a given report. This state is inherently error prone, and is analogous to using a global variable. Instead, just create a temporary map, one per report, and when we are done with it we throw it away. No extra state. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180974 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 7a87e47f74..5c560b2f0e 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -406,11 +406,6 @@ private: /// A vector of BugReports for tracking the allocated pointers and cleanup. std::vector EQClassesVector; - /// A map from PathDiagnosticPiece to the LocationContext of the inlined - /// function call it represents. - llvm::DenseMap LocationContextMap; - protected: BugReporter(BugReporterData& d, Kind k) : BugTypes(F.getEmptySet()), kind(k), D(d) {} @@ -482,10 +477,6 @@ public: EmitBasicReport(DeclWithIssue, BugName, Category, BugStr, Loc, &R, 1); } - void addCallPieceLocationContextPair(const PathDiagnosticCallPiece *C, - const LocationContext *LC) { - LocationContextMap[C] = LC; - } private: llvm::StringMap StrBugTypes; diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 52c364e891..9e5a764e17 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -143,10 +143,16 @@ static void removeRedundantMsgs(PathPieces &path) { } } +/// A map from PathDiagnosticPiece to the LocationContext of the inlined +/// function call it represents. +typedef llvm::DenseMap + LocationContextMap; + /// Recursively scan through a path and prune out calls and macros pieces /// that aren't needed. Return true if afterwards the path contains /// "interesting stuff" which means it shouldn't be pruned from the parent path. -bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { +static bool removeUnneededCalls(PathPieces &pieces, BugReport *R, + LocationContextMap &LCM) { bool containsSomethingInteresting = false; const unsigned N = pieces.size(); @@ -167,13 +173,13 @@ bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { case PathDiagnosticPiece::Call: { PathDiagnosticCallPiece *call = cast(piece); // Check if the location context is interesting. - assert(LocationContextMap.count(call)); - if (R->isInteresting(LocationContextMap[call])) { + assert(LCM.count(call)); + if (R->isInteresting(LCM[call])) { containsSomethingInteresting = true; break; } - if (!RemoveUnneededCalls(call->path, R)) + if (!removeUnneededCalls(call->path, R, LCM)) continue; containsSomethingInteresting = true; @@ -181,7 +187,7 @@ bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { } case PathDiagnosticPiece::Macro: { PathDiagnosticMacroPiece *macro = cast(piece); - if (!RemoveUnneededCalls(macro->subPieces, R)) + if (!removeUnneededCalls(macro->subPieces, R, LCM)) continue; containsSomethingInteresting = true; break; @@ -511,6 +517,7 @@ static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM); static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, + LocationContextMap &LCM, ArrayRef visitors) { SourceManager& SMgr = PDB.getSourceManager(); @@ -531,8 +538,8 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (Optional CE = P.getAs()) { PathDiagnosticCallPiece *C = PathDiagnosticCallPiece::construct(N, *CE, SMgr); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + // Record the mapping from call piece to LocationContext. + LCM[C] = CE->getCalleeContext(); PD.getActivePath().push_front(C); PD.pushActivePath(&C->path); CallStack.push_back(StackDiagPair(C, N)); @@ -555,8 +562,8 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, } else { const Decl *Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + // Record the mapping from call piece to LocationContext. + LCM[C] = CE->getCalleeContext(); } C->setCallee(*CE, SMgr); @@ -1324,6 +1331,7 @@ static bool isInLoopBody(ParentMap &PM, const Stmt *S, const Stmt *Term) { static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, + LocationContextMap &LCM, ArrayRef visitors) { EdgeBuilder EB(PD, PDB); const SourceManager& SM = PDB.getSourceManager(); @@ -1354,8 +1362,7 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticCallPiece *C = PathDiagnosticCallPiece::construct(N, *CE, SM); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + LCM[C] = CE->getCalleeContext(); EB.addEdge(C->callReturn, /*AlwaysAdd=*/true, /*IsPostJump=*/true); EB.flushLocations(); @@ -1392,8 +1399,7 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, } else { const Decl *Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + LCM[C] = CE->getCalleeContext(); } C->setCallee(*CE, SM); @@ -2120,6 +2126,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, BugReport::VisitorList visitors; unsigned origReportConfigToken, finalReportConfigToken; + LocationContextMap LCM; // While generating diagnostics, it's possible the visitors will decide // new symbols and regions are interesting, or add other visitors based on @@ -2156,10 +2163,10 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, switch (ActiveScheme) { case PathDiagnosticConsumer::Extensive: - GenerateExtensivePathDiagnostic(PD, PDB, N, visitors); + GenerateExtensivePathDiagnostic(PD, PDB, N, LCM, visitors); break; case PathDiagnosticConsumer::Minimal: - GenerateMinimalPathDiagnostic(PD, PDB, N, visitors); + GenerateMinimalPathDiagnostic(PD, PDB, N, LCM, visitors); break; case PathDiagnosticConsumer::None: GenerateVisitorsOnlyPathDiagnostic(PD, PDB, N, visitors); @@ -2183,7 +2190,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, if (R->shouldPrunePath() && getEngine().getAnalysisManager().options.shouldPrunePaths()) { - bool stillHasNotes = RemoveUnneededCalls(PD.getMutablePieces(), R); + bool stillHasNotes = removeUnneededCalls(PD.getMutablePieces(), R, LCM); assert(stillHasNotes); (void)stillHasNotes; } @@ -2288,6 +2295,10 @@ FindReportInEquivalenceClass(BugReportEquivClass& EQ, continue; } + // Make sure we get a clean location context map so we don't + // hold onto old mappings. + LCM.clear(); + // At this point we know that 'N' is not a sink and it has at least one // successor. Use a DFS worklist to find a non-sink end-of-path node. typedef FRIEC_WLItem WLItem;