[analyzer] Make checker silencing work for non-pathsensitive bug reports

[Szelethus]

Summary

D66572 separated BugReport and BugReporter into basic and path sensitive versions. As a result, checker silencing, which worked deep in the path sensitive report generation facilities became specific to it. DeadStoresChecker, for instance, despite being in the static analyzer, emits non-pathsensitive reports, and was impossible to silence.

This patch moves the corresponding code before the call to the virtual function generateDiagnosticForConsumerMap (which is overriden by the specific kinds of bug reporters). Although we see bug reporting as relatively lightweight compared to the analysis, this will get rid of several steps we used to throw away.

Quoting from D65379:

At a very high level, this consists of 3 steps:

1. For all BugReports in the same BugReportEquivClass, collect all their error nodes in a set. With that set, create a new, trimmed ExplodedGraph whose leafs are all error nodes.
2. Until a valid report is found, construct a bug path, which is yet another ExplodedGraph, that is linear from a given error node to the root of the graph.
3. Run all visitors on the constructed bug path. If in this process the report got invalidated, start over from step 2.

Checker silencing used to kick in after all of these. Now it does before any of them :^)

[Szelethus]

Fix the test file.

[martong]

This makes sense, and looks good to me! Though, my review confidence is weak.

[NoQ]

Checker silencing used to kick in after all of these. Now it does before any of them :^)

This is probably the main benefit of the patch. Running visitors is cheap most of the time but it can be expensive.

Silencing path-insensitive checkers isn't particularly useful because they don't interact with other checkers so it's entirely equivalent to disabling them. Unless, well, somebody someday introduces an interaction. Still, it's great to have the interface work consistently.

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

 
 std::unique_ptr<PathDiagnostic>
 PathDiagnosticBuilder::generate(const PathDiagnosticConsumer *PDC) const {
   PathDiagnosticConstruct Construct(PDC, ErrorNode, R);
 
   const SourceManager &SM = getSourceManager();
   const AnalyzerOptions &Opts = getAnalyzerOptions();
 
-  // See whether we need to silence the checker/package.
-  // FIXME: This will not work if the report was emitted with an incorrect tag.
-  for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) {
-    if (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
-      return nullptr;
-  }
-
   if (!PDC->shouldGenerateDiagnostics())
     return generateEmptyDiagnosticForReport(R, getSourceManager());
 
   // Construct the final (warning) event for the bug report.
   auto EndNotes = VisitorsDiagnostics->find(ErrorNode);
   PathDiagnosticPieceRef LastPiece;
   if (EndNotes != VisitorsDiagnostics->end()) {
     assert(!EndNotes->second.empty());

 }
 
 void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   SmallVector<BugReport*, 10> bugReports;
   BugReport *report = findReportInEquivalenceClass(EQ, bugReports);
   if (!report)
     return;
 
+  // See whether we need to silence the checker/package.
+  for (const std::string &CheckerOrPackage :
+       getAnalyzerOptions().SilencedCheckersAndPackages) {
+    if (report->getBugType().getCheckerName().startswith(
+            CheckerOrPackage))
+      return;
+  }
+
   ArrayRef<PathDiagnosticConsumer*> Consumers = getPathDiagnosticConsumers();
   std::unique_ptr<DiagnosticForConsumerMapTy> Diagnostics =
       generateDiagnosticForConsumerMap(report, Consumers, bugReports);
 
   for (auto &P : *Diagnostics) {
     PathDiagnosticConsumer *Consumer = P.first;
     std::unique_ptr<PathDiagnostic> &PD = P.second;
 

clang/test/Analysis/silence-checkers.cpp

 // RUN: %clang_analyze_cc1 -verify="no-silence" %s \
 // RUN:   -triple i386-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,apiModeling \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=cplusplus.NewDelete
 
 // RUN: %clang_analyze_cc1 -verify="unix-silenced" %s \
 // RUN:   -triple i386-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,apiModeling \
 // RUN:   -analyzer-checker=unix.Malloc \
 // RUN:   -analyzer-checker=cplusplus.NewDelete\
 // RUN:   -analyzer-config silence-checkers="unix"
 
+// RUN: %clang_analyze_cc1 -verify="deadstore-silenced" %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling \
+// RUN:   -analyzer-checker=deadcode \
+// RUN:   -analyzer-config silence-checkers="deadcode.DeadStores"
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);
 void *realloc(void *ptr, size_t size);
 void *calloc(size_t nmemb, size_t size);
 char *strdup(const char *s);

 #define ZERO_SIZE_PTR ((void *)16)
 
 void test_delete_ZERO_SIZE_PTR() {
   int *Ptr = (int *)ZERO_SIZE_PTR;
   // ZERO_SIZE_PTR is specially handled but only for malloc family
   delete Ptr; // no-silence-warning{{Argument to 'delete' is a constant address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
               // unix-silenced-warning@-1{{Argument to 'delete' is a constant address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
 }
+
+// deadstore-silenced-no-diagnostics
+
+int foo() {
+  int x = 42;
+  return x;
+}
+
+void g() {
+  int y;
+  y = 7;
+  int x = foo();
+  y = 10;
+}