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

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 7 12:25:10 PDT 2016


Author: dergachev
Date: Fri Oct  7 14:25:10 2016
New Revision: 283584

URL: http://llvm.org/viewvc/llvm-project?rev=283584&view=rev
Log:
[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.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
    cfe/trunk/lib/Rewrite/HTMLRewrite.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Oct  7 14:25:10 2016
@@ -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),

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri Oct  7 14:25:10 2016
@@ -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. 

Modified: cfe/trunk/lib/Rewrite/HTMLRewrite.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/HTMLRewrite.cpp?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/lib/Rewrite/HTMLRewrite.cpp (original)
+++ cfe/trunk/lib/Rewrite/HTMLRewrite.cpp Fri Oct  7 14:25:10 2016
@@ -324,6 +324,7 @@ void html::AddHeaderFooterInternalBuilti
       " .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::AddHeaderFooterInternalBuilti
       "   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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Fri Oct  7 14:25:10 2016
@@ -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();
+}

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Oct  7 14:25:10 2016
@@ -3410,25 +3410,28 @@ void BugReporter::FlushReport(BugReport
       exampleReport->getUniqueingLocation(),
       exampleReport->getUniqueingDecl()));
 
-  MaxBugClassSize = std::max(bugReports.size(),
-                             static_cast<size_t>(MaxBugClassSize));
+  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));
 
-  // 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 (!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
     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(),

Modified: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Fri Oct  7 14:25:10 2016
@@ -152,13 +152,30 @@ void HTMLDiagnostics::ReportDiag(const P
   }
 
   // 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 P
   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(Rewrit
   // 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(Rewrit
 
   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(Rewrit
 
   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(Rewrit
 
     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(Rewrit
   else {
     os << html::EscapeText(P.getString());
 
-    if (max > 1) {
+    if (!SuppressIndex) {
       os << "</td>";
       if (num < max) {
         os << "<td><div class=\"PathNav\"><a href=\"#";

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Fri Oct  7 14:25:10 2016
@@ -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;
   }
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=283584&r1=283583&r2=283584&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Fri Oct  7 14:25:10 2016
@@ -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();
       }
     }
   }




More information about the cfe-commits mailing list