[cfe-commits] r164446 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticAnalyzer/Core/ExplodedGraph.cpp
Jordan Rose
jordan_rose at apple.com
Fri Sep 21 18:24:53 PDT 2012
Author: jrose
Date: Fri Sep 21 20:24:53 2012
New Revision: 164446
URL: http://llvm.org/viewvc/llvm-project?rev=164446&view=rev
Log:
[analyzer] Allow a BugReport to be marked "invalid" during path generation.
This is intended to allow visitors to make decisions about whether a
BugReport is likely a false positive. Currently there are no visitors
making use of this feature, so there are no tests.
When a BugReport is marked invalid, the invalidator must provide a key
that identifies the invaliation (intended to be the visitor type and a
context pointer of some kind). This allows us to reverse the decision
later on. Being able to reverse a decision about invalidation gives us more
flexibility, and allows us to formulate conditions like "this report is
invalid UNLESS the original argument is 'foo'". We can use this to
fine-tune our false-positive suppression (coming soon).
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.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=164446&r1=164445&r2=164446&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri Sep 21 20:24:53 2012
@@ -116,6 +116,19 @@
/// when reporting an issue.
bool DoNotPrunePath;
+ /// Used to track unique reasons why a bug report might be invalid.
+ ///
+ /// \sa markInvalid
+ /// \sa removeInvalidation
+ typedef std::pair<const void *, const void *> InvalidationRecord;
+
+ /// If non-empty, this bug report is likely a false positive and should not be
+ /// shown to the user.
+ ///
+ /// \sa markInvalid
+ /// \sa removeInvalidation
+ llvm::SmallSet<InvalidationRecord, 4> Invalidations;
+
private:
// Used internally by BugReporter.
Symbols &getInterestingSymbols();
@@ -152,7 +165,8 @@
PathDiagnosticLocation LocationToUnique)
: BT(bt), DeclWithIssue(0), Description(desc),
UniqueingLocation(LocationToUnique),
- ErrorNode(errornode), ConfigurationChangeToken(0) {}
+ ErrorNode(errornode), ConfigurationChangeToken(0),
+ DoNotPrunePath(false) {}
virtual ~BugReport();
@@ -189,6 +203,34 @@
unsigned getConfigurationChangeToken() const {
return ConfigurationChangeToken;
}
+
+ /// Returns whether or not this report should be considered valid.
+ ///
+ /// Invalid reports are those that have been classified as likely false
+ /// positives after the fact.
+ bool isValid() const {
+ return Invalidations.empty();
+ }
+
+ /// Marks the current report as invalid, meaning that it is probably a false
+ /// positive and should not be reported to the user.
+ ///
+ /// The \p Tag and \p Data arguments are intended to be opaque identifiers for
+ /// this particular invalidation, where \p Tag represents the visitor
+ /// responsible for invalidation, and \p Data represents the reason this
+ /// visitor decided to invalidate the bug report.
+ ///
+ /// \sa removeInvalidation
+ void markInvalid(const void *Tag, const void *Data) {
+ Invalidations.insert(std::make_pair(Tag, Data));
+ }
+
+ /// Reverses the effects of a previous invalidation.
+ ///
+ /// \sa markInvalid
+ void removeInvalidation(const void *Tag, const void *Data) {
+ Invalidations.erase(std::make_pair(Tag, Data));
+ }
/// Return the canonical declaration, be it a method or class, where
/// this issue semantically occurred.
@@ -392,9 +434,11 @@
SourceManager& getSourceManager() { return D.getSourceManager(); }
- virtual void GeneratePathDiagnostic(PathDiagnostic& pathDiagnostic,
+ virtual bool generatePathDiagnostic(PathDiagnostic& pathDiagnostic,
PathDiagnosticConsumer &PC,
- ArrayRef<BugReport *> &bugReports) {}
+ ArrayRef<BugReport *> &bugReports) {
+ return true;
+ }
bool RemoveUneededCalls(PathPieces &pieces, BugReport *R,
PathDiagnosticCallPiece *CallWithLoc = 0);
@@ -461,7 +505,15 @@
/// engine.
ProgramStateManager &getStateManager();
- virtual void GeneratePathDiagnostic(PathDiagnostic &pathDiagnostic,
+ /// Generates a path corresponding to one of the given bug reports.
+ ///
+ /// Which report is used for path generation is not specified. The
+ /// bug reporter will try to pick the shortest path, but this is not
+ /// guaranteed.
+ ///
+ /// \return True if the report was valid and a path was generated,
+ /// false if the reports should be considered invalid.
+ virtual bool generatePathDiagnostic(PathDiagnostic &PD,
PathDiagnosticConsumer &PC,
ArrayRef<BugReport*> &bugReports);
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=164446&r1=164445&r2=164446&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Sep 21 20:24:53 2012
@@ -432,7 +432,7 @@
static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM);
-static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
+static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB,
const ExplodedNode *N,
ArrayRef<BugReporterVisitor *> visitors) {
@@ -756,9 +756,13 @@
}
}
+ if (!PDB.getBugReport()->isValid())
+ return false;
+
// After constructing the full PathDiagnostic, do a pass over it to compact
// PathDiagnosticPieces that occur within a macro.
CompactPathDiagnostic(PD.getMutablePieces(), PDB.getSourceManager());
+ return true;
}
//===----------------------------------------------------------------------===//
@@ -1164,7 +1168,7 @@
}
}
-static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
+static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
PathDiagnosticBuilder &PDB,
const ExplodedNode *N,
ArrayRef<BugReporterVisitor *> visitors) {
@@ -1337,6 +1341,8 @@
}
}
}
+
+ return PDB.getBugReport()->isValid();
}
//===----------------------------------------------------------------------===//
@@ -1866,17 +1872,27 @@
path.push_back(*I);
}
-void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
+bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
PathDiagnosticConsumer &PC,
ArrayRef<BugReport *> &bugReports) {
-
assert(!bugReports.empty());
+
+ bool HasValid = false;
SmallVector<const ExplodedNode *, 10> errorNodes;
for (ArrayRef<BugReport*>::iterator I = bugReports.begin(),
E = bugReports.end(); I != E; ++I) {
+ if ((*I)->isValid()) {
+ HasValid = true;
errorNodes.push_back((*I)->getErrorNode());
+ } else {
+ errorNodes.push_back(0);
+ }
}
+ // If all the reports have been marked invalid, we're done.
+ if (!HasValid)
+ return false;
+
// Construct a new graph that contains only a single path from the error
// node to a root.
const std::pair<std::pair<ExplodedGraph*, NodeBackMap*>,
@@ -1887,6 +1903,7 @@
assert(GPair.second.second < bugReports.size());
BugReport *R = bugReports[GPair.second.second];
assert(R && "No original report found for sliced graph.");
+ assert(R->isValid() && "Report selected from trimmed graph marked invalid.");
OwningPtr<ExplodedGraph> ReportGraph(GPair.first.first);
OwningPtr<NodeBackMap> BackMap(GPair.first.second);
@@ -1932,14 +1949,24 @@
if (LastPiece)
PD.setEndOfPath(LastPiece);
else
- return;
+ return false;
switch (PDB.getGenerationScheme()) {
case PathDiagnosticConsumer::Extensive:
- GenerateExtensivePathDiagnostic(PD, PDB, N, visitors);
+ if (!GenerateExtensivePathDiagnostic(PD, PDB, N, visitors)) {
+ assert(!R->isValid() && "Failed on valid report");
+ // Try again. We'll filter out the bad report when we trim the graph.
+ // FIXME: It would be more efficient to use the same intermediate
+ // trimmed graph, and just repeat the shortest-path search.
+ return generatePathDiagnostic(PD, PC, bugReports);
+ }
break;
case PathDiagnosticConsumer::Minimal:
- GenerateMinimalPathDiagnostic(PD, PDB, N, visitors);
+ if (!GenerateMinimalPathDiagnostic(PD, PDB, N, visitors)) {
+ assert(!R->isValid() && "Failed on valid report");
+ // Try again. We'll filter out the bad report when we trim the graph.
+ return generatePathDiagnostic(PD, PC, bugReports);
+ }
break;
case PathDiagnosticConsumer::None:
llvm_unreachable("PathDiagnosticConsumer::None should never appear here");
@@ -1958,6 +1985,8 @@
assert(hasSomethingInteresting);
(void) hasSomethingInteresting;
}
+
+ return true;
}
void BugReporter::Register(BugType *BT) {
@@ -2129,7 +2158,8 @@
// specified by the PathDiagnosticConsumer.
if (PD.getGenerationScheme() != PathDiagnosticConsumer::None) {
if (!bugReports.empty())
- GeneratePathDiagnostic(*D.get(), PD, bugReports);
+ if (!generatePathDiagnostic(*D.get(), PD, bugReports))
+ return;
}
// If the path is empty, generate a single step path with the location
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp?rev=164446&r1=164445&r2=164446&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp Fri Sep 21 20:24:53 2012
@@ -332,8 +332,8 @@
// ===- Pass 1 (reverse DFS) -===
for (const ExplodedNode* const* I = BeginSources; I != EndSources; ++I) {
- assert(*I);
- WL1.push_back(*I);
+ if (*I)
+ WL1.push_back(*I);
}
// Process the first worklist until it is empty. Because it is a std::list
More information about the cfe-commits
mailing list