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 @@ -153,11 +153,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; } @@ -166,7 +169,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 @@ -10,11 +10,11 @@ // //===----------------------------------------------------------------------===// -#include "clang/Analysis/IssueHash.h" -#include "clang/Analysis/PathDiagnostic.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/Stmt.h" +#include "clang/Analysis/IssueHash.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" @@ -26,6 +26,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" @@ -88,6 +89,10 @@ 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); @@ -112,14 +117,22 @@ // Rewrite the file specified by FID with HTML formatting. 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); @@ -130,6 +143,17 @@ 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( @@ -434,7 +458,7 @@ if (event.key == "S") { var checked = document.getElementsByName("showCounterexample")[0].checked; filterCounterexample(!checked); - document.getElementsByName("showCounterexample")[0].checked = !checked; + document.getElementsByName("showCounterexample")[0].click(); } else { return; } @@ -454,6 +478,11 @@ + + )<<<"; @@ -482,6 +511,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)); @@ -549,6 +581,7 @@ Close )<<<"; + R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), os.str()); } @@ -595,7 +628,7 @@ << ColumnNumber << " -->\n"; - os << "\n\n"; + os << "\n\n"; // Mark the end of the tags. os << "\n\n"; @@ -690,23 +723,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; @@ -723,6 +758,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); @@ -750,7 +790,7 @@ if (PopUpPieceIndex > 0) --IndexMap[NumRegularPieces]; - } else if (!isa(Piece)) { + } else if (!isa(Piece) && !isArrowPiece(Piece)) { --NumRegularPieces; } } @@ -762,6 +802,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. @@ -1028,6 +1070,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, @@ -1150,3 +1265,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