diff --git a/clang/include/clang/Analysis/PathDiagnostic.h b/clang/include/clang/Analysis/PathDiagnostic.h --- a/clang/include/clang/Analysis/PathDiagnostic.h +++ b/clang/include/clang/Analysis/PathDiagnostic.h @@ -151,11 +151,14 @@ /// Only runs visitors, no output generated. None, - /// Used for HTML, SARIF, and text output. + /// Used for SARIF and text output. Minimal, /// Used for plist output, used for "arrows" generation. Extensive, + + /// Used for HTML, shows both "arrows" and control notes. + Everything }; virtual PathGenerationScheme getGenerationScheme() const { return Minimal; } @@ -164,7 +167,11 @@ return getGenerationScheme() != None; } - bool shouldAddPathEdges() const { return getGenerationScheme() == Extensive; } + bool shouldAddPathEdges() const { return getGenerationScheme() >= Extensive; } + bool shouldAddControlNotes() const { + return getGenerationScheme() == Minimal || + getGenerationScheme() == Everything; + } virtual bool supportsLogicalOpControlFlow() const { return false; } diff --git a/clang/lib/Rewrite/HTMLRewrite.cpp b/clang/lib/Rewrite/HTMLRewrite.cpp --- a/clang/lib/Rewrite/HTMLRewrite.cpp +++ b/clang/lib/Rewrite/HTMLRewrite.cpp @@ -371,6 +371,7 @@ .msg { border-radius:5px } .msg { font-family:Helvetica, sans-serif; font-size:8pt } .msg { float:left } +.msg { position:relative } .msg { padding:0.25em 1ex 0.25em 1ex } .msg { margin-top:10px; margin-bottom:10px } .msg { font-weight:bold } diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -188,6 +188,9 @@ PathPieces &getMutablePieces() { return PD->getMutablePieces(); } bool shouldAddPathEdges() const { return Consumer->shouldAddPathEdges(); } + bool shouldAddControlNotes() const { + return Consumer->shouldAddControlNotes(); + } bool shouldGenerateDiagnostics() const { return Consumer->shouldGenerateDiagnostics(); } @@ -1232,8 +1235,11 @@ } else if (auto BE = P.getAs()) { - if (!C.shouldAddPathEdges()) { + if (C.shouldAddControlNotes()) { generateMinimalDiagForBlockEdge(C, *BE); + } + + if (!C.shouldAddPathEdges()) { return; } @@ -1254,12 +1260,14 @@ // do-while statements are explicitly excluded here auto p = std::make_shared( - L, "Looping back to the head " - "of the loop"); + L, "Looping back to the head of the loop"); p->setPrunable(true); addEdgeToPath(C.getActivePath(), PrevLoc, p->getLocation()); - C.getActivePath().push_front(std::move(p)); + // We might've added a very similar control node already + if (!C.shouldAddControlNotes()) { + C.getActivePath().push_front(std::move(p)); + } if (const auto *CS = dyn_cast_or_null(Body)) { addEdgeToPath(C.getActivePath(), PrevLoc, @@ -1300,10 +1308,14 @@ auto PE = std::make_shared(L, str); PE->setPrunable(true); addEdgeToPath(C.getActivePath(), PrevLoc, PE->getLocation()); - C.getActivePath().push_front(std::move(PE)); + + // We might've added a very similar control node already + if (!C.shouldAddControlNotes()) { + C.getActivePath().push_front(std::move(PE)); + } } } else if (isa(Term) || isa(Term) || - isa(Term)) { + isa(Term)) { PathDiagnosticLocation L(Term, SM, C.getCurrLocationContext()); addEdgeToPath(C.getActivePath(), PrevLoc, L); } diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp --- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -27,6 +27,7 @@ #include "clang/Rewrite/Core/Rewriter.h" #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Sequence.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator_range.h" @@ -77,60 +78,78 @@ void FlushDiagnosticsImpl(std::vector &Diags, FilesMade *filesMade) override; - StringRef getName() const override { - return "HTMLDiagnostics"; - } + StringRef getName() const override { return "HTMLDiagnostics"; } bool supportsCrossFileDiagnostics() const override { return SupportsCrossFileDiagnostics; } - unsigned ProcessMacroPiece(raw_ostream &os, - const PathDiagnosticMacroPiece& P, + unsigned ProcessMacroPiece(raw_ostream &os, const PathDiagnosticMacroPiece &P, unsigned num); + unsigned ProcessControlFlowPiece(Rewriter &R, FileID BugFileID, + const PathDiagnosticControlFlowPiece &P, + unsigned Number); + void HandlePiece(Rewriter &R, FileID BugFileID, const PathDiagnosticPiece &P, const std::vector &PopUpRanges, unsigned num, unsigned max); - void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range, + void HighlightRange(Rewriter &R, FileID BugFileID, SourceRange Range, const char *HighlightStart = "", const char *HighlightEnd = ""); - void ReportDiag(const PathDiagnostic& D, - FilesMade *filesMade); + void ReportDiag(const PathDiagnostic &D, FilesMade *filesMade); // Generate the full HTML report - std::string GenerateHTML(const PathDiagnostic& D, Rewriter &R, - const SourceManager& SMgr, const PathPieces& path, + std::string GenerateHTML(const PathDiagnostic &D, Rewriter &R, + const SourceManager &SMgr, const PathPieces &path, const char *declName); // Add HTML header/footers to file specified by FID - void FinalizeHTML(const PathDiagnostic& D, Rewriter &R, - const SourceManager& SMgr, const PathPieces& path, + void FinalizeHTML(const PathDiagnostic &D, Rewriter &R, + const SourceManager &SMgr, const PathPieces &path, FileID FID, const FileEntry *Entry, const char *declName); // Rewrite the file specified by FID with HTML formatting. - void RewriteFile(Rewriter &R, const PathPieces& path, FileID FID); + void RewriteFile(Rewriter &R, const PathPieces &path, FileID FID); + PathGenerationScheme getGenerationScheme() const override { + return Everything; + } private: + void addArrowSVGs(Rewriter &R, FileID BugFileID, unsigned NumberOfArrows); + /// \return Javascript for displaying shortcuts help; StringRef showHelpJavascript(); /// \return Javascript for navigating the HTML report using j/k keys. StringRef generateKeyboardNavigationJavascript(); + /// \return Javascript for drawing control-flow arrows. + StringRef generateArrowDrawingJavascript(); + /// \return JavaScript for an option to only show relevant lines. - std::string showRelevantLinesJavascript( - const PathDiagnostic &D, const PathPieces &path); + std::string showRelevantLinesJavascript(const PathDiagnostic &D, + const PathPieces &path); /// Write executed lines from \p D in JSON format into \p os. - void dumpCoverageData(const PathDiagnostic &D, - const PathPieces &path, + void dumpCoverageData(const PathDiagnostic &D, const PathPieces &path, llvm::raw_string_ostream &os); }; +bool isArrowPiece(const PathDiagnosticPiece &P) { + return isa(P) && P.getString().empty(); +} + +unsigned getPathSizeWithoutArrows(const PathPieces &Path) { + unsigned TotalPieces = Path.size(); + unsigned TotalArrowPieces = llvm::count_if( + Path, [](const PathDiagnosticPieceRef &P) { return isArrowPiece(*P); }); + return TotalPieces - TotalArrowPieces; +} + } // namespace void ento::createHTMLDiagnosticConsumer( @@ -452,10 +471,11 @@ if (event.defaultPrevented) { return; } - if (event.key == "S") { + // SHIFT + S + if (event.shiftKey && event.keyCode == 83) { var checked = document.getElementsByName("showCounterexample")[0].checked; filterCounterexample(!checked); - document.getElementsByName("showCounterexample")[0].checked = !checked; + document.getElementsByName("showCounterexample")[0].click(); } else { return; } @@ -475,6 +495,11 @@ + + )<<<"; @@ -503,6 +528,9 @@ R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), generateKeyboardNavigationJavascript()); + R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), + generateArrowDrawingJavascript()); + // Checkbox and javascript for filtering the output to the counterexample. R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), showRelevantLinesJavascript(D, path)); @@ -570,6 +598,7 @@ Close )<<<"; + R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), os.str()); } @@ -616,7 +645,7 @@ << ColumnNumber << " -->\n"; - os << "\n\n"; + os << "\n\n"; // Mark the end of the tags. os << "\n\n"; @@ -711,23 +740,25 @@ } } -void HTMLDiagnostics::RewriteFile(Rewriter &R, - const PathPieces& path, FileID FID) { +void HTMLDiagnostics::RewriteFile(Rewriter &R, 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 PathDiagnosticPieceRef &p) { + unsigned TotalPieces = getPathSizeWithoutArrows(path); + unsigned TotalNotePieces = + llvm::count_if(path, [](const PathDiagnosticPieceRef &p) { return isa(*p); }); - unsigned PopUpPieceCount = std::count_if( - path.begin(), path.end(), [](const PathDiagnosticPieceRef &p) { + unsigned PopUpPieceCount = + llvm::count_if(path, [](const PathDiagnosticPieceRef &p) { return isa(*p); }); unsigned TotalRegularPieces = TotalPieces - TotalNotePieces - PopUpPieceCount; unsigned NumRegularPieces = TotalRegularPieces; unsigned NumNotePieces = TotalNotePieces; + unsigned NumberOfArrows = 0; // Stores the count of the regular piece indices. std::map IndexMap; @@ -744,6 +775,11 @@ // as a separate pass through the piece list. HandlePiece(R, FID, Piece, PopUpRanges, NumNotePieces, TotalNotePieces); --NumNotePieces; + + } else if (isArrowPiece(Piece)) { + NumberOfArrows = ProcessControlFlowPiece( + R, FID, cast(Piece), NumberOfArrows); + } else { HandlePiece(R, FID, Piece, PopUpRanges, NumRegularPieces, TotalRegularPieces); @@ -771,7 +807,7 @@ if (PopUpPieceIndex > 0) --IndexMap[NumRegularPieces]; - } else if (!isa(Piece)) { + } else if (!isa(Piece) && !isArrowPiece(Piece)) { --NumRegularPieces; } } @@ -783,6 +819,8 @@ html::EscapeText(R, FID); html::AddLineNumbers(R, FID); + addArrowSVGs(R, FID, NumberOfArrows); + // 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. @@ -1049,6 +1087,79 @@ return num; } +void HTMLDiagnostics::addArrowSVGs(Rewriter &R, FileID BugFileID, + unsigned NumberOfArrows) { + std::string S; + llvm::raw_string_ostream OS(S); + + OS << R"<<<( + + + + + + + + +)<<<"; + + for (unsigned Index : llvm::seq(0u, NumberOfArrows)) { + OS << " \n"; + } + + OS << R"<<<( + + +)<<<"; + + R.InsertTextBefore(R.getSourceMgr().getLocForStartOfFile(BugFileID), + OS.str()); +} + +std::string getSpanBeginForControl(const char *ClassName, unsigned Index) { + std::string Result; + llvm::raw_string_ostream OS(Result); + OS << ""; + return OS.str(); +} + +std::string getSpanBeginForControlStart(unsigned Index) { + return getSpanBeginForControl("start", Index); +} + +std::string getSpanBeginForControlEnd(unsigned Index) { + return getSpanBeginForControl("end", Index); +} + +unsigned HTMLDiagnostics::ProcessControlFlowPiece( + Rewriter &R, FileID BugFileID, const PathDiagnosticControlFlowPiece &P, + unsigned Number) { + for (const PathDiagnosticLocationPair &LPair : P) { + std::string Start = getSpanBeginForControlStart(Number), + End = getSpanBeginForControlEnd(Number++); + + HighlightRange(R, BugFileID, LPair.getStart().asRange().getBegin(), + Start.c_str()); + HighlightRange(R, BugFileID, LPair.getEnd().asRange().getBegin(), + End.c_str()); + } + + return Number; +} + void HTMLDiagnostics::HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range, const char *HighlightStart, @@ -1171,3 +1282,224 @@ )<<<"; } + +StringRef HTMLDiagnostics::generateArrowDrawingJavascript() { + return R"<<<( + + )<<<"; +} diff --git a/clang/test/Analysis/html_diagnostics/control-arrows.cpp b/clang/test/Analysis/html_diagnostics/control-arrows.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/html_diagnostics/control-arrows.cpp @@ -0,0 +1,27 @@ +// RUN: rm -fR %t +// RUN: mkdir %t +// RUN: %clang_analyze_cc1 -analyzer-checker=core \ +// RUN: -analyzer-output=html -o %t -verify %s +// RUN: cat %t/report-*.html | FileCheck %s + +int dereference(int *x) { + return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}} +} + +int foobar(bool cond, int *x) { + if (cond) + x = 0; + return dereference(x); +} + +// CHECK: +// CHECK-NOT: +// CHECK: +// CHECK-NEXT: +// +// Except for arrows we still want to have grey bubbles with control notes. +// CHECK:
2
+// CHECK-SAME: Taking true branch