r334540 - [analyzer] [NFC] Remove most usages of getEndPath
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 12 13:50:44 PDT 2018
Author: george.karpenkov
Date: Tue Jun 12 13:50:44 2018
New Revision: 334540
URL: http://llvm.org/viewvc/llvm-project?rev=334540&view=rev
Log:
[analyzer] [NFC] Remove most usages of getEndPath
getEndPath is a problematic API, because it's not clear when it's called
(hint: not always at the end of the path), it crashes at runtime with
more than one non-nullptr returning implementation, and diagnostics
internal depend on it being called at some exact place.
However, most visitors don't actually need that: all they want is a
function consistently called after all nodes are traversed, to perform
finalization and to decide whether invalidation is needed.
Differential Revision: https://reviews.llvm.org/D48042
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=334540&r1=334539&r2=334540&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Tue Jun 12 13:50:44 2018
@@ -73,12 +73,17 @@ public:
VisitNode(const ExplodedNode *Succ, const ExplodedNode *Pred,
BugReporterContext &BRC, BugReport &BR) = 0;
+ /// Last function called on the visitor, no further calls to VisitNode
+ /// would follow.
+ virtual void finalizeVisitor(BugReporterContext &BRC,
+ const ExplodedNode *EndPathNode,
+ BugReport &BR);
+
/// Provide custom definition for the final diagnostic piece on the
/// path - the piece, which is displayed before the path is expanded.
///
- /// If returns NULL the default implementation will be used.
- /// Also note that at most one visitor of a BugReport should generate a
- /// non-NULL end of path diagnostic piece.
+ /// NOTE that this function can be implemented on at most one used visitor,
+ /// and otherwise it crahes at runtime.
virtual std::unique_ptr<PathDiagnosticPiece>
getEndPath(BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR);
@@ -268,9 +273,8 @@ public:
return nullptr;
}
- std::unique_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
- const ExplodedNode *N,
- BugReport &BR) override;
+ void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *N,
+ BugReport &BR) override;
};
/// When a region containing undefined value or '0' value is passed
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=334540&r1=334539&r2=334540&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Tue Jun 12 13:50:44 2018
@@ -1270,7 +1270,9 @@ static bool generatePathDiagnostics(
PathDiagnostic &PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N,
LocationContextMap &LCM,
ArrayRef<std::unique_ptr<BugReporterVisitor>> visitors,
+ BugReport *R,
PathDiagnosticConsumer::PathGenerationScheme ActiveScheme) {
+ const ExplodedNode *LastNode = N;
BugReport *report = PDB.getBugReport();
StackDiagVector CallStack;
InterestingExprs IE;
@@ -1289,8 +1291,12 @@ static bool generatePathDiagnostics(
generatePathDiagnosticsForNode(
N, PD, PrevLoc, PDB, LCM, CallStack, IE, AddPathEdges);
- if (!NextNode)
+ if (!NextNode) {
+ for (auto &V : visitors) {
+ V->finalizeVisitor(PDB, LastNode, *R);
+ }
continue;
+ }
// Add pieces from custom visitors.
llvm::FoldingSet<PathDiagnosticPiece> DeduplicationSet;
@@ -2583,7 +2589,7 @@ bool GRBugReporter::generatePathDiagnost
// hold onto old mappings.
LCM.clear();
- generatePathDiagnostics(PD, PDB, N, LCM, visitors, ActiveScheme);
+ generatePathDiagnostics(PD, PDB, N, LCM, visitors, R, ActiveScheme);
// Clean up the visitors we used.
visitors.clear();
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=334540&r1=334539&r2=334540&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Tue Jun 12 13:50:44 2018
@@ -181,6 +181,11 @@ BugReporterVisitor::getEndPath(BugReport
return nullptr;
}
+void
+BugReporterVisitor::finalizeVisitor(BugReporterContext &BRC,
+ const ExplodedNode *EndPathNode,
+ BugReport &BR) {};
+
std::unique_ptr<PathDiagnosticPiece> BugReporterVisitor::getDefaultEndPath(
BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) {
PathDiagnosticLocation L =
@@ -866,12 +871,10 @@ public:
llvm_unreachable("Invalid visit mode!");
}
- std::unique_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
- const ExplodedNode *N,
- BugReport &BR) override {
+ void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *N,
+ BugReport &BR) override {
if (EnableNullFPSuppression)
BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
- return nullptr;
}
};
@@ -2144,10 +2147,8 @@ bool ConditionBRVisitor::isPieceMessageG
Piece->getString() == GenericFalseMessage;
}
-std::unique_ptr<PathDiagnosticPiece>
-LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
- const ExplodedNode *N,
- BugReport &BR) {
+void LikelyFalsePositiveSuppressionBRVisitor::finalizeVisitor(
+ BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR) {
// Here we suppress false positives coming from system headers. This list is
// based on known issues.
ExprEngine &Eng = BRC.getBugReporter().getEngine();
@@ -2161,7 +2162,7 @@ LikelyFalsePositiveSuppressionBRVisitor:
// TR1, Boost, or llvm/ADT.
if (Options.shouldSuppressFromCXXStandardLibrary()) {
BR.markInvalid(getTag(), nullptr);
- return nullptr;
+ return;
} else {
// If the complete 'std' suppression is not enabled, suppress reports
// from the 'std' namespace that are known to produce false positives.
@@ -2173,7 +2174,7 @@ LikelyFalsePositiveSuppressionBRVisitor:
const CXXRecordDecl *CD = MD->getParent();
if (CD->getName() == "list") {
BR.markInvalid(getTag(), nullptr);
- return nullptr;
+ return;
}
}
@@ -2183,7 +2184,7 @@ LikelyFalsePositiveSuppressionBRVisitor:
const CXXRecordDecl *CD = MD->getParent();
if (CD->getName() == "__independent_bits_engine") {
BR.markInvalid(getTag(), nullptr);
- return nullptr;
+ return;
}
}
@@ -2202,7 +2203,7 @@ LikelyFalsePositiveSuppressionBRVisitor:
// data structure.
if (CD->getName() == "basic_string") {
BR.markInvalid(getTag(), nullptr);
- return nullptr;
+ return;
}
// The analyzer issues a false positive on
@@ -2210,7 +2211,7 @@ LikelyFalsePositiveSuppressionBRVisitor:
// because it does not reason properly about temporary destructors.
if (CD->getName() == "shared_ptr") {
BR.markInvalid(getTag(), nullptr);
- return nullptr;
+ return;
}
}
}
@@ -2224,11 +2225,9 @@ LikelyFalsePositiveSuppressionBRVisitor:
Loc = Loc.getSpellingLoc();
if (SM.getFilename(Loc).endswith("sys/queue.h")) {
BR.markInvalid(getTag(), nullptr);
- return nullptr;
+ return;
}
}
-
- return nullptr;
}
std::shared_ptr<PathDiagnosticPiece>
More information about the cfe-commits
mailing list