[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@334540 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 80a6763783bb51fad47e8841c3573da99da3cfd0..9662d91fea249d9b6ee9b184f91eea9890103886 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -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 

diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 98c90559e250dedbe4311dbdc7ab066daa0ead7c..22185d2d03851728914b105d03bc34331a4d6296 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -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::generatePathDiagnostic(PathDiagnostic& PD,
       // 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();

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index fe1a711a82c2a428a4d2b2d719359ba6a64be5db..75126c6c3785b88311d9cc5baa7933b3a15add93 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -181,6 +181,11 @@ BugReporterVisitor::getEndPath(BugReporterContext &BRC,
   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::isPieceMessageGeneric(
          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::getEndPath(BugReporterContext &BRC,
     // 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::getEndPath(BugReporterContext &BRC,
         const CXXRecordDecl *CD = MD->getParent();
         if (CD->getName() == "list") {
           BR.markInvalid(getTag(), nullptr);
-          return nullptr;
+          return;
         }
       }
 
@@ -2183,7 +2184,7 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
         const CXXRecordDecl *CD = MD->getParent();
         if (CD->getName() == "__independent_bits_engine") {
           BR.markInvalid(getTag(), nullptr);
-          return nullptr;
+          return;
         }
       }
 
@@ -2202,7 +2203,7 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
         // 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::getEndPath(BugReporterContext &BRC,
         // 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::getEndPath(BugReporterContext &BRC,
     Loc = Loc.getSpellingLoc();
     if (SM.getFilename(Loc).endswith("sys/queue.h")) {
       BR.markInvalid(getTag(), nullptr);
-      return nullptr;
+      return;
     }
   }
-
-  return nullptr;
 }
 
 std::shared_ptr<PathDiagnosticPiece>