vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, Szelethus, steakhal, martong,
ASDenysPetrov.
Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This commit adds a very first version of this feature.
It is off by default and has to be turned on by checking the
corresponding box. For this reason, HTML reports still keep
control notes (aka grey bubbles).
Further on, we plan on attaching arrows to events and having all arrows
not related to a currently selected event barely visible. This will
help with reports where control flow goes back and forth (eg in loops).
Right now, it can get pretty crammed with all the arrows.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92639
Files:
clang/include/clang/Analysis/PathDiagnostic.h
clang/lib/Rewrite/HTMLRewrite.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/test/Analysis/html_diagnostics/control-arrows.cpp
Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===================================================================
--- /dev/null
+++ 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: <svg
+// CHECK: <g
+// CHECK-COUNT-9: <path id="arrow{{[0-9]+}}"/>
+// CHECK-NOT: <path id="arrow{{[0-9]+}}"/>
+// CHECK: </g>
+// CHECK-NEXT: </svg>
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK: <div id="Path2" class="msg msgControl"
+// CHECK-SAME: <div class="PathIndex PathIndexControl">2</div>
+// CHECK-SAME: <td>Taking true branch</td>
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ 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<SourceRange> &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<PathDiagnosticControlFlowPiece>(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 @@
<label for="showCounterexample">
Show only relevant lines
</label>
+ <input type="checkbox" name="showArrows"
+ id="showArrows" style="margin-left: 10px" />
+ <label for="showArrows">
+ Show control flow arrows
+ </label>
</form>
)<<<";
@@ -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 @@
<a href="#" onclick="toggleHelp(); return false;">Close</a>
</div>
)<<<";
+
R.InsertTextBefore(SMgr.getLocForStartOfFile(FID), os.str());
}
@@ -595,7 +628,7 @@
<< ColumnNumber
<< " -->\n";
- os << "\n<!-- BUGPATHLENGTH " << path.size() << " -->\n";
+ os << "\n<!-- BUGPATHLENGTH " << getPathSizeWithoutArrows(path) << " -->\n";
// Mark the end of the tags.
os << "\n<!-- BUGMETAEND -->\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<PathDiagnosticNotePiece>(*p);
});
- unsigned PopUpPieceCount = std::count_if(
- path.begin(), path.end(), [](const PathDiagnosticPieceRef &p) {
+ unsigned PopUpPieceCount =
+ llvm::count_if(path, [](const PathDiagnosticPieceRef &p) {
return isa<PathDiagnosticPopUpPiece>(*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<int, int> 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<PathDiagnosticControlFlowPiece>(Piece), NumberOfArrows);
+
} else {
HandlePiece(R, FID, Piece, PopUpRanges, NumRegularPieces,
TotalRegularPieces);
@@ -750,7 +790,7 @@
if (PopUpPieceIndex > 0)
--IndexMap[NumRegularPieces];
- } else if (!isa<PathDiagnosticNotePiece>(Piece)) {
+ } else if (!isa<PathDiagnosticNotePiece>(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"<<<(
+<style type="text/css">
+ svg {
+ position:absolute;
+ top:0;
+ left:0;
+ height:100%;
+ width:100%;
+ pointer-events: none;
+ overflow: visible
+ }
+</style>
+<svg xmlns="http://www.w3.org/2000/svg";>
+ <defs>
+ <marker id="arrowhead" viewBox="0 0 10 10" refX="3" refY="5"
+ markerWidth="4" markerHeight="4" orient="auto" stroke="none" opacity="0.6" fill="blue">
+ <path d="M 0 0 L 10 5 L 0 10 z" />
+ </marker>
+ </defs>
+ <g id="arrows" fill="none" stroke="blue"
+ visibility="hidden" stroke-width="2"
+ stroke-opacity="0.6" marker-end="url(#arrowhead)">
+)<<<";
+
+ for (unsigned Index : llvm::seq(0u, NumberOfArrows)) {
+ OS << " <path id=\"arrow" << Index << "\"/>\n";
+ }
+
+ OS << R"<<<(
+ </g>
+</svg>
+)<<<";
+
+ 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 << "<span id=\"" << ClassName << Index << "\">";
+ 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 @@
</script>
)<<<";
}
+
+StringRef HTMLDiagnostics::generateArrowDrawingJavascript() {
+ return R"<<<(
+<script type='text/javascript'>
+var getAbsoluteBoundingRect = function(element) {
+ let relative = element.getBoundingClientRect();
+ return {
+ left: relative.left + window.scrollX,
+ right: relative.right + window.scrollX,
+ top: relative.top + window.scrollY,
+ bottom: relative.bottom + window.scrollY,
+ height: relative.height,
+ width: relative.width
+ };
+}
+
+var drawArrow = function(index) {
+ // This function is based on the great answer from SO:
+ // https://stackoverflow.com/a/39575674/11582326
+ var start = document.querySelector("#start" + index);
+ var end = document.querySelector("#end" + index);
+ var arrow = document.querySelector("#arrow" + index);
+
+ var startRect = getAbsoluteBoundingRect(start);
+ var endRect = getAbsoluteBoundingRect(end);
+
+ // It is an arrow from a token to itself, no need to visualize it.
+ if (startRect.top == endRect.top &&
+ startRect.left == endRect.left)
+ return;
+
+ // Each arrow is a very simple Bézier curve, with two nodes and
+ // two handles. So, we need to calculate four points in the window:
+ // * start node
+ var posStart = { x: 0, y: 0 };
+ // * end node
+ var posEnd = { x: 0, y: 0 };
+ // * handle for the start node
+ var startHandle = { x: 0, y: 0 };
+ // * handle for the end node
+ var endHandle = { x: 0, y: 0 };
+ // One can visualize it as follows:
+ //
+ // start handle
+ // /
+ // X"""_.-""""X
+ // .' \
+ // / start node
+ // |
+ // |
+ // | end node
+ // \ /
+ // `->X
+ // X-'
+ // \
+ // end handle
+ //
+ // NOTE: (0, 0) is the top left corner of the window.
+
+ // We have 3 similar, but still different scenarios to cover:
+ //
+ // 1. Two tokens on different lines.
+ // -xxx
+ // /
+ // \
+ // -> xxx
+ // In this situation, we draw arrow on the left curving to the left.
+ // 2. Two tokens on the same line, and the destination is on the right.
+ // ____
+ // / \
+ // / V
+ // xxx xxx
+ // In this situation, we draw arrow above curving upwards.
+ // 3. Two tokens on the same line, and the destination is on the right.
+ // xxx xxx
+ // ^ /
+ // \____/
+ // In this situation, we draw arrow below curving downwards.
+ let onDifferentLines = startRect.top <= endRect.top - 5 ||
+ startRect.top >= endRect.top + 5;
+ let leftToRight = startRect.left < endRect.left;
+
+ // NOTE: various magic constants are chosen empirically for
+ // better positioning and look
+ if (onDifferentLines) {
+ // Case #1
+ let topToBottom = startRect.top < endRect.top;
+ posStart.x = startRect.left - 1;
+ // We don't want to start it at the top left corner of the token,
+ // it doesn't feel like this is where the arrow comes from.
+ // For this reason, we start it in the middle of the left side
+ // of the token.
+ posStart.y = startRect.top + startRect.height / 2;
+
+ // End node has arrow head and we give it a bit more space.
+ posEnd.x = endRect.left - 4;
+ posEnd.y = endRect.top;
+
+ // Utility object with x and y offsets for handles.
+ var curvature = {
+ // We want bottom-to-top arrow to curve a bit more, so it doesn't
+ // overlap much with top-to-bottom curves (much more frequent).
+ x: topToBottom ? 15 : 25,
+ y: Math.min((posEnd.y - posStart.y) / 3, 10)
+ }
+
+ // When destination is on the different line, we can make a
+ // curvier arrow because we have space for it.
+ // So, instead of using
+ //
+ // startHandle.x = posStart.x - curvature.x
+ // endHandle.x = posEnd.x - curvature.x
+ //
+ // We use the leftmost of these two values for both handles.
+ startHandle.x = Math.min(posStart.x, posEnd.x) - curvature.x;
+ endHandle.x = startHandle.x;
+
+ // Curving downwards from the start node...
+ startHandle.y = posStart.y + curvature.y;
+ // ... and upwards from the end node.
+ endHandle.y = posEnd.y - curvature.y;
+
+ } else if (leftToRight) {
+ // Case #2
+ // Starting from the top right corner...
+ posStart.x = startRect.right - 1;
+ posStart.y = startRect.top;
+
+ // ...and ending at the top left corner of the end token.
+ posEnd.x = endRect.left + 1;
+ posEnd.y = endRect.top - 1;
+
+ // Utility object with x and y offsets for handles.
+ var curvature = {
+ x: Math.min((posEnd.x - posStart.x) / 3, 15),
+ y: 5
+ }
+
+ // Curving to the right...
+ startHandle.x = posStart.x + curvature.x;
+ // ... and upwards from the start node.
+ startHandle.y = posStart.y - curvature.y;
+
+ // And to the left...
+ endHandle.x = posEnd.x - curvature.x;
+ // ... and upwards from the end node.
+ endHandle.y = posEnd.y - curvature.y;
+
+ } else {
+ // Case #3
+ // Starting from the bottom right corner...
+ posStart.x = startRect.right;
+ posStart.y = startRect.bottom;
+
+ // ...and ending also at the bottom right corner, but of the end token.
+ posEnd.x = endRect.right - 1;
+ posEnd.y = endRect.bottom + 1;
+
+ // Utility object with x and y offsets for handles.
+ var curvature = {
+ x: Math.min((posStart.x - posEnd.x) / 3, 15),
+ y: 5
+ }
+
+ // Curving to the left...
+ startHandle.x = posStart.x - curvature.x;
+ // ... and downwards from the start node.
+ startHandle.y = posStart.y + curvature.y;
+
+ // And to the right...
+ endHandle.x = posEnd.x + curvature.x;
+ // ... and downwards from the end node.
+ endHandle.y = posEnd.y + curvature.y;
+ }
+
+ // Put it all together into a path.
+ // More information on the format:
+ // https://developer.mozilla.org/en-US/docs/Web/SVG/Tutorial/Paths
+ var pathStr = "M" + posStart.x + "," + posStart.y + " " +
+ "C" + startHandle.x + "," + startHandle.y + " " +
+ endHandle.x + "," + endHandle.y + " " +
+ posEnd.x + "," + posEnd.y;
+
+ arrow.setAttribute("d", pathStr);
+};
+
+var drawArrows = function() {
+ let numOfArrows = document.querySelectorAll("path[id^=arrow]").length;
+ for (var i = 0; i < numOfArrows; ++i) {
+ drawArrow(i);
+ }
+}
+
+var toggleArrows = function(event) {
+ let arrows = document.querySelector("#arrows");
+ if (event.target.checked) {
+ arrows.setAttribute("visibility", "visible");
+ } else {
+ arrows.setAttribute("visibility", "hidden");
+ }
+}
+
+window.addEventListener("resize", drawArrows);
+document.addEventListener("DOMContentLoaded", function() {
+ // Whenever we show invocation, locations change, i.e. we
+ // need to redraw arrows.
+ document
+ .querySelector('input[id="showinvocation"]')
+ .addEventListener("click", drawArrows);
+ // Hiding irrelevant lines also should cause arrow rerender.
+ document
+ .querySelector('input[name="showCounterexample"]')
+ .addEventListener("change", drawArrows);
+ document
+ .querySelector('input[name="showArrows"]')
+ .addEventListener("change", toggleArrows);
+ drawArrows();
+});
+</script>
+ )<<<";
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ 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<BlockEdge>()) {
- 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<PathDiagnosticEventPiece>(
- 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<CompoundStmt>(Body)) {
addEdgeToPath(C.getActivePath(), PrevLoc,
@@ -1300,10 +1308,14 @@
auto PE = std::make_shared<PathDiagnosticEventPiece>(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<BreakStmt>(Term) || isa<ContinueStmt>(Term) ||
- isa<GotoStmt>(Term)) {
+ isa<GotoStmt>(Term)) {
PathDiagnosticLocation L(Term, SM, C.getCurrLocationContext());
addEdgeToPath(C.getActivePath(), PrevLoc, L);
}
Index: clang/lib/Rewrite/HTMLRewrite.cpp
===================================================================
--- clang/lib/Rewrite/HTMLRewrite.cpp
+++ 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 }
Index: clang/include/clang/Analysis/PathDiagnostic.h
===================================================================
--- clang/include/clang/Analysis/PathDiagnostic.h
+++ 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; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
