[analyzer diagnostics] Change CompactPathDiagnostic to recursively compact diagnostics in calls into macro pieces.

Also fix handling of macros within calls in the HTMLDiagnostics.

This also adds a test case for r151774.

llvm-svn: 151872

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -564,7 +564,7 @@ public:
 // "Minimal" path diagnostic generation algorithm.
 //===----------------------------------------------------------------------===//
 
-static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM);
+static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM);
 
 static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
                                           PathDiagnosticBuilder &PDB,
@@ -877,7 +877,7 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
 
   // After constructing the full PathDiagnostic, do a pass over it to compact
   // PathDiagnosticPieces that occur within a macro.
-  CompactPathDiagnostic(PD, PDB.getSourceManager());
+  CompactPathDiagnostic(PD.getMutablePieces(), PDB.getSourceManager());
 }
 
 //===----------------------------------------------------------------------===//
@@ -1652,7 +1652,7 @@ MakeReportGraph(const ExplodedGraph* G,
 
 /// CompactPathDiagnostic - This function postprocesses a PathDiagnostic object
 ///  and collapses PathDiagosticPieces that are expanded by macros.
-static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
+static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM) {
   typedef std::vector<std::pair<IntrusiveRefCntPtr<PathDiagnosticMacroPiece>,
                                 SourceLocation> > MacroStackTy;
 
@@ -1662,10 +1662,18 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
   MacroStackTy MacroStack;
   PiecesTy Pieces;
 
-  for (PathPieces::const_iterator I = PD.path.begin(), E = PD.path.end();
+  for (PathPieces::const_iterator I = path.begin(), E = path.end();
        I!=E; ++I) {
+    
+    PathDiagnosticPiece *piece = I->getPtr();
+
+    // Recursively compact calls.
+    if (PathDiagnosticCallPiece *call=dyn_cast<PathDiagnosticCallPiece>(piece)){
+      CompactPathDiagnostic(call->path, SM);
+    }
+    
     // Get the location of the PathDiagnosticPiece.
-    const FullSourceLoc Loc = (*I)->getLocation().asLocation();
+    const FullSourceLoc Loc = piece->getLocation().asLocation();
 
     // Determine the instantiation location, which is the location we group
     // related PathDiagnosticPieces.
@@ -1675,7 +1683,7 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
 
     if (Loc.isFileID()) {
       MacroStack.clear();
-      Pieces.push_back(*I);
+      Pieces.push_back(piece);
       continue;
     }
 
@@ -1683,7 +1691,7 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
 
     // Is the PathDiagnosticPiece within the same macro group?
     if (!MacroStack.empty() && InstantiationLoc == MacroStack.back().second) {
-      MacroStack.back().first->subPieces.push_back(*I);
+      MacroStack.back().first->subPieces.push_back(piece);
       continue;
     }
 
@@ -1714,7 +1722,7 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
       // Create a new macro group and add it to the stack.
       PathDiagnosticMacroPiece *NewGroup =
         new PathDiagnosticMacroPiece(
-          PathDiagnosticLocation::createSingleLocation((*I)->getLocation()));
+          PathDiagnosticLocation::createSingleLocation(piece->getLocation()));
 
       if (MacroGroup)
         MacroGroup->subPieces.push_back(NewGroup);
@@ -1728,15 +1736,14 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
     }
 
     // Finally, add the PathDiagnosticPiece to the group.
-    MacroGroup->subPieces.push_back(*I);
+    MacroGroup->subPieces.push_back(piece);
   }
 
   // Now take the pieces and construct a new PathDiagnostic.
-  PD.getMutablePieces().clear();
+  path.clear();
 
-  for (PiecesTy::iterator I=Pieces.begin(), E=Pieces.end(); I!=E; ++I) {
-    PD.getMutablePieces().push_back(*I);
-  }
+  for (PiecesTy::iterator I=Pieces.begin(), E=Pieces.end(); I!=E; ++I)
+    path.push_back(*I);
 }
 
 void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

@@ -95,7 +95,8 @@ void HTMLDiagnostics::FlushDiagnosticsImpl(
   }
 }
 
-static void flattenPath(PathPieces &path, const PathPieces &oldPath) {
+static void flattenPath(PathPieces &primaryPath, PathPieces &currentPath,
+                        const PathPieces &oldPath) {
   for (PathPieces::const_iterator it = oldPath.begin(), et = oldPath.end();
        it != et; ++it ) {
     PathDiagnosticPiece *piece = it->getPtr();
@@ -104,16 +105,24 @@ static void flattenPath(PathPieces &path, const PathPieces &oldPath) {
       IntrusiveRefCntPtr<PathDiagnosticEventPiece> callEnter =
         call->getCallEnterEvent();
       if (callEnter)
-        path.push_back(callEnter);
-      flattenPath(path, call->path);
+        currentPath.push_back(callEnter);
+      flattenPath(primaryPath, primaryPath, call->path);
       IntrusiveRefCntPtr<PathDiagnosticEventPiece> callExit =
         call->getCallExitEvent();
       if (callExit)
-        path.push_back(callExit);
+        currentPath.push_back(callExit);
       continue;
     }
-
-    path.push_back(piece);
+    if (PathDiagnosticMacroPiece *macro =
+        dyn_cast<PathDiagnosticMacroPiece>(piece)) {
+      currentPath.push_back(piece);
+      PathPieces newPath;
+      flattenPath(primaryPath, newPath, macro->subPieces);
+      macro->subPieces = newPath;
+      continue;
+    }
+    
+    currentPath.push_back(piece);
   }
 }
 
@@ -144,7 +153,7 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
 
   // First flatten out the entire path to make it easier to use.
   PathPieces path;
-  flattenPath(path, D.path);
+  flattenPath(path, path, D.path);
 
   // The path as already been prechecked that all parts of the path are
   // from the same file and that it is non-empty.

clang/test/Analysis/html-diags-multifile.c

@@ -0,0 +1,17 @@
+// RUN: mkdir -p %t.dir
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t.dir
+// RUN: ls %t.dir | grep report | count 0
+// RUN: rm -fR %t.dir
+// REQUIRES: shell
+
+// This tests that we do not currently emit HTML diagnostics for reports that
+// cross file boundaries.
+
+#include "html-diags-multifile.h"
+
+#define CALL_HAS_BUG(q) has_bug(q)
+
+void test_call_macro() {
+  CALL_HAS_BUG(0);
+}
+

clang/test/Analysis/html-diags-multifile.h

@@ -0,0 +1,4 @@
+#define DEREF(p) *p = 0xDEADBEEF
+void has_bug(int *p) {
+  DEREF(p);
+}

clang/test/Analysis/html-diags.c

@@ -0,0 +1,20 @@
+// RUN: mkdir %t.dir
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %T.dir %s
+// RUN: rm -fR %t.dir
+
+// Currently this test mainly checks that the HTML diagnostics doesn't crash
+// when handling macros will calls with macros.  We should actually validate
+// the output, but that requires being able to match against a specifically
+// generate HTML file.
+
+#define DEREF(p) *p = 0xDEADBEEF
+
+void has_bug(int *p) {
+  DEREF(p);
+}
+
+#define CALL_HAS_BUG(q) has_bug(q)
+
+void test_call_macro() {
+  CALL_HAS_BUG(0);
+}