maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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.

llvm-svn: 180974
This commit is contained in:
Ted Kremenek 2013-05-02 23:56:33 +00:00
parent d2472d4cdb
commit 48bd5fddf3
2 changed files with 27 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
View File

@ -406,11 +406,6 @@ private:
/// A vector of BugReports for tracking the allocated pointers and cleanup. /// A vector of BugReports for tracking the allocated pointers and cleanup.
std::vector<BugReportEquivClass *> EQClassesVector; std::vector<BugReportEquivClass *> EQClassesVector;
/// A map from PathDiagnosticPiece to the LocationContext of the inlined
/// function call it represents.
llvm::DenseMap<const PathDiagnosticCallPiece*,
const LocationContext*> LocationContextMap;
protected: protected:
BugReporter(BugReporterData& d, Kind k) : BugTypes(F.getEmptySet()), kind(k), BugReporter(BugReporterData& d, Kind k) : BugTypes(F.getEmptySet()), kind(k),
D(d) {} D(d) {}
@ -482,10 +477,6 @@ public:
EmitBasicReport(DeclWithIssue, BugName, Category, BugStr, Loc, &R, 1); EmitBasicReport(DeclWithIssue, BugName, Category, BugStr, Loc, &R, 1);
} }
void addCallPieceLocationContextPair(const PathDiagnosticCallPiece *C,
const LocationContext *LC) {
LocationContextMap[C] = LC;
}
private: private:
llvm::StringMap<BugType *> StrBugTypes; llvm::StringMap<BugType *> StrBugTypes;

43
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
View File

@ -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<const PathDiagnosticCallPiece*, const LocationContext*>
LocationContextMap;
/// Recursively scan through a path and prune out calls and macros pieces /// Recursively scan through a path and prune out calls and macros pieces
/// that aren't needed. Return true if afterwards the path contains /// that aren't needed. Return true if afterwards the path contains
/// "interesting stuff" which means it shouldn't be pruned from the parent path. /// "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; bool containsSomethingInteresting = false;
const unsigned N = pieces.size(); const unsigned N = pieces.size();
@ -167,13 +173,13 @@ bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) {
case PathDiagnosticPiece::Call: { case PathDiagnosticPiece::Call: {
PathDiagnosticCallPiece *call = cast<PathDiagnosticCallPiece>(piece); PathDiagnosticCallPiece *call = cast<PathDiagnosticCallPiece>(piece);
// Check if the location context is interesting. // Check if the location context is interesting.
assert(LocationContextMap.count(call)); assert(LCM.count(call));
if (R->isInteresting(LocationContextMap[call])) { if (R->isInteresting(LCM[call])) {
containsSomethingInteresting = true; containsSomethingInteresting = true;
break; break;
} }
if (!RemoveUnneededCalls(call->path, R)) if (!removeUnneededCalls(call->path, R, LCM))
continue; continue;
containsSomethingInteresting = true; containsSomethingInteresting = true;
@ -181,7 +187,7 @@ bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) {
} }
case PathDiagnosticPiece::Macro: { case PathDiagnosticPiece::Macro: {
PathDiagnosticMacroPiece *macro = cast<PathDiagnosticMacroPiece>(piece); PathDiagnosticMacroPiece *macro = cast<PathDiagnosticMacroPiece>(piece);
if (!RemoveUnneededCalls(macro->subPieces, R)) if (!removeUnneededCalls(macro->subPieces, R, LCM))
continue; continue;
containsSomethingInteresting = true; containsSomethingInteresting = true;
break; break;
@ -511,6 +517,7 @@ static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM);
static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB, PathDiagnosticBuilder &PDB,
const ExplodedNode *N, const ExplodedNode *N,
LocationContextMap &LCM,
ArrayRef<BugReporterVisitor *> visitors) { ArrayRef<BugReporterVisitor *> visitors) {
SourceManager& SMgr = PDB.getSourceManager(); SourceManager& SMgr = PDB.getSourceManager();
@ -531,8 +538,8 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
if (Optional<CallExitEnd> CE = P.getAs<CallExitEnd>()) { if (Optional<CallExitEnd> CE = P.getAs<CallExitEnd>()) {
PathDiagnosticCallPiece *C = PathDiagnosticCallPiece *C =
PathDiagnosticCallPiece::construct(N, *CE, SMgr); PathDiagnosticCallPiece::construct(N, *CE, SMgr);
GRBugReporter& BR = PDB.getBugReporter(); // Record the mapping from call piece to LocationContext.
BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); LCM[C] = CE->getCalleeContext();
PD.getActivePath().push_front(C); PD.getActivePath().push_front(C);
PD.pushActivePath(&C->path); PD.pushActivePath(&C->path);
CallStack.push_back(StackDiagPair(C, N)); CallStack.push_back(StackDiagPair(C, N));
@ -555,8 +562,8 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
} else { } else {
const Decl *Caller = CE->getLocationContext()->getDecl(); const Decl *Caller = CE->getLocationContext()->getDecl();
C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller);
GRBugReporter& BR = PDB.getBugReporter(); // Record the mapping from call piece to LocationContext.
BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); LCM[C] = CE->getCalleeContext();
} }
C->setCallee(*CE, SMgr); C->setCallee(*CE, SMgr);
@ -1324,6 +1331,7 @@ static bool isInLoopBody(ParentMap &PM, const Stmt *S, const Stmt *Term) {
static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB, PathDiagnosticBuilder &PDB,
const ExplodedNode *N, const ExplodedNode *N,
LocationContextMap &LCM,
ArrayRef<BugReporterVisitor *> visitors) { ArrayRef<BugReporterVisitor *> visitors) {
EdgeBuilder EB(PD, PDB); EdgeBuilder EB(PD, PDB);
const SourceManager& SM = PDB.getSourceManager(); const SourceManager& SM = PDB.getSourceManager();
@ -1354,8 +1362,7 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
PathDiagnosticCallPiece *C = PathDiagnosticCallPiece *C =
PathDiagnosticCallPiece::construct(N, *CE, SM); PathDiagnosticCallPiece::construct(N, *CE, SM);
GRBugReporter& BR = PDB.getBugReporter(); LCM[C] = CE->getCalleeContext();
BR.addCallPieceLocationContextPair(C, CE->getCalleeContext());
EB.addEdge(C->callReturn, /*AlwaysAdd=*/true, /*IsPostJump=*/true); EB.addEdge(C->callReturn, /*AlwaysAdd=*/true, /*IsPostJump=*/true);
EB.flushLocations(); EB.flushLocations();
@ -1392,8 +1399,7 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
} else { } else {
const Decl *Caller = CE->getLocationContext()->getDecl(); const Decl *Caller = CE->getLocationContext()->getDecl();
C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller);
GRBugReporter& BR = PDB.getBugReporter(); LCM[C] = CE->getCalleeContext();
BR.addCallPieceLocationContextPair(C, CE->getCalleeContext());
} }
C->setCallee(*CE, SM); C->setCallee(*CE, SM);
@ -2120,6 +2126,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
BugReport::VisitorList visitors; BugReport::VisitorList visitors;
unsigned origReportConfigToken, finalReportConfigToken; unsigned origReportConfigToken, finalReportConfigToken;
LocationContextMap LCM;
// While generating diagnostics, it's possible the visitors will decide // While generating diagnostics, it's possible the visitors will decide
// new symbols and regions are interesting, or add other visitors based on // new symbols and regions are interesting, or add other visitors based on
@ -2156,10 +2163,10 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
switch (ActiveScheme) { switch (ActiveScheme) {
case PathDiagnosticConsumer::Extensive: case PathDiagnosticConsumer::Extensive:
GenerateExtensivePathDiagnostic(PD, PDB, N, visitors); GenerateExtensivePathDiagnostic(PD, PDB, N, LCM, visitors);
break; break;
case PathDiagnosticConsumer::Minimal: case PathDiagnosticConsumer::Minimal:
GenerateMinimalPathDiagnostic(PD, PDB, N, visitors); GenerateMinimalPathDiagnostic(PD, PDB, N, LCM, visitors);
break; break;
case PathDiagnosticConsumer::None: case PathDiagnosticConsumer::None:
GenerateVisitorsOnlyPathDiagnostic(PD, PDB, N, visitors); GenerateVisitorsOnlyPathDiagnostic(PD, PDB, N, visitors);
@ -2183,7 +2190,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
if (R->shouldPrunePath() && if (R->shouldPrunePath() &&
getEngine().getAnalysisManager().options.shouldPrunePaths()) { getEngine().getAnalysisManager().options.shouldPrunePaths()) {
bool stillHasNotes = RemoveUnneededCalls(PD.getMutablePieces(), R); bool stillHasNotes = removeUnneededCalls(PD.getMutablePieces(), R, LCM);
assert(stillHasNotes); assert(stillHasNotes);
(void)stillHasNotes; (void)stillHasNotes;
} }
@ -2288,6 +2295,10 @@ FindReportInEquivalenceClass(BugReportEquivClass& EQ,
continue; 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 // 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. // successor. Use a DFS worklist to find a non-sink end-of-path node.
typedef FRIEC_WLItem WLItem; typedef FRIEC_WLItem WLItem;