vlad.tsyrklevich created this revision.
While looking at checker output with https://reviews.llvm.org/D30289 applied I
realized I was missing results. After some digging I found that it's because
the Linux kernel makes heavy use of inlined functions included in header files,
and the resulting diagnostic path now included cross-file diagnostics which are
unsupported. This change adds support for cross-file diagnostic paths,
resulting in output that looks like
https://rawgit.com/vlad902/805a58327e12636fef119cdeb21ad639/raw/48f4183152a15e3e0f6fddfdc4edeb75c5a62295/report.html
If the diagnostic path is not cross-file, there is no change in the output.
https://reviews.llvm.org/D30406
Files:
lib/Rewrite/HTMLRewrite.cpp
lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
test/Analysis/diagnostics/diag-cross-file-boundaries.c
test/Analysis/diagnostics/diag-cross-file-boundaries.h
test/Analysis/html-diags-multifile.c
test/Coverage/html-diagnostics.c
test/Coverage/html-multifile-diagnostics.c
test/Coverage/html-multifile-diagnostics.h
www/analyzer/open_projects.html
Index: www/analyzer/open_projects.html
===================================================================
--- www/analyzer/open_projects.html
+++ www/analyzer/open_projects.html
@@ -107,13 +107,6 @@
<li>Bug Reporting
<ul>
- <li>Add support for displaying cross-file diagnostic paths in HTML output
- (used by <tt>scan-build</tt>).
- <p>Currently <tt>scan-build</tt> output does not display reports that span
- multiple files. The main problem is that we do not have a good format to
- display such paths in HTML output. <i>(Difficulty: Medium)</i> </p>
- </li>
-
<li>Refactor path diagnostic generation in <a href="http://clang.llvm.org/doxygen/BugReporter_8cpp_source.html";>BugReporter.cpp</a>.
<p>It would be great to have more code reuse between "Minimal" and
"Extensive" PathDiagnostic generation algorithms. One idea is to create an
Index: test/Coverage/html-multifile-diagnostics.h
===================================================================
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.h
@@ -0,0 +1,3 @@
+void f1(int *ptr) {
+ *ptr = 0;
+}
Index: test/Coverage/html-multifile-diagnostics.c
===================================================================
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+
+// REQUIRES: staticanalyzer
+
+// CHECK: <h3>Annotated Source Code</h3>
+
+// Make sure it's generated as multi-file HTML output
+// CHECK: <h4 class=FileName>{{.*}}html-multifile-diagnostics.c</h4>
+// CHECK: <h4 class=FileName>{{.*}}html-multifile-diagnostics.h</h4>
+
+// Without tweaking expr, the expr would hit to the line below
+// emitted to the output as comment.
+// CHECK: {{[D]ereference of null pointer}}
+
+#include "html-multifile-diagnostics.h"
+
+void f0() {
+ f1((int*)0);
+}
Index: test/Coverage/html-diagnostics.c
===================================================================
--- test/Coverage/html-diagnostics.c
+++ test/Coverage/html-diagnostics.c
@@ -6,6 +6,9 @@
// CHECK: <h3>Annotated Source Code</h3>
+// Make sure it's not generated as a multi-file HTML output
+// CHECK-NOT: <h4 class=FileName>{{.*}}
+
// Without tweaking expr, the expr would hit to the line below
// emitted to the output as comment.
// CHECK: {{[D]ereference of null pointer}}
Index: test/Analysis/html-diags-multifile.c
===================================================================
--- test/Analysis/html-diags-multifile.c
+++ test/Analysis/html-diags-multifile.c
@@ -1,10 +1,9 @@
// RUN: mkdir -p %t.dir
// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t.dir %s
-// RUN: ls %t.dir | not grep report
+// RUN: ls %t.dir | grep report
// RUN: rm -fR %t.dir
-// This tests that we do not currently emit HTML diagnostics for reports that
-// cross file boundaries.
+// This tests that we emit HTML diagnostics for reports that cross file boundaries.
#include "html-diags-multifile.h"
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.h
===================================================================
--- test/Analysis/diagnostics/diag-cross-file-boundaries.h
+++ /dev/null
@@ -1,4 +0,0 @@
-static void f() {
- int *p = 0;
- *p = 1; // expected-warning{{Dereference of null pointer}}
-}
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.c
===================================================================
--- test/Analysis/diagnostics/diag-cross-file-boundaries.c
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=html -o PR12421.html %s 2>&1 | FileCheck %s
-
-// Test for PR12421
-#include "diag-cross-file-boundaries.h"
-
-int main(){
- f();
- return 0;
-}
-
-// CHECK: warning: Path diagnostic report is not generated.
Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -11,6 +11,7 @@
//
//===----------------------------------------------------------------------===//
+#include "llvm/ADT/SmallSet.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/Basic/FileManager.h"
@@ -56,6 +57,10 @@
return "HTMLDiagnostics";
}
+ bool supportsCrossFileDiagnostics() const override {
+ return true;
+ }
+
unsigned ProcessMacroPiece(raw_ostream &os,
const PathDiagnosticMacroPiece& P,
unsigned num);
@@ -69,6 +74,15 @@
void ReportDiag(const PathDiagnostic& D,
FilesMade *filesMade);
+
+ // Rewrite the file specified by FID with HTML formatting.
+ void RewriteFile(Rewriter &R, const SourceManager& SMgr,
+ const PathPieces& path, FileID FID);
+
+ // Add HTML header/footers to file specified by FID
+ void FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
+ const SourceManager& SMgr, const PathPieces& path,
+ FileID FID, const FileEntry *Entry, const char *declName);
};
} // end anonymous namespace
@@ -121,24 +135,19 @@
// First flatten out the entire path to make it easier to use.
PathPieces path = D.path.flatten(/*ShouldFlattenMacros=*/false);
- // The path as already been prechecked that all parts of the path are
- // from the same file and that it is non-empty.
- const SourceManager &SMgr = path.front()->getLocation().getManager();
+ // The path as already been prechecked that the path is non-empty.
assert(!path.empty());
- FileID FID =
- path.front()->getLocation().asLocation().getExpansionLoc().getFileID();
- assert(FID.isValid());
+ const SourceManager &SMgr = path.front()->getLocation().getManager();
// Create a new rewriter to generate HTML.
Rewriter R(const_cast<SourceManager&>(SMgr), PP.getLangOpts());
// Get the function/method name
SmallString<128> declName("unknown");
int offsetDecl = 0;
if (const Decl *DeclWithIssue = D.getDeclWithIssue()) {
- if (const NamedDecl *ND = dyn_cast<NamedDecl>(DeclWithIssue)) {
+ if (const NamedDecl *ND = dyn_cast<NamedDecl>(DeclWithIssue))
declName = ND->getDeclName().getAsString();
- }
if (const Stmt *Body = DeclWithIssue->getBody()) {
// Retrieve the relative position of the declaration which will be used
@@ -151,49 +160,121 @@
}
}
- // Process the path.
- // Maintain the counts of extra note pieces separately.
- unsigned TotalPieces = path.size();
- unsigned TotalNotePieces =
- std::count_if(path.begin(), path.end(),
- [](const std::shared_ptr<PathDiagnosticPiece> &p) {
- return isa<PathDiagnosticNotePiece>(*p);
- });
+ // Rewrite source files as HTML for every new file the path crosses
+ std::vector<FileID> FileIDs;
+ for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
+ FileID FID = (*I)->getLocation().asLocation().getExpansionLoc().getFileID();
+ if (std::find(FileIDs.begin(), FileIDs.end(), FID) != FileIDs.end())
+ continue;
- unsigned TotalRegularPieces = TotalPieces - TotalNotePieces;
- unsigned NumRegularPieces = TotalRegularPieces;
- unsigned NumNotePieces = TotalNotePieces;
+ FileIDs.push_back(FID);
+ RewriteFile(R, SMgr, path, FID);
+ }
- 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;
+ if (FileIDs.size() > 1) {
+ std::vector<FileID>::iterator I, E;
+
+ // Prefix file names to every file if this is a multi-file report
+ for (I = FileIDs.begin(), E = FileIDs.end(); I != E; I++) {
+ SourceLocation StartLoc = SMgr.getLocForStartOfFile(*I);
+ const FileEntry* Entry = SMgr.getFileEntryForID(*I);
+
+ std::string s;
+ llvm::raw_string_ostream os(s);
+ os << "<h4 class=FileName>" << Entry->getName() << "</h4>\n";
+ R.InsertTextBefore(StartLoc, os.str());
+ }
+
+ // Then append the HTML for other files in this report to the end of the
+ // main file's HTML
+ for (I = FileIDs.begin(), E = FileIDs.end(); I != E; I++) {
+ if (*I == SMgr.getMainFileID())
+ continue;
+
+ const RewriteBuffer *Buf = R.getRewriteBufferFor(*I);
+
+ std::string file;
+ llvm::raw_string_ostream o(file);
+ o << "<hr>\n";
+ for (RewriteBuffer::iterator BI = Buf->begin(), BE = Buf->end(); BI!=BE; ++BI)
+ o << *BI;
+
+ SourceLocation MainFileEndLoc = SMgr.getLocForEndOfFile(SMgr.getMainFileID());
+ R.InsertTextAfter(MainFileEndLoc, o.str());
}
}
- // Add line numbers, header, footer, etc.
+ // Get the rewrite buffer.
+ const RewriteBuffer *Buf = R.getRewriteBufferFor(SMgr.getMainFileID());
+ if (!Buf) {
+ llvm::errs() << "warning: no diagnostics generated for main file.\n";
+ return;
+ }
- // unsigned FID = R.getSourceMgr().getMainFileID();
- html::EscapeText(R, FID);
- html::AddLineNumbers(R, FID);
+ // Add CSS, header, and footer.
+ const FileEntry* Entry = SMgr.getFileEntryForID(SMgr.getMainFileID());
+ FinalizeHTML(D, R, SMgr, path, SMgr.getMainFileID(), Entry, declName.c_str());
- // If we have a preprocessor, relex the file and syntax highlight.
- // We might not have a preprocessor if we come from a deserialized AST file,
- // for example.
+ // Create a path for the target HTML file.
+ int FD;
+ SmallString<128> Model, ResultPath;
- html::SyntaxHighlight(R, FID, PP);
- html::HighlightMacros(R, FID, PP);
+ if (!AnalyzerOpts.shouldWriteStableReportFilename()) {
+ llvm::sys::path::append(Model, Directory, "report-%%%%%%.html");
+ if (std::error_code EC =
+ llvm::sys::fs::make_absolute(Model)) {
+ llvm::errs() << "warning: could not make '" << Model
+ << "' absolute: " << EC.message() << '\n';
+ return;
+ }
+ if (std::error_code EC =
+ llvm::sys::fs::createUniqueFile(Model, FD, ResultPath)) {
+ llvm::errs() << "warning: could not create file in '" << Directory
+ << "': " << EC.message() << '\n';
+ return;
+ }
+
+ } else {
+ int i = 1;
+ std::error_code EC;
+ do {
+ // Find a filename which is not already used
+ std::stringstream filename;
+ Model = "";
+ filename << "report-"
+ << llvm::sys::path::filename(Entry->getName()).str()
+ << "-" << declName.c_str()
+ << "-" << offsetDecl
+ << "-" << i << ".html";
+ llvm::sys::path::append(Model, Directory,
+ filename.str());
+ EC = llvm::sys::fs::openFileForWrite(Model,
+ FD,
+ llvm::sys::fs::F_RW |
+ llvm::sys::fs::F_Excl);
+ if (EC && EC != llvm::errc::file_exists) {
+ llvm::errs() << "warning: could not create file '" << Model
+ << "': " << EC.message() << '\n';
+ return;
+ }
+ i++;
+ } while (EC);
+ }
- // Get the full directory name of the analyzed file.
+ llvm::raw_fd_ostream os(FD, true);
- const FileEntry* Entry = SMgr.getFileEntryForID(FID);
+ if (filesMade)
+ filesMade->addDiagnostic(D, getName(),
+ llvm::sys::path::filename(ResultPath));
+ // Emit the HTML to disk.
+ for (RewriteBuffer::iterator I = Buf->begin(), E = Buf->end(); I!=E; ++I)
+ os << *I;
+}
+
+void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
+ const SourceManager& SMgr, const PathPieces& path, FileID FID,
+ const FileEntry *Entry, const char *declName) {
// This is a cludge; basically we want to append either the full
// working directory if we have no directory information. This is
// a work in progress.
@@ -306,73 +387,48 @@
R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), os.str());
}
- // Add CSS, header, and footer.
+ html::AddHeaderFooterInternalBuiltinCSS(R, SMgr.getMainFileID(), Entry->getName());
+}
- html::AddHeaderFooterInternalBuiltinCSS(R, FID, Entry->getName());
+void HTMLDiagnostics::RewriteFile(Rewriter &R, const SourceManager& SMgr,
+ const PathPieces& path, FileID FID) {
+ // Process the path.
+ // Maintain the counts of extra note pieces separately.
+ unsigned TotalPieces = path.size();
+ unsigned TotalNotePieces =
+ std::count_if(path.begin(), path.end(),
+ [](const std::shared_ptr<PathDiagnosticPiece> &p) {
+ return isa<PathDiagnosticNotePiece>(*p);
+ });
- // Get the rewrite buffer.
- const RewriteBuffer *Buf = R.getRewriteBufferFor(FID);
+ unsigned TotalRegularPieces = TotalPieces - TotalNotePieces;
+ unsigned NumRegularPieces = TotalRegularPieces;
+ unsigned NumNotePieces = TotalNotePieces;
- if (!Buf) {
- llvm::errs() << "warning: no diagnostics generated for main file.\n";
- return;
+ 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;
+ }
}
- // Create a path for the target HTML file.
- int FD;
- SmallString<128> Model, ResultPath;
-
- if (!AnalyzerOpts.shouldWriteStableReportFilename()) {
- llvm::sys::path::append(Model, Directory, "report-%%%%%%.html");
- if (std::error_code EC =
- llvm::sys::fs::make_absolute(Model)) {
- llvm::errs() << "warning: could not make '" << Model
- << "' absolute: " << EC.message() << '\n';
- return;
- }
- if (std::error_code EC =
- llvm::sys::fs::createUniqueFile(Model, FD, ResultPath)) {
- llvm::errs() << "warning: could not create file in '" << Directory
- << "': " << EC.message() << '\n';
- return;
- }
-
- } else {
- int i = 1;
- std::error_code EC;
- do {
- // Find a filename which is not already used
- std::stringstream filename;
- Model = "";
- filename << "report-"
- << llvm::sys::path::filename(Entry->getName()).str()
- << "-" << declName.c_str()
- << "-" << offsetDecl
- << "-" << i << ".html";
- llvm::sys::path::append(Model, Directory,
- filename.str());
- EC = llvm::sys::fs::openFileForWrite(Model,
- FD,
- llvm::sys::fs::F_RW |
- llvm::sys::fs::F_Excl);
- if (EC && EC != llvm::errc::file_exists) {
- llvm::errs() << "warning: could not create file '" << Model
- << "': " << EC.message() << '\n';
- return;
- }
- i++;
- } while (EC);
- }
+ // Add line numbers, header, footer, etc.
- llvm::raw_fd_ostream os(FD, true);
+ html::EscapeText(R, FID);
+ html::AddLineNumbers(R, FID);
- if (filesMade)
- filesMade->addDiagnostic(D, getName(),
- llvm::sys::path::filename(ResultPath));
+ // If we have a preprocessor, relex the file and syntax highlight.
+ // We might not have a preprocessor if we come from a deserialized AST file,
+ // for example.
- // Emit the HTML to disk.
- for (RewriteBuffer::iterator I = Buf->begin(), E = Buf->end(); I!=E; ++I)
- os << *I;
+ html::SyntaxHighlight(R, FID, PP);
+ html::HighlightMacros(R, FID, PP);
}
void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
Index: lib/Rewrite/HTMLRewrite.cpp
===================================================================
--- lib/Rewrite/HTMLRewrite.cpp
+++ lib/Rewrite/HTMLRewrite.cpp
@@ -289,6 +289,7 @@
" body { color:#000000; background-color:#ffffff }\n"
" body { font-family:Helvetica, sans-serif; font-size:10pt }\n"
" h1 { font-size:14pt }\n"
+ " .FileName { margin-top: 5px; margin-bottom: 5px; }\n"
" .code { border-collapse:collapse; width:100%; }\n"
" .code { font-family: \"Monospace\", monospace; font-size:10pt }\n"
" .code { line-height: 1.2em }\n"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
