NoQ updated this revision to Diff 72480.
NoQ added a comment.
This diff is a part of the original diff that contains core changes without
checker changes (in particular, without tests). No changes were made, no
comments addressed, just removed checker stuff. Uploading it separately so that
it was easier to track changes in phabricator.
Actually improved diff coming soon.
Separate reviews for the checkers coming soon.
https://reviews.llvm.org/D24278
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
lib/Rewrite/HTMLRewrite.cpp
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -113,13 +113,27 @@
Diag.Report(WarnLoc, WarnID) << PD->getShortDescription()
<< PD->path.back()->getRanges();
+ // First, add extra notes, even if paths should not be included.
+ for (PathPieces::const_iterator PI = PD->path.begin(),
+ PE = PD->path.end(); PI != PE; ++PI) {
+ if (!isa<PathDiagnosticExtraNotePiece>(PI->get()))
+ continue;
+
+ SourceLocation NoteLoc = (*PI)->getLocation().asLocation();
+ Diag.Report(NoteLoc, NoteID) << (*PI)->getString()
+ << (*PI)->getRanges();
+ }
+
if (!IncludePath)
continue;
+ // Then, add the path notes if necessary.
PathPieces FlatPath = PD->path.flatten(/*ShouldFlattenMacros=*/true);
for (PathPieces::const_iterator PI = FlatPath.begin(),
- PE = FlatPath.end();
- PI != PE; ++PI) {
+ PE = FlatPath.end(); PI != PE; ++PI) {
+ if (isa<PathDiagnosticExtraNotePiece>(PI->get()))
+ continue;
+
SourceLocation NoteLoc = (*PI)->getLocation().asLocation();
Diag.Report(NoteLoc, NoteID) << (*PI)->getString()
<< (*PI)->getRanges();
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -281,6 +281,9 @@
ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, SM, LangOpts,
indent, depth);
break;
+ case PathDiagnosticPiece::ExtraNote:
+ // FIXME: Extend the plist format to support those.
+ break;
}
}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -60,6 +60,7 @@
PathDiagnosticCallPiece::~PathDiagnosticCallPiece() {}
PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() {}
PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() {}
+PathDiagnosticExtraNotePiece::~PathDiagnosticExtraNotePiece() {}
void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
bool ShouldFlattenMacros) const {
@@ -95,6 +96,7 @@
}
case PathDiagnosticPiece::Event:
case PathDiagnosticPiece::ControlFlow:
+ case PathDiagnosticPiece::ExtraNote:
Current.push_back(Piece);
break;
}
@@ -342,15 +344,16 @@
}
switch (X.getKind()) {
- case clang::ento::PathDiagnosticPiece::ControlFlow:
+ case PathDiagnosticPiece::ControlFlow:
return compareControlFlow(cast<PathDiagnosticControlFlowPiece>(X),
cast<PathDiagnosticControlFlowPiece>(Y));
- case clang::ento::PathDiagnosticPiece::Event:
+ case PathDiagnosticPiece::Event:
+ case PathDiagnosticPiece::ExtraNote:
return None;
- case clang::ento::PathDiagnosticPiece::Macro:
+ case PathDiagnosticPiece::Macro:
return compareMacro(cast<PathDiagnosticMacroPiece>(X),
cast<PathDiagnosticMacroPiece>(Y));
- case clang::ento::PathDiagnosticPiece::Call:
+ case PathDiagnosticPiece::Call:
return compareCall(cast<PathDiagnosticCallPiece>(X),
cast<PathDiagnosticCallPiece>(Y));
}
@@ -1098,6 +1101,10 @@
ID.Add(**I);
}
+void PathDiagnosticExtraNotePiece::Profile(llvm::FoldingSetNodeID &ID) const {
+ PathDiagnosticSpotPiece::Profile(ID);
+}
+
void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
ID.Add(getLocation());
ID.AddString(BugType);
Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -152,13 +152,29 @@
}
// Process the path.
- unsigned n = path.size();
- unsigned max = n;
-
- for (PathPieces::const_reverse_iterator I = path.rbegin(),
- E = path.rend();
- I != E; ++I, --n)
- HandlePiece(R, FID, **I, n, max);
+ // Maintain the counts of extra note pieces separately.
+ unsigned TotalPieces = path.size();
+ unsigned TotalExtraPieces =
+ std::count_if(path.begin(), path.end(),
+ [](const IntrusiveRefCntPtr<PathDiagnosticPiece> &p) {
+ return isa<PathDiagnosticExtraNotePiece>(p.get());
+ });
+
+ unsigned TotalRegularPieces = TotalPieces - TotalExtraPieces;
+ unsigned NumRegularPieces = TotalRegularPieces;
+ unsigned NumExtraPieces = TotalExtraPieces;
+
+ for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
+ if (isa<PathDiagnosticExtraNotePiece>(I->get())) {
+ // Will also make a second pass through those later below,
+ // when the header text is ready.
+ HandlePiece(R, FID, **I, NumExtraPieces, TotalExtraPieces);
+ --NumExtraPieces;
+ } else {
+ HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces);
+ --NumRegularPieces;
+ }
+ }
// Add line numbers, header, footer, etc.
@@ -192,24 +208,38 @@
int ColumnNumber = path.back()->getLocation().asLocation().getExpansionColumnNumber();
// Add the name of the file as an <h1> tag.
-
{
std::string s;
llvm::raw_string_ostream os(s);
os << "<!-- REPORTHEADER -->\n"
- << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
+ << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
"<tr><td class=\"rowname\">File:</td><td>"
- << html::EscapeText(DirName)
- << html::EscapeText(Entry->getName())
- << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
- "<a href=\"#EndPath\">line "
- << LineNumber
- << ", column "
- << ColumnNumber
- << "</a></td></tr>\n"
- "<tr><td class=\"rowname\">Description:</td><td>"
- << D.getVerboseDescription() << "</td></tr>\n";
+ << html::EscapeText(DirName)
+ << html::EscapeText(Entry->getName())
+ << "</td></tr>\n<tr><td class=\"rowname\">Warning:</td><td>"
+ "<a href=\"#EndPath\">line "
+ << LineNumber
+ << ", column "
+ << ColumnNumber
+ << "</a><br />"
+ << D.getVerboseDescription() << "</td></tr>\n";
+
+ // The navigation across the extra notes pieces.
+ unsigned NumExtraPieces = 0;
+ for (auto I = path.begin(), E = path.end(); I != E; ++I) {
+ if (const auto *P = dyn_cast<PathDiagnosticExtraNotePiece>(I->get())) {
+ int LineNumber =
+ P->getLocation().asLocation().getExpansionLineNumber();
+ int ColumnNumber =
+ P->getLocation().asLocation().getExpansionColumnNumber();
+ os << "<tr><td class=\"rowname\">Note:</td><td>"
+ << "<a href=\"#ExtraNote" << NumExtraPieces << "\">line "
+ << LineNumber << ", column " << ColumnNumber << "</a><br />"
+ << P->getString() << "</td></tr>";
+ ++NumExtraPieces;
+ }
+ }
// Output any other meta data.
@@ -385,21 +415,30 @@
// Create the html for the message.
const char *Kind = nullptr;
+ bool IsExtraNote = false;
+ bool SuppressIndex = (max == 1);
switch (P.getKind()) {
case PathDiagnosticPiece::Call:
- llvm_unreachable("Calls should already be handled");
+ llvm_unreachable("Calls and extra notes should already be handled");
case PathDiagnosticPiece::Event: Kind = "Event"; break;
case PathDiagnosticPiece::ControlFlow: Kind = "Control"; break;
// Setting Kind to "Control" is intentional.
case PathDiagnosticPiece::Macro: Kind = "Control"; break;
+ case PathDiagnosticPiece::ExtraNote:
+ Kind = "ExtraNote";
+ IsExtraNote = true;
+ SuppressIndex = true;
+ break;
}
std::string sbuf;
llvm::raw_string_ostream os(sbuf);
os << "\n<tr><td class=\"num\"></td><td class=\"line\"><div id=\"";
- if (num == max)
+ if (IsExtraNote)
+ os << "ExtraNote" << num;
+ else if (num == max)
os << "EndPath";
else
os << "Path" << num;
@@ -461,7 +500,7 @@
os << "\">";
- if (max > 1) {
+ if (!SuppressIndex) {
os << "<table class=\"msgT\"><tr><td valign=\"top\">";
os << "<div class=\"PathIndex";
if (Kind) os << " PathIndex" << Kind;
@@ -501,7 +540,7 @@
os << "':\n";
- if (max > 1) {
+ if (!SuppressIndex) {
os << "</td>";
if (num < max) {
os << "<td><div class=\"PathNav\"><a href=\"#";
@@ -523,7 +562,7 @@
else {
os << html::EscapeText(P.getString());
- if (max > 1) {
+ if (!SuppressIndex) {
os << "</td>";
if (num < max) {
os << "<td><div class=\"PathNav\"><a href=\"#";
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -112,15 +112,15 @@
path.pop_front();
switch (piece->getKind()) {
- case clang::ento::PathDiagnosticPiece::Call:
+ case PathDiagnosticPiece::Call:
removeRedundantMsgs(cast<PathDiagnosticCallPiece>(piece)->path);
break;
- case clang::ento::PathDiagnosticPiece::Macro:
+ case PathDiagnosticPiece::Macro:
removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(piece)->subPieces);
break;
- case clang::ento::PathDiagnosticPiece::ControlFlow:
+ case PathDiagnosticPiece::ControlFlow:
break;
- case clang::ento::PathDiagnosticPiece::Event: {
+ case PathDiagnosticPiece::Event: {
if (i == N-1)
break;
@@ -140,6 +140,8 @@
}
break;
}
+ case PathDiagnosticPiece::ExtraNote:
+ break;
}
path.push_back(piece);
}
@@ -197,6 +199,9 @@
}
case PathDiagnosticPiece::ControlFlow:
break;
+
+ case PathDiagnosticPiece::ExtraNote:
+ break;
}
pieces.push_back(piece);
@@ -3403,25 +3408,28 @@
exampleReport->getUniqueingLocation(),
exampleReport->getUniqueingDecl()));
- MaxBugClassSize = std::max(bugReports.size(),
- static_cast<size_t>(MaxBugClassSize));
+ if (exampleReport->isPathSensitive()) {
+ // Generate the full path diagnostic, using the generation scheme
+ // specified by the PathDiagnosticConsumer. Note that we have to generate
+ // path diagnostics even for consumers which do not support paths, because
+ // the BugReporterVisitors may mark this bug as a false positive.
+ assert(!bugReports.empty());
+
+ MaxBugClassSize =
+ std::max(bugReports.size(), static_cast<size_t>(MaxBugClassSize));
- // Generate the full path diagnostic, using the generation scheme
- // specified by the PathDiagnosticConsumer. Note that we have to generate
- // path diagnostics even for consumers which do not support paths, because
- // the BugReporterVisitors may mark this bug as a false positive.
- if (!bugReports.empty())
if (!generatePathDiagnostic(*D.get(), PD, bugReports))
return;
- MaxValidBugClassSize = std::max(bugReports.size(),
- static_cast<size_t>(MaxValidBugClassSize));
+ MaxValidBugClassSize =
+ std::max(bugReports.size(), static_cast<size_t>(MaxValidBugClassSize));
- // Examine the report and see if the last piece is in a header. Reset the
- // report location to the last piece in the main source file.
- AnalyzerOptions& Opts = getAnalyzerOptions();
- if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
- D->resetDiagnosticLocationToMainFile();
+ // Examine the report and see if the last piece is in a header. Reset the
+ // report location to the last piece in the main source file.
+ AnalyzerOptions &Opts = getAnalyzerOptions();
+ if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
+ D->resetDiagnosticLocationToMainFile();
+ }
// If the path is empty, generate a single step path with the location
// of the issue.
@@ -3434,6 +3442,27 @@
D->setEndOfPath(std::move(piece));
}
+ PathPieces &Pieces = D->getMutablePieces();
+ if (getAnalyzerOptions().shouldDisplayExtraNotesAsEvents()) {
+ // For path diagnostic consumers that don't support extra notes,
+ // we may optionally convert those to path notes.
+ for (auto I = exampleReport->getExtraNotes().rbegin(),
+ E = exampleReport->getExtraNotes().rend(); I != E; ++I) {
+ PathDiagnosticExtraNotePiece *Piece = I->get();
+ PathDiagnosticEventPiece *ConvertedPiece =
+ new PathDiagnosticEventPiece(Piece->getLocation(),
+ Piece->getString());
+ for (const auto &R: Piece->getRanges())
+ ConvertedPiece->addRange(R);
+
+ Pieces.push_front(ConvertedPiece);
+ }
+ } else {
+ for (auto I = exampleReport->getExtraNotes().rbegin(),
+ E = exampleReport->getExtraNotes().rend(); I != E; ++I)
+ Pieces.push_front(*I);
+ }
+
// Get the meta data.
const BugReport::ExtraTextList &Meta = exampleReport->getExtraText();
for (BugReport::ExtraTextList::const_iterator i = Meta.begin(),
@@ -3518,6 +3547,13 @@
// FIXME: Print which macro is being invoked.
}
+LLVM_DUMP_METHOD void PathDiagnosticExtraNotePiece::dump() const {
+ llvm::errs() << "EXTRANOTE\n--------------\n";
+ llvm::errs() << getString() << "\n";
+ llvm::errs() << " ---- at ----\n";
+ getLocation().dump();
+}
+
LLVM_DUMP_METHOD void PathDiagnosticLocation::dump() const {
if (!isValid()) {
llvm::errs() << "<INVALID>\n";
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -344,3 +344,11 @@
WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
return WidenLoops.getValue();
}
+
+
+bool AnalyzerOptions::shouldDisplayExtraNotesAsEvents() {
+ if (!DisplayExtraNotesAsEvents.hasValue())
+ DisplayExtraNotesAsEvents =
+ getBooleanOption("extra-notes-as-events", /*Default=*/false);
+ return DisplayExtraNotesAsEvents.getValue();
+}
Index: lib/Rewrite/HTMLRewrite.cpp
===================================================================
--- lib/Rewrite/HTMLRewrite.cpp
+++ lib/Rewrite/HTMLRewrite.cpp
@@ -321,6 +321,7 @@
" .msgT { padding:0x; spacing:0x }\n"
" .msgEvent { background-color:#fff8b4; color:#000000 }\n"
" .msgControl { background-color:#bbbbbb; color:#000000 }\n"
+ " .msgExtraNote { background-color:#ddeeff; color:#000000 }\n"
" .mrange { background-color:#dfddf3 }\n"
" .mrange { border-bottom:1px solid #6F9DBE }\n"
" .PathIndex { font-weight: bold; padding:0px 5px; "
@@ -339,8 +340,12 @@
" border-collapse: collapse; border-spacing: 0px;\n"
" }\n"
" td.rowname {\n"
- " text-align:right; font-weight:bold; color:#444444;\n"
- " padding-right:2ex; }\n"
+ " text-align: right;\n"
+ " vertical-align: top;\n"
+ " font-weight: bold;\n"
+ " color:#444444;\n"
+ " padding-right:2ex;\n"
+ " }\n"
"</style>\n</head>\n<body>";
// Generate header
Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===================================================================
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -336,7 +336,7 @@
class PathDiagnosticPiece : public RefCountedBaseVPTR {
public:
- enum Kind { ControlFlow, Event, Macro, Call };
+ enum Kind { ControlFlow, Event, Macro, Call, ExtraNote };
enum DisplayHint { Above, Below };
private:
@@ -452,7 +452,8 @@
void Profile(llvm::FoldingSetNodeID &ID) const override;
static bool classof(const PathDiagnosticPiece *P) {
- return P->getKind() == Event || P->getKind() == Macro;
+ return P->getKind() == Event || P->getKind() == Macro ||
+ P->getKind() == ExtraNote;
}
};
@@ -710,6 +711,23 @@
void Profile(llvm::FoldingSetNodeID &ID) const override;
};
+class PathDiagnosticExtraNotePiece: public PathDiagnosticSpotPiece {
+public:
+ PathDiagnosticExtraNotePiece(const PathDiagnosticLocation &Pos, StringRef S,
+ bool AddPosRange = true)
+ : PathDiagnosticSpotPiece(Pos, S, ExtraNote, AddPosRange) {}
+
+ ~PathDiagnosticExtraNotePiece() override;
+
+ static inline bool classof(const PathDiagnosticPiece *P) {
+ return P->getKind() == ExtraNote;
+ }
+
+ void dump() const override;
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override;
+};
+
/// PathDiagnostic - PathDiagnostic objects represent a single path-sensitive
/// diagnostic. It represents an ordered-collection of PathDiagnosticPieces,
/// each which represent the pieces of the path.
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -66,6 +66,8 @@
typedef SmallVector<std::unique_ptr<BugReporterVisitor>, 8> VisitorList;
typedef VisitorList::iterator visitor_iterator;
typedef SmallVector<StringRef, 2> ExtraTextList;
+ typedef SmallVector<llvm::IntrusiveRefCntPtr<PathDiagnosticExtraNotePiece>, 4>
+ ExtraNoteList;
protected:
friend class BugReporter;
@@ -82,7 +84,8 @@
const ExplodedNode *ErrorNode;
SmallVector<SourceRange, 4> Ranges;
ExtraTextList ExtraText;
-
+ ExtraNoteList ExtraNotes;
+
typedef llvm::DenseSet<SymbolRef> Symbols;
typedef llvm::DenseSet<const MemRegion *> Regions;
@@ -177,6 +180,18 @@
const BugType& getBugType() const { return BT; }
BugType& getBugType() { return BT; }
+ /// \brief True when the report has an execution path associated with it.
+ ///
+ /// A report is said to be path-sensitive if it was thrown against a
+ /// particular exploded node in the path-sensitive analysis graph.
+ /// Path-sensitive reports have their intermediate path diagnostics
+ /// auto-generated, perhaps with the help of checker-defined visitors,
+ /// and may contain extra notes.
+ /// Path-insensitive reports consist only of a single warning message
+ /// in a specific location, and perhaps extra notes.
+ /// Path-sensitive checkers are allowed to throw path-insensitive reports.
+ bool isPathSensitive() const { return ErrorNode != nullptr; }
+
const ExplodedNode *getErrorNode() const { return ErrorNode; }
StringRef getDescription() const { return Description; }
@@ -245,7 +260,27 @@
void setDeclWithIssue(const Decl *declWithIssue) {
DeclWithIssue = declWithIssue;
}
-
+
+ /// Add new item to the list of additional notes that need to be attached to
+ /// this path-insensitive report. If you want to add extra notes to a
+ /// path-sensitive report, you need to use a BugReporterVisitor because it
+ /// allows you to specify where exactly in the auto-generated path diagnostic
+ /// the extra note should appear.
+ void addExtraNote(StringRef Msg, const PathDiagnosticLocation &Pos,
+ ArrayRef<SourceRange> Ranges = {}) {
+ PathDiagnosticExtraNotePiece *P =
+ new PathDiagnosticExtraNotePiece(Pos, Msg);
+
+ for (const auto &R : Ranges)
+ P->addRange(R);
+
+ ExtraNotes.push_back(P);
+ }
+
+ virtual const ExtraNoteList &getExtraNotes() {
+ return ExtraNotes;
+ }
+
/// \brief This allows for addition of meta data to the diagnostic.
///
/// Currently, only the HTMLDiagnosticClient knows how to display it.
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -266,6 +266,9 @@
/// \sa shouldWidenLoops
Optional<bool> WidenLoops;
+ /// \sa shouldDisplayExtraNotesAsEvents
+ Optional<bool> DisplayExtraNotesAsEvents;
+
/// A helper function that retrieves option for a given full-qualified
/// checker name.
/// Options for checkers can be specified via 'analyzer-config' command-line
@@ -534,6 +537,14 @@
/// This is controlled by the 'widen-loops' config option.
bool shouldWidenLoops();
+ /// Returns true if the bug reporter should transparently treat extra note
+ /// diagnostic pieces as event diagnostic pieces. Useful when the diagnostic
+ /// consumer doesn't support the extra note pieces.
+ ///
+ /// This is controlled by the 'extra-notes-as-events' option, which defaults
+ /// to false when unset.
+ bool shouldDisplayExtraNotesAsEvents();
+
public:
AnalyzerOptions() :
AnalysisStoreOpt(RegionStoreModel),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
