[analyzer] Re-apply r283092, attempt no.4, chunk no.4 (last)

The problem that caused the msvc crash has been indentified and fixed
in the previous commit. This patch contains the rest of r283092.


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

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

@@ -266,6 +266,9 @@ private:
   /// \sa shouldWidenLoops
   Optional<bool> WidenLoops;
 
+  /// \sa shouldDisplayNotesAsEvents
+  Optional<bool> DisplayNotesAsEvents;
+
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -534,6 +537,14 @@ public:
   /// This is controlled by the 'widen-loops' config option.
   bool shouldWidenLoops();
 
+  /// Returns true if the bug reporter should transparently treat extra note
+  /// diagnostic pieces as event diagnostic pieces. Useful when the diagnostic
+  /// consumer doesn't support the extra note pieces.
+  ///
+  /// This is controlled by the 'extra-notes-as-events' option, which defaults
+  /// to false when unset.
+  bool shouldDisplayNotesAsEvents();
+
 public:
   AnalyzerOptions() :
     AnalysisStoreOpt(RegionStoreModel),

include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -180,6 +180,18 @@ public:
   const BugType& getBugType() const { return BT; }
   BugType& getBugType() { return BT; }
 
+  /// \brief True when the report has an execution path associated with it.
+  ///
+  /// A report is said to be path-sensitive if it was thrown against a
+  /// particular exploded node in the path-sensitive analysis graph.
+  /// Path-sensitive reports have their intermediate path diagnostics
+  /// auto-generated, perhaps with the help of checker-defined visitors,
+  /// and may contain extra notes.
+  /// Path-insensitive reports consist only of a single warning message
+  /// in a specific location, and perhaps extra notes.
+  /// Path-sensitive checkers are allowed to throw path-insensitive reports.
+  bool isPathSensitive() const { return ErrorNode != nullptr; }
+
   const ExplodedNode *getErrorNode() const { return ErrorNode; }
 
   StringRef getDescription() const { return Description; }
@@ -256,8 +268,7 @@ public:
   /// the extra note should appear.
   void addNote(StringRef Msg, const PathDiagnosticLocation &Pos,
                ArrayRef<SourceRange> Ranges) {
-    PathDiagnosticNotePiece *P =
-        new PathDiagnosticNotePiece(Pos, Msg);
+    PathDiagnosticNotePiece *P = new PathDiagnosticNotePiece(Pos, Msg);
 
     for (const auto &R : Ranges)
       P->addRange(R);
@@ -265,6 +276,17 @@ public:
     Notes.push_back(P);
   }
 
+  // FIXME: Instead of making an override, we could have default-initialized
+  // Ranges with {}, however it crashes the MSVC 2013 compiler.
+  void addNote(StringRef Msg, const PathDiagnosticLocation &Pos) {
+    std::vector<SourceRange> Ranges;
+    addNote(Msg, Pos, Ranges);
+  }
+
+  virtual const NoteList &getNotes() {
+    return Notes;
+  }
+
   /// \brief This allows for addition of meta data to the diagnostic.
   ///
   /// Currently, only the HTMLDiagnosticClient knows how to display it. 

lib/Rewrite/HTMLRewrite.cpp

@@ -324,6 +324,7 @@ void html::AddHeaderFooterInternalBuiltinCSS(Rewriter &R, FileID FID,
       " .msgT { padding:0x; spacing:0x }\n"
       " .msgEvent { background-color:#fff8b4; color:#000000 }\n"
       " .msgControl { background-color:#bbbbbb; color:#000000 }\n"
+      " .msgNote { background-color:#ddeeff; color:#000000 }\n"
       " .mrange { background-color:#dfddf3 }\n"
       " .mrange { border-bottom:1px solid #6F9DBE }\n"
       " .PathIndex { font-weight: bold; padding:0px 5px; "
@@ -343,8 +344,12 @@ void html::AddHeaderFooterInternalBuiltinCSS(Rewriter &R, FileID FID,
       "   border-collapse: collapse; border-spacing: 0px;\n"
       " }\n"
       " td.rowname {\n"
-      "   text-align:right; font-weight:bold; color:#444444;\n"
-      "   padding-right:2ex; }\n"
+      "   text-align: right;\n"
+      "   vertical-align: top;\n"
+      "   font-weight: bold;\n"
+      "   color:#444444;\n"
+      "   padding-right:2ex;\n"
+      " }\n"
       "</style>\n</head>\n<body>";
 
   // Generate header

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

@@ -344,3 +344,10 @@ bool AnalyzerOptions::shouldWidenLoops() {
     WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
   return WidenLoops.getValue();
 }
+
+bool AnalyzerOptions::shouldDisplayNotesAsEvents() {
+  if (!DisplayNotesAsEvents.hasValue())
+    DisplayNotesAsEvents =
+        getBooleanOption("notes-as-events", /*Default=*/false);
+  return DisplayNotesAsEvents.getValue();
+}

lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -3410,25 +3410,28 @@ void BugReporter::FlushReport(BugReport *exampleReport,
       exampleReport->getUniqueingLocation(),
       exampleReport->getUniqueingDecl()));
 
-  MaxBugClassSize = std::max(bugReports.size(),
-                             static_cast<size_t>(MaxBugClassSize));
-
-  // Generate the full path diagnostic, using the generation scheme
-  // specified by the PathDiagnosticConsumer. Note that we have to generate
-  // path diagnostics even for consumers which do not support paths, because
-  // the BugReporterVisitors may mark this bug as a false positive.
-  if (!bugReports.empty())
+  if (exampleReport->isPathSensitive()) {
+    // Generate the full path diagnostic, using the generation scheme
+    // specified by the PathDiagnosticConsumer. Note that we have to generate
+    // path diagnostics even for consumers which do not support paths, because
+    // the BugReporterVisitors may mark this bug as a false positive.
+    assert(!bugReports.empty());
+
+    MaxBugClassSize =
+        std::max(bugReports.size(), static_cast<size_t>(MaxBugClassSize));
+
     if (!generatePathDiagnostic(*D.get(), PD, bugReports))
       return;
 
-  MaxValidBugClassSize = std::max(bugReports.size(),
-                                  static_cast<size_t>(MaxValidBugClassSize));
+    MaxValidBugClassSize =
+        std::max(bugReports.size(), static_cast<size_t>(MaxValidBugClassSize));
 
-  // Examine the report and see if the last piece is in a header. Reset the
-  // report location to the last piece in the main source file.
-  AnalyzerOptions& Opts = getAnalyzerOptions();
-  if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
-    D->resetDiagnosticLocationToMainFile();
+    // Examine the report and see if the last piece is in a header. Reset the
+    // report location to the last piece in the main source file.
+    AnalyzerOptions &Opts = getAnalyzerOptions();
+    if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
+      D->resetDiagnosticLocationToMainFile();
+  }
 
   // If the path is empty, generate a single step path with the location
   // of the issue.
@@ -3441,6 +3444,27 @@ void BugReporter::FlushReport(BugReport *exampleReport,
     D->setEndOfPath(std::move(piece));
   }
 
+  PathPieces &Pieces = D->getMutablePieces();
+  if (getAnalyzerOptions().shouldDisplayNotesAsEvents()) {
+    // For path diagnostic consumers that don't support extra notes,
+    // we may optionally convert those to path notes.
+    for (auto I = exampleReport->getNotes().rbegin(),
+              E = exampleReport->getNotes().rend(); I != E; ++I) {
+      PathDiagnosticNotePiece *Piece = I->get();
+      PathDiagnosticEventPiece *ConvertedPiece =
+          new PathDiagnosticEventPiece(Piece->getLocation(),
+                                       Piece->getString());
+      for (const auto &R: Piece->getRanges())
+        ConvertedPiece->addRange(R);
+
+      Pieces.push_front(ConvertedPiece);
+    }
+  } else {
+    for (auto I = exampleReport->getNotes().rbegin(),
+              E = exampleReport->getNotes().rend(); I != E; ++I)
+      Pieces.push_front(*I);
+  }
+
   // Get the meta data.
   const BugReport::ExtraTextList &Meta = exampleReport->getExtraText();
   for (BugReport::ExtraTextList::const_iterator i = Meta.begin(),

lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

@@ -152,13 +152,30 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
   }
 
   // Process the path.
-  unsigned n = path.size();
-  unsigned max = n;
-
-  for (PathPieces::const_reverse_iterator I = path.rbegin(),
-       E = path.rend();
-        I != E; ++I, --n)
-    HandlePiece(R, FID, **I, n, max);
+  // Maintain the counts of extra note pieces separately.
+  unsigned TotalPieces = path.size();
+  unsigned TotalNotePieces =
+      std::count_if(path.begin(), path.end(),
+                    [](const IntrusiveRefCntPtr<PathDiagnosticPiece> &p) {
+                      return isa<PathDiagnosticNotePiece>(p.get());
+                    });
+
+  unsigned TotalRegularPieces = TotalPieces - TotalNotePieces;
+  unsigned NumRegularPieces = TotalRegularPieces;
+  unsigned NumNotePieces = TotalNotePieces;
+
+  for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
+    if (isa<PathDiagnosticNotePiece>(I->get())) {
+      // This adds diagnostic bubbles, but not navigation.
+      // Navigation through note pieces would be added later,
+      // as a separate pass through the piece list.
+      HandlePiece(R, FID, **I, NumNotePieces, TotalNotePieces);
+      --NumNotePieces;
+    } else {
+      HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces);
+      --NumRegularPieces;
+    }
+  }
 
   // Add line numbers, header, footer, etc.
 
@@ -192,24 +209,38 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
   int ColumnNumber = path.back()->getLocation().asLocation().getExpansionColumnNumber();
 
   // Add the name of the file as an <h1> tag.
-
   {
     std::string s;
     llvm::raw_string_ostream os(s);
 
     os << "<!-- REPORTHEADER -->\n"
-      << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
+       << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
           "<tr><td class=\"rowname\">File:</td><td>"
-      << html::EscapeText(DirName)
-      << html::EscapeText(Entry->getName())
-      << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
-         "<a href=\"#EndPath\">line "
-      << LineNumber
-      << ", column "
-      << ColumnNumber
-      << "</a></td></tr>\n"
-         "<tr><td class=\"rowname\">Description:</td><td>"
-      << D.getVerboseDescription() << "</td></tr>\n";
+       << html::EscapeText(DirName)
+       << html::EscapeText(Entry->getName())
+       << "</td></tr>\n<tr><td class=\"rowname\">Warning:</td><td>"
+          "<a href=\"#EndPath\">line "
+       << LineNumber
+       << ", column "
+       << ColumnNumber
+       << "</a><br />"
+       << D.getVerboseDescription() << "</td></tr>\n";
+
+    // The navigation across the extra notes pieces.
+    unsigned NumExtraPieces = 0;
+    for (const auto &Piece : path) {
+      if (const auto *P = dyn_cast<PathDiagnosticNotePiece>(Piece.get())) {
+        int LineNumber =
+            P->getLocation().asLocation().getExpansionLineNumber();
+        int ColumnNumber =
+            P->getLocation().asLocation().getExpansionColumnNumber();
+        os << "<tr><td class=\"rowname\">Note:</td><td>"
+           << "<a href=\"#Note" << NumExtraPieces << "\">line "
+           << LineNumber << ", column " << ColumnNumber << "</a><br />"
+           << P->getString() << "</td></tr>";
+        ++NumExtraPieces;
+      }
+    }
 
     // Output any other meta data.
 
@@ -385,13 +416,20 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
   // Create the html for the message.
 
   const char *Kind = nullptr;
+  bool IsNote = false;
+  bool SuppressIndex = (max == 1);
   switch (P.getKind()) {
   case PathDiagnosticPiece::Call:
-      llvm_unreachable("Calls should already be handled");
+      llvm_unreachable("Calls and extra notes should already be handled");
   case PathDiagnosticPiece::Event:  Kind = "Event"; break;
   case PathDiagnosticPiece::ControlFlow: Kind = "Control"; break;
     // Setting Kind to "Control" is intentional.
   case PathDiagnosticPiece::Macro: Kind = "Control"; break;
+  case PathDiagnosticPiece::Note:
+    Kind = "Note";
+    IsNote = true;
+    SuppressIndex = true;
+    break;
   }
 
   std::string sbuf;
@@ -399,7 +437,9 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
 
   os << "\n<tr><td class=\"num\"></td><td class=\"line\"><div id=\"";
 
-  if (num == max)
+  if (IsNote)
+    os << "Note" << num;
+  else if (num == max)
     os << "EndPath";
   else
     os << "Path" << num;
@@ -461,7 +501,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
 
   os << "\">";
 
-  if (max > 1) {
+  if (!SuppressIndex) {
     os << "<table class=\"msgT\"><tr><td valign=\"top\">";
     os << "<div class=\"PathIndex";
     if (Kind) os << " PathIndex" << Kind;
@@ -501,7 +541,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
 
     os << "':\n";
 
-    if (max > 1) {
+    if (!SuppressIndex) {
       os << "</td>";
       if (num < max) {
         os << "<td><div class=\"PathNav\"><a href=\"#";
@@ -523,7 +563,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
   else {
     os << html::EscapeText(P.getString());
 
-    if (max > 1) {
+    if (!SuppressIndex) {
       os << "</td>";
       if (num < max) {
         os << "<td><div class=\"PathNav\"><a href=\"#";

lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

@@ -281,6 +281,9 @@ static void ReportPiece(raw_ostream &o,
       ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, SM, LangOpts,
                   indent, depth);
       break;
+    case PathDiagnosticPiece::Note:
+      // FIXME: Extend the plist format to support those.
+      break;
   }
 }
 

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

@@ -113,16 +113,28 @@ public:
       Diag.Report(WarnLoc, WarnID) << PD->getShortDescription()
                                    << PD->path.back()->getRanges();
 
+      // First, add extra notes, even if paths should not be included.
+      for (const auto &Piece : PD->path) {
+        if (!isa<PathDiagnosticNotePiece>(Piece.get()))
+          continue;
+
+        SourceLocation NoteLoc = Piece->getLocation().asLocation();
+        Diag.Report(NoteLoc, NoteID) << Piece->getString()
+                                     << Piece->getRanges();
+      }
+
       if (!IncludePath)
         continue;
 
+      // Then, add the path notes if necessary.
       PathPieces FlatPath = PD->path.flatten(/*ShouldFlattenMacros=*/true);
-      for (PathPieces::const_iterator PI = FlatPath.begin(),
-                                      PE = FlatPath.end();
-           PI != PE; ++PI) {
-        SourceLocation NoteLoc = (*PI)->getLocation().asLocation();
-        Diag.Report(NoteLoc, NoteID) << (*PI)->getString()
-                                     << (*PI)->getRanges();
+      for (const auto &Piece : FlatPath) {
+        if (isa<PathDiagnosticNotePiece>(Piece.get()))
+          continue;
+
+        SourceLocation NoteLoc = Piece->getLocation().asLocation();
+        Diag.Report(NoteLoc, NoteID) << Piece->getString()
+                                     << Piece->getRanges();
       }
     }
   }