Index: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -266,6 +266,9 @@ /// \sa shouldWidenLoops Optional WidenLoops; + /// \sa shouldDisplayNotesAsEvents + Optional DisplayNotesAsEvents; + /// 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 'notes-as-events' option, which defaults + /// to false when unset. + bool shouldDisplayNotesAsEvents(); + public: AnalyzerOptions() : AnalysisStoreOpt(RegionStoreModel), Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -66,6 +66,8 @@ typedef SmallVector, 8> VisitorList; typedef VisitorList::iterator visitor_iterator; typedef SmallVector ExtraTextList; + typedef SmallVector, 4> + NoteList; protected: friend class BugReporter; @@ -82,7 +84,8 @@ const ExplodedNode *ErrorNode; SmallVector Ranges; ExtraTextList ExtraText; - + NoteList Notes; + typedef llvm::DenseSet Symbols; typedef llvm::DenseSet 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 addNote(StringRef Msg, const PathDiagnosticLocation &Pos, + ArrayRef Ranges = {}) { + PathDiagnosticNotePiece *P = + new PathDiagnosticNotePiece(Pos, Msg); + + for (const auto &R : Ranges) + P->addRange(R); + + Notes.push_back(P); + } + + virtual const NoteList &getNotes() { + return Notes; + } + /// \brief This allows for addition of meta data to the diagnostic. /// /// Currently, only the HTMLDiagnosticClient knows how to display it. Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ cfe/trunk/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, Note }; 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() == Note; } }; @@ -710,6 +711,23 @@ void Profile(llvm::FoldingSetNodeID &ID) const override; }; +class PathDiagnosticNotePiece: public PathDiagnosticSpotPiece { +public: + PathDiagnosticNotePiece(const PathDiagnosticLocation &Pos, StringRef S, + bool AddPosRange = true) + : PathDiagnosticSpotPiece(Pos, S, Note, AddPosRange) {} + + ~PathDiagnosticNotePiece() override; + + static inline bool classof(const PathDiagnosticPiece *P) { + return P->getKind() == Note; + } + + 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: cfe/trunk/lib/Rewrite/HTMLRewrite.cpp =================================================================== --- cfe/trunk/lib/Rewrite/HTMLRewrite.cpp +++ cfe/trunk/lib/Rewrite/HTMLRewrite.cpp @@ -324,6 +324,7 @@ " .msgT { padding:0x; spacing:0x }\n" " .msgEvent { background-color:#fff8b4; color:#000000 }\n" " .msgControl { background-color:#bbbbbb; color:#000000 }\n" + " .msgNote { 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; " @@ -343,8 +344,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" "\n\n"; // Generate header Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -344,3 +344,10 @@ WidenLoops = getBooleanOption("widen-loops", /*Default=*/false); return WidenLoops.getValue(); } + +bool AnalyzerOptions::shouldDisplayNotesAsEvents() { + if (!DisplayNotesAsEvents.hasValue()) + DisplayNotesAsEvents = + getBooleanOption("notes-as-events", /*Default=*/false); + return DisplayNotesAsEvents.getValue(); +} Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp +++ cfe/trunk/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(piece)->path); break; - case clang::ento::PathDiagnosticPiece::Macro: + case PathDiagnosticPiece::Macro: removeRedundantMsgs(cast(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::Note: + break; } path.push_back(piece); } @@ -197,6 +199,9 @@ } case PathDiagnosticPiece::ControlFlow: break; + + case PathDiagnosticPiece::Note: + break; } pieces.push_back(piece); @@ -3403,25 +3408,28 @@ exampleReport->getUniqueingLocation(), exampleReport->getUniqueingDecl())); - MaxBugClassSize = std::max(bugReports.size(), - static_cast(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(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(MaxValidBugClassSize)); + MaxValidBugClassSize = + std::max(bugReports.size(), static_cast(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().shouldDisplayNotesAsEvents()) { + // For path diagnostic consumers that don't support extra notes, + // we may optionally convert those to path notes. + for (auto I = exampleReport->getNotes().rbegin(), + E = exampleReport->getNotes().rend(); I != E; ++I) { + PathDiagnosticNotePiece *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->getNotes().rbegin(), + E = exampleReport->getNotes().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 PathDiagnosticNotePiece::dump() const { + llvm::errs() << "NOTE\n--------------\n"; + llvm::errs() << getString() << "\n"; + llvm::errs() << " ---- at ----\n"; + getLocation().dump(); +} + LLVM_DUMP_METHOD void PathDiagnosticLocation::dump() const { if (!isValid()) { llvm::errs() << "\n"; Index: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -152,13 +152,30 @@ } // 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 TotalNotePieces = + std::count_if(path.begin(), path.end(), + [](const IntrusiveRefCntPtr &p) { + return isa(p.get()); + }); + + unsigned TotalRegularPieces = TotalPieces - TotalNotePieces; + unsigned NumRegularPieces = TotalRegularPieces; + unsigned NumNotePieces = TotalNotePieces; + + for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) { + if (isa(I->get())) { + // This adds diagnostic bubbles, but not navigation. + // Navigation through note pieces would be added later, + // as a separate pass through the piece list. + HandlePiece(R, FID, **I, NumNotePieces, TotalNotePieces); + --NumNotePieces; + } else { + HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces); + --NumRegularPieces; + } + } // Add line numbers, header, footer, etc. @@ -192,24 +209,38 @@ int ColumnNumber = path.back()->getLocation().asLocation().getExpansionColumnNumber(); // Add the name of the file as an

tag. - { std::string s; llvm::raw_string_ostream os(s); os << "\n" - << "

Bug Summary

\n\n" + << "

Bug Summary

\n
\n" "\n\n" - "\n"; + << html::EscapeText(DirName) + << html::EscapeText(Entry->getName()) + << "\n\n"; + + // The navigation across the extra notes pieces. + unsigned NumExtraPieces = 0; + for (const auto &Piece : path) { + if (const auto *P = dyn_cast(Piece.get())) { + int LineNumber = + P->getLocation().asLocation().getExpansionLineNumber(); + int ColumnNumber = + P->getLocation().asLocation().getExpansionColumnNumber(); + os << ""; + ++NumExtraPieces; + } + } // Output any other meta data. @@ -385,13 +416,20 @@ // Create the html for the message. const char *Kind = nullptr; + bool IsNote = 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::Note: + Kind = "Note"; + IsNote = true; + SuppressIndex = true; + break; } std::string sbuf; @@ -399,7 +437,9 @@ os << "\n
File:" - << html::EscapeText(DirName) - << html::EscapeText(Entry->getName()) - << "
Location:" - "line " - << LineNumber - << ", column " - << ColumnNumber - << "
Description:" - << D.getVerboseDescription() << "
Warning:" + "line " + << LineNumber + << ", column " + << ColumnNumber + << "
" + << D.getVerboseDescription() << "
Note:" + << "line " + << LineNumber << ", column " << ColumnNumber << "
" + << P->getString() << "
"; - if (max > 1) { + if (!SuppressIndex) { os << ""; if (num < max) { os << ""; if (num < max) { os << "
"; os << "
1) { + if (!SuppressIndex) { os << "
(X), cast(Y)); - case clang::ento::PathDiagnosticPiece::Event: + case PathDiagnosticPiece::Event: + case PathDiagnosticPiece::Note: return None; - case clang::ento::PathDiagnosticPiece::Macro: + case PathDiagnosticPiece::Macro: return compareMacro(cast(X), cast(Y)); - case clang::ento::PathDiagnosticPiece::Call: + case PathDiagnosticPiece::Call: return compareCall(cast(X), cast(Y)); } @@ -1098,6 +1101,10 @@ ID.Add(**I); } +void PathDiagnosticNotePiece::Profile(llvm::FoldingSetNodeID &ID) const { + PathDiagnosticSpotPiece::Profile(ID); +} + void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const { ID.Add(getLocation()); ID.AddString(BugType); Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -281,6 +281,9 @@ ReportMacro(o, cast(P), FM, SM, LangOpts, indent, depth); break; + case PathDiagnosticPiece::Note: + // FIXME: Extend the plist format to support those. + break; } } Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -113,16 +113,28 @@ Diag.Report(WarnLoc, WarnID) << PD->getShortDescription() << PD->path.back()->getRanges(); + // First, add extra notes, even if paths should not be included. + for (const auto &Piece : PD->path) { + if (!isa(Piece.get())) + continue; + + SourceLocation NoteLoc = Piece->getLocation().asLocation(); + Diag.Report(NoteLoc, NoteID) << Piece->getString() + << Piece->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) { - SourceLocation NoteLoc = (*PI)->getLocation().asLocation(); - Diag.Report(NoteLoc, NoteID) << (*PI)->getString() - << (*PI)->getRanges(); + for (const auto &Piece : FlatPath) { + if (isa(Piece.get())) + continue; + + SourceLocation NoteLoc = Piece->getLocation().asLocation(); + Diag.Report(NoteLoc, NoteID) << Piece->getString() + << Piece->getRanges(); } } }