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> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
