Skip to content

Commit a5ddd3c

Browse files
author
George Karpenkov
committed
[analyzer] support a mode to only show relevant lines in HTML diagnostics
HTML diagnostics can be an overwhelming blob of pages of code. This patch adds a checkbox which filters this list down to only the lines *relevant* to the counterexample by e.g. skipping branches which analyzer has assumed to be infeasible at a time. The resulting amount of output is much smaller, and often fits on one screen, and also provides a much more readable diagnostics. Differential Revision: https://reviews.llvm.org/D41378 llvm-svn: 322612
1 parent 8321ad9 commit a5ddd3c

File tree

12 files changed

+305
-22
lines changed

12 files changed

+305
-22
lines changed

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#include <deque>
2424
#include <iterator>
2525
#include <list>
26+
#include <map>
27+
#include <set>
2628
#include <string>
2729
#include <vector>
2830

@@ -733,6 +735,9 @@ class PathDiagnosticNotePiece: public PathDiagnosticSpotPiece {
733735
void Profile(llvm::FoldingSetNodeID &ID) const override;
734736
};
735737

738+
/// File IDs mapped to sets of line numbers.
739+
typedef std::map<unsigned, std::set<unsigned>> FilesToLineNumsMap;
740+
736741
/// PathDiagnostic - PathDiagnostic objects represent a single path-sensitive
737742
/// diagnostic. It represents an ordered-collection of PathDiagnosticPieces,
738743
/// each which represent the pieces of the path.
@@ -756,12 +761,16 @@ class PathDiagnostic : public llvm::FoldingSetNode {
756761
PathDiagnosticLocation UniqueingLoc;
757762
const Decl *UniqueingDecl;
758763

764+
/// Lines executed in the path.
765+
std::unique_ptr<FilesToLineNumsMap> ExecutedLines;
766+
759767
PathDiagnostic() = delete;
760768
public:
761769
PathDiagnostic(StringRef CheckName, const Decl *DeclWithIssue,
762770
StringRef bugtype, StringRef verboseDesc, StringRef shortDesc,
763771
StringRef category, PathDiagnosticLocation LocationToUnique,
764-
const Decl *DeclToUnique);
772+
const Decl *DeclToUnique,
773+
std::unique_ptr<FilesToLineNumsMap> ExecutedLines);
765774

766775
~PathDiagnostic();
767776

@@ -830,6 +839,12 @@ class PathDiagnostic : public llvm::FoldingSetNode {
830839
meta_iterator meta_end() const { return OtherDesc.end(); }
831840
void addMeta(StringRef s) { OtherDesc.push_back(s); }
832841

842+
typedef FilesToLineNumsMap::const_iterator filesmap_iterator;
843+
filesmap_iterator executedLines_begin() const {
844+
return ExecutedLines->begin();
845+
}
846+
filesmap_iterator executedLines_end() const { return ExecutedLines->end(); }
847+
833848
PathDiagnosticLocation getLocation() const {
834849
assert(Loc.isValid() && "No report location set yet!");
835850
return Loc;

clang/lib/Rewrite/HTMLRewrite.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,9 @@ static void AddLineNumber(RewriteBuffer &RB, unsigned LineNo,
210210
SmallString<256> Str;
211211
llvm::raw_svector_ostream OS(Str);
212212

213-
OS << "<tr><td class=\"num\" id=\"LN"
214-
<< LineNo << "\">"
215-
<< LineNo << "</td><td class=\"line\">";
213+
OS << "<tr class=\"codeline\" data-linenumber=\"" << LineNo << "\">"
214+
<< "<td class=\"num\" id=\"LN" << LineNo << "\">" << LineNo
215+
<< "</td><td class=\"line\">";
216216

217217
if (B == E) { // Handle empty lines.
218218
OS << " </td></tr>";
@@ -263,7 +263,10 @@ void html::AddLineNumbers(Rewriter& R, FileID FID) {
263263
}
264264

265265
// Add one big table tag that surrounds all of the code.
266-
RB.InsertTextBefore(0, "<table class=\"code\">\n");
266+
std::string s;
267+
llvm::raw_string_ostream os(s);
268+
os << "<table class=\"code\" data-fileid=\"" << FID.getHashValue() << "\">\n";
269+
RB.InsertTextBefore(0, os.str());
267270
RB.InsertTextAfter(FileEnd - FileBeg, "</table>");
268271
}
269272

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3509,6 +3509,66 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
35093509
}
35103510
}
35113511

3512+
/// Insert all lines participating in the function signature \p Signature
3513+
/// into \p ExecutedLines.
3514+
static void populateExecutedLinesWithFunctionSignature(
3515+
const Decl *Signature, SourceManager &SM,
3516+
std::unique_ptr<FilesToLineNumsMap> &ExecutedLines) {
3517+
3518+
SourceRange SignatureSourceRange;
3519+
const Stmt* Body = Signature->getBody();
3520+
if (auto FD = dyn_cast<FunctionDecl>(Signature)) {
3521+
SignatureSourceRange = FD->getSourceRange();
3522+
} else if (auto OD = dyn_cast<ObjCMethodDecl>(Signature)) {
3523+
SignatureSourceRange = OD->getSourceRange();
3524+
} else {
3525+
return;
3526+
}
3527+
SourceLocation Start = SignatureSourceRange.getBegin();
3528+
SourceLocation End = Body ? Body->getSourceRange().getBegin()
3529+
: SignatureSourceRange.getEnd();
3530+
unsigned StartLine = SM.getExpansionLineNumber(Start);
3531+
unsigned EndLine = SM.getExpansionLineNumber(End);
3532+
3533+
FileID FID = SM.getFileID(SM.getExpansionLoc(Start));
3534+
for (unsigned Line = StartLine; Line <= EndLine; Line++)
3535+
ExecutedLines->operator[](FID.getHashValue()).insert(Line);
3536+
}
3537+
3538+
/// \return all executed lines including function signatures on the path
3539+
/// starting from \p N.
3540+
static std::unique_ptr<FilesToLineNumsMap>
3541+
findExecutedLines(SourceManager &SM, const ExplodedNode *N) {
3542+
auto ExecutedLines = llvm::make_unique<FilesToLineNumsMap>();
3543+
3544+
while (N) {
3545+
if (N->getFirstPred() == nullptr) {
3546+
3547+
// First node: show signature of the entrance point.
3548+
const Decl *D = N->getLocationContext()->getDecl();
3549+
populateExecutedLinesWithFunctionSignature(D, SM, ExecutedLines);
3550+
3551+
} else if (auto CE = N->getLocationAs<CallEnter>()) {
3552+
3553+
// Inlined function: show signature.
3554+
const Decl* D = CE->getCalleeContext()->getDecl();
3555+
populateExecutedLinesWithFunctionSignature(D, SM, ExecutedLines);
3556+
3557+
} else if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) {
3558+
3559+
// Otherwise: show lines associated with the processed statement.
3560+
SourceLocation Loc = S->getSourceRange().getBegin();
3561+
SourceLocation ExpansionLoc = SM.getExpansionLoc(Loc);
3562+
FileID FID = SM.getFileID(ExpansionLoc);
3563+
unsigned LineNo = SM.getExpansionLineNumber(ExpansionLoc);
3564+
ExecutedLines->operator[](FID.getHashValue()).insert(LineNo);
3565+
}
3566+
3567+
N = N->getFirstPred();
3568+
}
3569+
return ExecutedLines;
3570+
}
3571+
35123572
void BugReporter::FlushReport(BugReport *exampleReport,
35133573
PathDiagnosticConsumer &PD,
35143574
ArrayRef<BugReport*> bugReports) {
@@ -3517,13 +3577,13 @@ void BugReporter::FlushReport(BugReport *exampleReport,
35173577
// Probably doesn't make a difference in practice.
35183578
BugType& BT = exampleReport->getBugType();
35193579

3520-
std::unique_ptr<PathDiagnostic> D(new PathDiagnostic(
3580+
auto D = llvm::make_unique<PathDiagnostic>(
35213581
exampleReport->getBugType().getCheckName(),
35223582
exampleReport->getDeclWithIssue(), exampleReport->getBugType().getName(),
35233583
exampleReport->getDescription(),
35243584
exampleReport->getShortDescription(/*Fallback=*/false), BT.getCategory(),
3525-
exampleReport->getUniqueingLocation(),
3526-
exampleReport->getUniqueingDecl()));
3585+
exampleReport->getUniqueingLocation(), exampleReport->getUniqueingDecl(),
3586+
findExecutedLines(getSourceManager(), exampleReport->getErrorNode()));
35273587

35283588
if (exampleReport->isPathSensitive()) {
35293589
// Generate the full path diagnostic, using the generation scheme

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ class HTMLDiagnostics : public PathDiagnosticConsumer {
9494

9595
/// \return Javascript for navigating the HTML report using j/k keys.
9696
std::string generateKeyboardNavigationJavascript();
97+
98+
private:
99+
/// \return JavaScript for an option to only show relevant lines.
100+
std::string showRelevantLinesJavascript(const PathDiagnostic &D);
101+
102+
/// \return Executed lines from \p D in JSON format.
103+
std::string serializeExecutedLines(const PathDiagnostic &D);
97104
};
98105

99106
} // end anonymous namespace
@@ -343,6 +350,10 @@ void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
343350
R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
344351
generateKeyboardNavigationJavascript());
345352

353+
// Checkbox and javascript for filtering the output to the counterexample.
354+
R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
355+
showRelevantLinesJavascript(D));
356+
346357
// Add the name of the file as an <h1> tag.
347358
{
348359
std::string s;
@@ -450,6 +461,94 @@ void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
450461
html::AddHeaderFooterInternalBuiltinCSS(R, FID, Entry->getName());
451462
}
452463

464+
std::string
465+
HTMLDiagnostics::showRelevantLinesJavascript(const PathDiagnostic &D) {
466+
std::string s;
467+
llvm::raw_string_ostream os(s);
468+
os << "<script type='text/javascript'>\n";
469+
os << serializeExecutedLines(D);
470+
os << R"<<<(
471+
472+
var filterCounterexample = function (hide) {
473+
var tables = document.getElementsByClassName("code");
474+
for (var t=0; t<tables.length; t++) {
475+
var table = tables[t];
476+
var file_id = table.getAttribute("data-fileid");
477+
var lines_in_fid = relevant_lines[file_id];
478+
if (!lines_in_fid) {
479+
lines_in_fid = {};
480+
}
481+
var lines = table.getElementsByClassName("codeline");
482+
for (var i=0; i<lines.length; i++) {
483+
var el = lines[i];
484+
var lineNo = el.getAttribute("data-linenumber");
485+
if (!lines_in_fid[lineNo]) {
486+
if (hide) {
487+
el.setAttribute("hidden", "");
488+
} else {
489+
el.removeAttribute("hidden");
490+
}
491+
}
492+
}
493+
}
494+
}
495+
496+
window.addEventListener("keydown", function (event) {
497+
if (event.defaultPrevented) {
498+
return;
499+
}
500+
if (event.key == "S") {
501+
var checked = document.getElementsByName("showCounterexample")[0].checked;
502+
filterCounterexample(!checked);
503+
document.getElementsByName("showCounterexample")[0].checked = !checked;
504+
} else {
505+
return;
506+
}
507+
event.preventDefault();
508+
}, true);
509+
510+
document.addEventListener("DOMContentLoaded", function() {
511+
document.querySelector('input[name="showCounterexample"]').onchange=
512+
function (event) {
513+
filterCounterexample(this.checked);
514+
};
515+
});
516+
</script>
517+
518+
<form>
519+
<input type="checkbox" name="showCounterexample" />
520+
<label for="showCounterexample">
521+
Show only relevant lines
522+
</label>
523+
</form>
524+
)<<<";
525+
526+
return os.str();
527+
}
528+
529+
std::string HTMLDiagnostics::serializeExecutedLines(const PathDiagnostic &D) {
530+
std::string s;
531+
llvm::raw_string_ostream os(s);
532+
os << "var relevant_lines = {";
533+
for (auto I = D.executedLines_begin(),
534+
E = D.executedLines_end(); I != E; ++I) {
535+
if (I != D.executedLines_begin())
536+
os << ", ";
537+
538+
os << "\"" << I->first << "\": {";
539+
for (unsigned LineNo : I->second) {
540+
if (LineNo != *(I->second.begin()))
541+
os << ", ";
542+
543+
os << "\"" << LineNo << "\": 1";
544+
}
545+
os << "}";
546+
}
547+
548+
os << "};";
549+
return os.str();
550+
}
551+
453552
void HTMLDiagnostics::RewriteFile(Rewriter &R, const SourceManager& SMgr,
454553
const PathPieces& path, FileID FID) {
455554
// Process the path.

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,20 +98,18 @@ void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
9898

9999
PathDiagnostic::~PathDiagnostic() {}
100100

101-
PathDiagnostic::PathDiagnostic(StringRef CheckName, const Decl *declWithIssue,
102-
StringRef bugtype, StringRef verboseDesc,
103-
StringRef shortDesc, StringRef category,
104-
PathDiagnosticLocation LocationToUnique,
105-
const Decl *DeclToUnique)
106-
: CheckName(CheckName),
107-
DeclWithIssue(declWithIssue),
108-
BugType(StripTrailingDots(bugtype)),
109-
VerboseDesc(StripTrailingDots(verboseDesc)),
110-
ShortDesc(StripTrailingDots(shortDesc)),
111-
Category(StripTrailingDots(category)),
112-
UniqueingLoc(LocationToUnique),
113-
UniqueingDecl(DeclToUnique),
114-
path(pathImpl) {}
101+
PathDiagnostic::PathDiagnostic(
102+
StringRef CheckName, const Decl *declWithIssue, StringRef bugtype,
103+
StringRef verboseDesc, StringRef shortDesc, StringRef category,
104+
PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique,
105+
std::unique_ptr<FilesToLineNumsMap> ExecutedLines)
106+
: CheckName(CheckName), DeclWithIssue(declWithIssue),
107+
BugType(StripTrailingDots(bugtype)),
108+
VerboseDesc(StripTrailingDots(verboseDesc)),
109+
ShortDesc(StripTrailingDots(shortDesc)),
110+
Category(StripTrailingDots(category)), UniqueingLoc(LocationToUnique),
111+
UniqueingDecl(DeclToUnique), ExecutedLines(std::move(ExecutedLines)),
112+
path(pathImpl) {}
115113

116114
static PathDiagnosticCallPiece *
117115
getFirstStackedCallToHeaderFile(PathDiagnosticCallPiece *CP,
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#define deref(X) (*X)
2+
3+
char helper(
4+
char *out,
5+
int doDereference) {
6+
if (doDereference) {
7+
return deref(out);
8+
} else {
9+
return 'x';
10+
}
11+
return 'c';
12+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#define deref(X) (*X)
2+
3+
int f(int coin) {
4+
if (coin) {
5+
int *x = 0;
6+
return deref(x);
7+
} else {
8+
return 0;
9+
}
10+
}
11+
12+
// RUN: rm -rf %t.output
13+
// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -analyzer-output html -o %t.output %s
14+
// RUN: cat %t.output/* | FileCheck %s --match-full-lines
15+
// CHECK: var relevant_lines = {"1": {"3": 1, "4": 1, "5": 1, "6": 1}};
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#include "header.h"
2+
3+
int f(int coin) {
4+
char *p = 0;
5+
if (coin) {
6+
return helper(p, coin);
7+
}
8+
return 0;
9+
}
10+
11+
// RUN: rm -rf %t.output
12+
// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -analyzer-output html -o %t.output %s
13+
// RUN: cat %t.output/* | FileCheck %s --match-full-lines
14+
// CHECK: var relevant_lines = {"1": {"3": 1, "4": 1, "5": 1, "6": 1}, "3": {"3": 1, "4": 1, "5": 1, "6": 1, "7": 1}};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
int f(
2+
int coin,
3+
int paramA,
4+
int paramB) {
5+
if (coin) {
6+
int *x = 0;
7+
return *x;
8+
} else {
9+
return 0;
10+
}
11+
}
12+
13+
// RUN: rm -rf %t.output
14+
// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -analyzer-output html -o %t.output %s
15+
// RUN: cat %t.output/* | FileCheck %s --match-full-lines
16+
// CHECK: var relevant_lines = {"1": {"1": 1, "2": 1, "3": 1, "4": 1, "5": 1, "6": 1, "7": 1}};
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
@interface I
2+
- (int)func;
3+
@end
4+
5+
@implementation I
6+
- (int)func:(int *)param {
7+
return *param;
8+
}
9+
@end
10+
11+
void foo(I *i) {
12+
int *x = 0;
13+
[i func:x];
14+
}
15+
16+
// RUN: rm -rf %t.output
17+
// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -analyzer-output html -o %t.output -Wno-objc-root-class %s
18+
// RUN: cat %t.output/* | FileCheck %s
19+
// CHECK: var relevant_lines = {"1": {"6": 1, "7": 1, "11": 1, "12": 1, "13": 1}};

0 commit comments

Comments
 (0)