[cfe-commits] r153369 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h lib/StaticAnalyzer/Checkers/MallocChecker.cpp lib/StaticAnalyzer/Core/BugReporter.cpp
Jordy Rose
jediknil at belkadan.com
Fri Mar 23 20:03:30 PDT 2012
Author: jrose
Date: Fri Mar 23 22:03:29 2012
New Revision: 153369
URL: http://llvm.org/viewvc/llvm-project?rev=153369&view=rev
Log:
[analyzer] Restart path diagnostic generation if any of the visitors change the report configuration while walking the path.
This required adding a change count token to BugReport, but also allowed us to ditch ImmutableList as the BugReporterVisitor data type.
Also, remove the hack from MallocChecker, now that visitors appear in the opposite order. This is not exactly a fix, but the common case -- custom diagnostics after generic ones -- is now the default behavior.
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=153369&r1=153368&r2=153369&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri Mar 23 22:03:29 2012
@@ -61,7 +61,8 @@
};
typedef const SourceRange *ranges_iterator;
- typedef llvm::ImmutableList<BugReporterVisitor*>::iterator visitor_iterator;
+ typedef SmallVector<BugReporterVisitor *, 8> VisitorList;
+ typedef VisitorList::iterator visitor_iterator;
typedef SmallVector<StringRef, 2> ExtraTextList;
protected:
@@ -90,25 +91,30 @@
/// to include when constructing the final path diagnostic.
Regions interestingRegions;
- // 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<BugReporterVisitor*>::Factory F;
- llvm::ImmutableList<BugReporterVisitor*> Callbacks;
+ /// A set of custom visitors which generate "event" diagnostics at
+ /// interesting points in the path.
+ VisitorList Callbacks;
+
+ /// Used for ensuring the visitors are only added once.
llvm::FoldingSet<BugReporterVisitor> CallbacksSet;
+ /// Used for clients to tell if the report's configuration has changed
+ /// since the last time they checked.
+ unsigned ConfigurationChangeToken;
+
public:
BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode)
: BT(bt), Description(desc), ErrorNode(errornode),
- Callbacks(F.getEmptyList()) {}
+ ConfigurationChangeToken(0) {}
BugReport(BugType& bt, StringRef shortDesc, StringRef desc,
const ExplodedNode *errornode)
: BT(bt), ShortDescription(shortDesc), Description(desc),
- ErrorNode(errornode), Callbacks(F.getEmptyList()) {}
+ ErrorNode(errornode), ConfigurationChangeToken(0) {}
BugReport(BugType& bt, StringRef desc, PathDiagnosticLocation l)
: BT(bt), Description(desc), Location(l), ErrorNode(0),
- Callbacks(F.getEmptyList()) {}
+ ConfigurationChangeToken(0) {}
/// \brief Create a BugReport with a custom uniqueing location.
///
@@ -120,7 +126,7 @@
BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode,
PathDiagnosticLocation LocationToUnique)
: BT(bt), Description(desc), UniqueingLocation(LocationToUnique),
- ErrorNode(errornode), Callbacks(F.getEmptyList()) {}
+ ErrorNode(errornode), ConfigurationChangeToken(0) {}
virtual ~BugReport();
@@ -142,6 +148,10 @@
bool isInteresting(SymbolRef sym) const;
bool isInteresting(const MemRegion *R) const;
bool isInteresting(SVal V) const;
+
+ unsigned getConfigurationChangeToken() const {
+ return ConfigurationChangeToken;
+ }
/// \brief This allows for addition of meta data to the diagnostic.
///
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=153369&r1=153368&r2=153369&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Fri Mar 23 22:03:29 2012
@@ -875,10 +875,6 @@
BugReport *R = new BugReport(*BT_Leak, os.str(), N, LocUsedForUniqueing);
R->markInteresting(Sym);
- // FIXME: This is a hack to make sure the MallocBugVisitor gets to look at
- // the ExplodedNode chain first, in order to mark any failed realloc symbols
- // as interesting for ConditionBRVisitor.
- R->addVisitor(new ConditionBRVisitor());
R->addVisitor(new MallocBugVisitor(Sym));
C.EmitReport(R);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=153369&r1=153368&r2=153369&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Mar 23 22:03:29 2012
@@ -410,7 +410,8 @@
static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB,
- const ExplodedNode *N) {
+ const ExplodedNode *N,
+ ArrayRef<BugReporterVisitor *> visitors) {
SourceManager& SMgr = PDB.getSourceManager();
const LocationContext *LC = PDB.LC;
@@ -712,8 +713,9 @@
if (NextNode) {
// 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) {
+ for (ArrayRef<BugReporterVisitor *>::iterator I = visitors.begin(),
+ E = visitors.end();
+ I != E; ++I) {
if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R)) {
PD.getActivePath().push_front(p);
updateStackPiecesWithMessage(p, CallStack);
@@ -1051,7 +1053,8 @@
static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB,
- const ExplodedNode *N) {
+ const ExplodedNode *N,
+ ArrayRef<BugReporterVisitor *> visitors) {
EdgeBuilder EB(PD, PDB);
const SourceManager& SM = PDB.getSourceManager();
StackDiagVector CallStack;
@@ -1183,8 +1186,9 @@
// Add pieces from custom visitors.
BugReport *R = PDB.getBugReport();
- for (BugReport::visitor_iterator I = R->visitor_begin(),
- E = R->visitor_end(); I!=E; ++I) {
+ for (ArrayRef<BugReporterVisitor *>::iterator I = visitors.begin(),
+ E = visitors.end();
+ I != E; ++I) {
if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R)) {
const PathDiagnosticLocation &Loc = p->getLocation();
EB.addEdge(Loc, true);
@@ -1227,7 +1231,8 @@
}
CallbacksSet.InsertNode(visitor, InsertPos);
- Callbacks = F.add(visitor, Callbacks);
+ Callbacks.push_back(visitor);
+ ++ConfigurationChangeToken;
}
BugReport::~BugReport() {
@@ -1261,7 +1266,10 @@
void BugReport::markInteresting(SymbolRef sym) {
if (!sym)
return;
- interestingSymbols.insert(sym);
+
+ // If the symbol wasn't already in our set, note a configuration change.
+ if (interestingSymbols.insert(sym).second)
+ ++ConfigurationChangeToken;
if (const SymbolMetadata *meta = dyn_cast<SymbolMetadata>(sym))
interestingRegions.insert(meta->getRegion());
@@ -1270,8 +1278,11 @@
void BugReport::markInteresting(const MemRegion *R) {
if (!R)
return;
+
+ // If the base region wasn't already in our set, note a configuration change.
R = R->getBaseRegion();
- interestingRegions.insert(R);
+ if (interestingRegions.insert(R).second)
+ ++ConfigurationChangeToken;
if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R))
interestingSymbols.insert(SR->getSymbol());
@@ -1696,33 +1707,57 @@
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;
+ BugReport::VisitorList visitors;
+ unsigned originalReportConfigToken, finalReportConfigToken;
+
+ // While generating diagnostics, it's possible the visitors will decide
+ // new symbols and regions are interesting, or add other visitors based on
+ // the information they find. If they do, we need to regenerate the path
+ // based on our new report configuration.
+ do {
+ // Get a clean copy of all the visitors.
+ for (BugReport::visitor_iterator I = R->visitor_begin(),
+ E = R->visitor_end(); I != E; ++I)
+ visitors.push_back((*I)->clone());
+
+ // Clear out the active path from any previous work.
+ PD.getActivePath().clear();
+ originalReportConfigToken = R->getConfigurationChangeToken();
+
+ // Generate the very last diagnostic piece - the piece is visible before
+ // the trace is expanded.
+ PathDiagnosticPiece *LastPiece = 0;
+ for (BugReport::visitor_iterator I = visitors.begin(), E = visitors.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.getActivePath().push_back(LastPiece);
- else
- return;
+ if (!LastPiece)
+ LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R);
+ if (LastPiece)
+ PD.getActivePath().push_back(LastPiece);
+ else
+ return;
- switch (PDB.getGenerationScheme()) {
+ switch (PDB.getGenerationScheme()) {
case PathDiagnosticConsumer::Extensive:
- GenerateExtensivePathDiagnostic(PD, PDB, N);
+ GenerateExtensivePathDiagnostic(PD, PDB, N, visitors);
break;
case PathDiagnosticConsumer::Minimal:
- GenerateMinimalPathDiagnostic(PD, PDB, N);
+ GenerateMinimalPathDiagnostic(PD, PDB, N, visitors);
break;
- }
-
+ }
+
+ // Clean up the visitors we used.
+ llvm::DeleteContainerPointers(visitors);
+
+ // Did anything change while generating this path?
+ finalReportConfigToken = R->getConfigurationChangeToken();
+ } while(finalReportConfigToken != originalReportConfigToken);
+
// Finally, prune the diagnostic path of uninteresting stuff.
bool hasSomethingInteresting = RemoveUneededCalls(PD.getMutablePieces());
assert(hasSomethingInteresting);
More information about the cfe-commits
mailing list