diff options
-rw-r--r-- | include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h | 6 | ||||
-rw-r--r-- | include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h | 53 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/BugReporter.cpp | 11 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp | 8 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/PathDiagnostic.cpp | 45 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/PlistDiagnostics.cpp | 3 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp | 1 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp | 3 | ||||
-rw-r--r-- | test/Analysis/plist-html-macros.c | 31 |
9 files changed, 96 insertions, 65 deletions
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index c8581e2e23..0e1d15253c 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -163,8 +163,10 @@ public: const StringRef getDescription() const { return Description; } - const StringRef getShortDescription() const { - return ShortDescription.empty() ? Description : ShortDescription; + const StringRef getShortDescription(bool UseFallback = true) const { + if (ShortDescription.empty() && UseFallback) + return Description; + return ShortDescription; } /// Indicates whether or not any path pruning should take place diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h index ad8a0694b4..dc275f08cc 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -97,7 +97,6 @@ public: virtual PathGenerationScheme getGenerationScheme() const { return Minimal; } virtual bool supportsLogicalOpControlFlow() const { return false; } virtual bool supportsAllBlockEdges() const { return false; } - virtual bool useVerboseDescription() const { return true; } /// Return true if the PathDiagnosticConsumer supports individual /// PathDiagnostics that span multiple files. @@ -653,14 +652,22 @@ public: class PathDiagnostic : public llvm::FoldingSetNode { const Decl *DeclWithIssue; std::string BugType; - std::string Desc; + std::string VerboseDesc; + std::string ShortDesc; std::string Category; std::deque<std::string> OtherDesc; + PathDiagnosticLocation Loc; PathPieces pathImpl; llvm::SmallVector<PathPieces *, 3> pathStack; PathDiagnostic(); // Do not implement. public: + PathDiagnostic(const Decl *DeclWithIssue, StringRef bugtype, + StringRef verboseDesc, StringRef shortDesc, + StringRef category); + + ~PathDiagnostic(); + const PathPieces &path; /// Return the path currently used by builders for constructing the @@ -683,16 +690,24 @@ public: void popActivePath() { if (!pathStack.empty()) pathStack.pop_back(); } bool isWithinCall() const { return !pathStack.empty(); } - - // PathDiagnostic(); - PathDiagnostic(const Decl *DeclWithIssue, - StringRef bugtype, - StringRef desc, - StringRef category); - ~PathDiagnostic(); + void setEndOfPath(PathDiagnosticPiece *EndPiece) { + assert(!Loc.isValid() && "End location already set!"); + Loc = EndPiece->getLocation(); + assert(Loc.isValid() && "Invalid location for end-of-path piece"); + getActivePath().push_back(EndPiece); + } - StringRef getDescription() const { return Desc; } + void resetPath() { + pathStack.clear(); + pathImpl.clear(); + Loc = PathDiagnosticLocation(); + } + + StringRef getVerboseDescription() const { return VerboseDesc; } + StringRef getShortDescription() const { + return ShortDesc.empty() ? VerboseDesc : ShortDesc; + } StringRef getBugType() const { return BugType; } StringRef getCategory() const { return Category; } @@ -706,15 +721,27 @@ public: meta_iterator meta_end() const { return OtherDesc.end(); } void addMeta(StringRef s) { OtherDesc.push_back(s); } - PathDiagnosticLocation getLocation() const; + PathDiagnosticLocation getLocation() const { + assert(Loc.isValid() && "No end-of-path location set yet!"); + return Loc; + } void flattenLocations() { + Loc.flatten(); for (PathPieces::iterator I = pathImpl.begin(), E = pathImpl.end(); I != E; ++I) (*I)->flattenLocations(); } - + + /// Profiles the diagnostic, independent of the path it references. + /// + /// This can be used to merge diagnostics that refer to the same issue + /// along different paths. void Profile(llvm::FoldingSetNodeID &ID) const; - + + /// Profiles the diagnostic, including its path. + /// + /// Two diagnostics with the same issue along different paths will generate + /// different profiles. void FullProfile(llvm::FoldingSetNodeID &ID) const; }; diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 1866a27f72..68cc7d88b1 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -1898,7 +1898,7 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, visitors.push_back((*I)->clone()); // Clear out the active path from any previous work. - PD.getActivePath().clear(); + PD.resetPath(); originalReportConfigToken = R->getConfigurationChangeToken(); // Generate the very last diagnostic piece - the piece is visible before @@ -1915,7 +1915,7 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, if (!LastPiece) LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R); if (LastPiece) - PD.getActivePath().push_back(LastPiece); + PD.setEndOfPath(LastPiece); else return; @@ -2106,9 +2106,8 @@ void BugReporter::FlushReport(BugReport *exampleReport, OwningPtr<PathDiagnostic> D(new PathDiagnostic(exampleReport->getDeclWithIssue(), exampleReport->getBugType().getName(), - PD.useVerboseDescription() - ? exampleReport->getDescription() - : exampleReport->getShortDescription(), + exampleReport->getDescription(), + exampleReport->getShortDescription(/*Fallback=*/false), BT.getCategory())); // Generate the full path diagnostic, using the generation scheme @@ -2128,7 +2127,7 @@ void BugReporter::FlushReport(BugReport *exampleReport, llvm::tie(Beg, End) = exampleReport->getRanges(); for ( ; Beg != End; ++Beg) piece->addRange(*Beg); - D->getActivePath().push_back(piece); + D->setEndOfPath(piece); } // Get the meta data. diff --git a/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp index 46adfbd795..211bcb9124 100644 --- a/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -189,7 +189,7 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D, << (*path.rbegin())->getLocation().asLocation().getExpansionColumnNumber() << "</a></td></tr>\n" "<tr><td class=\"rowname\">Description:</td><td>" - << D.getDescription() << "</td></tr>\n"; + << D.getVerboseDescription() << "</td></tr>\n"; // Output any other meta data. @@ -209,15 +209,15 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D, std::string s; llvm::raw_string_ostream os(s); - const std::string& BugDesc = D.getDescription(); + StringRef BugDesc = D.getVerboseDescription(); if (!BugDesc.empty()) os << "\n<!-- BUGDESC " << BugDesc << " -->\n"; - const std::string& BugType = D.getBugType(); + StringRef BugType = D.getBugType(); if (!BugType.empty()) os << "\n<!-- BUGTYPE " << BugType << " -->\n"; - const std::string& BugCategory = D.getCategory(); + StringRef BugCategory = D.getCategory(); if (!BugCategory.empty()) os << "\n<!-- BUGCATEGORY " << BugCategory << " -->\n"; diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index 97ef906cac..d010a668d9 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -104,11 +104,12 @@ void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current, PathDiagnostic::~PathDiagnostic() {} PathDiagnostic::PathDiagnostic(const Decl *declWithIssue, - StringRef bugtype, StringRef desc, - StringRef category) + StringRef bugtype, StringRef verboseDesc, + StringRef shortDesc, StringRef category) : DeclWithIssue(declWithIssue), BugType(StripTrailingDots(bugtype)), - Desc(StripTrailingDots(desc)), + VerboseDesc(StripTrailingDots(verboseDesc)), + ShortDesc(StripTrailingDots(shortDesc)), Category(StripTrailingDots(category)), path(pathImpl) {} @@ -227,8 +228,8 @@ struct CompareDiagnostics { return false; // Next, compare by bug description. - StringRef XDesc = X->getDescription(); - StringRef YDesc = Y->getDescription(); + StringRef XDesc = X->getVerboseDescription(); + StringRef YDesc = Y->getVerboseDescription(); if (XDesc < YDesc) return true; if (XDesc != YDesc) @@ -271,18 +272,11 @@ void PathDiagnosticConsumer::FlushDiagnostics( } } -static void ProfileDiagnostic(const PathDiagnostic &PD, - llvm::FoldingSetNodeID &NodeID) { - NodeID.AddString(PD.getBugType()); - NodeID.AddString(PD.getDescription()); - NodeID.Add(PD.getLocation()); -} - void PathDiagnosticConsumer::FilesMade::addDiagnostic(const PathDiagnostic &PD, StringRef ConsumerName, StringRef FileName) { llvm::FoldingSetNodeID NodeID; - ProfileDiagnostic(PD, NodeID); + NodeID.Add(PD); void *InsertPos; PDFileEntry *Entry = FindNodeOrInsertPos(NodeID, InsertPos); if (!Entry) { @@ -303,7 +297,7 @@ void PathDiagnosticConsumer::FilesMade::addDiagnostic(const PathDiagnostic &PD, PathDiagnosticConsumer::PDFileEntry::ConsumerFiles * PathDiagnosticConsumer::FilesMade::getFiles(const PathDiagnostic &PD) { llvm::FoldingSetNodeID NodeID; - ProfileDiagnostic(PD, NodeID); + NodeID.Add(PD); void *InsertPos; PDFileEntry *Entry = FindNodeOrInsertPos(NodeID, InsertPos); if (!Entry) @@ -627,24 +621,6 @@ void PathDiagnosticLocation::flatten() { } } -PathDiagnosticLocation PathDiagnostic::getLocation() const { - assert(path.size() > 0 && - "getLocation() requires a non-empty PathDiagnostic."); - - PathDiagnosticPiece *p = path.rbegin()->getPtr(); - - while (true) { - if (PathDiagnosticCallPiece *cp = dyn_cast<PathDiagnosticCallPiece>(p)) { - assert(!cp->path.empty()); - p = cp->path.rbegin()->getPtr(); - continue; - } - break; - } - - return p->getLocation(); -} - //===----------------------------------------------------------------------===// // Manipulation of PathDiagnosticCallPieces. //===----------------------------------------------------------------------===// @@ -793,10 +769,9 @@ void PathDiagnosticMacroPiece::Profile(llvm::FoldingSetNodeID &ID) const { } void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const { - if (!path.empty()) - getLocation().Profile(ID); + ID.Add(getLocation()); ID.AddString(BugType); - ID.AddString(Desc); + ID.AddString(VerboseDesc); ID.AddString(Category); } diff --git a/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp index c1c46c293a..0a534bf7c0 100644 --- a/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -47,7 +47,6 @@ namespace { PathGenerationScheme getGenerationScheme() const { return Extensive; } bool supportsLogicalOpControlFlow() const { return true; } bool supportsAllBlockEdges() const { return true; } - virtual bool useVerboseDescription() const { return false; } virtual bool supportsCrossFileDiagnostics() const { return SupportsCrossFileDiagnostics; } @@ -443,7 +442,7 @@ void PlistDiagnostics::FlushDiagnosticsImpl( // Output the bug type and bug category. o << " <key>description</key>"; - EmitString(o, D->getDescription()) << '\n'; + EmitString(o, D->getShortDescription()) << '\n'; o << " <key>category</key>"; EmitString(o, D->getCategory()) << '\n'; o << " <key>type</key>"; diff --git a/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp b/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp index 66bf4bb222..e09f4e3653 100644 --- a/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp @@ -41,7 +41,6 @@ public: PathGenerationScheme getGenerationScheme() const { return Minimal; } bool supportsLogicalOpControlFlow() const { return true; } bool supportsAllBlockEdges() const { return true; } - virtual bool useVerboseDescription() const { return true; } virtual bool supportsCrossFileDiagnostics() const { return true; } }; diff --git a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp index bd643bab03..53747d4b4d 100644 --- a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -78,7 +78,6 @@ public: ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag) : Diag(Diag) {} virtual ~ClangDiagPathDiagConsumer() {} virtual StringRef getName() const { return "ClangDiags"; } - virtual bool useVerboseDescription() const { return false; } virtual PathGenerationScheme getGenerationScheme() const { return None; } void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags, @@ -86,7 +85,7 @@ public: for (std::vector<const PathDiagnostic*>::iterator I = Diags.begin(), E = Diags.end(); I != E; ++I) { const PathDiagnostic *PD = *I; - StringRef desc = PD->getDescription(); + StringRef desc = PD->getShortDescription(); SmallString<512> TmpStr; llvm::raw_svector_ostream Out(TmpStr); for (StringRef::iterator I=desc.begin(), E=desc.end(); I!=E; ++I) { diff --git a/test/Analysis/plist-html-macros.c b/test/Analysis/plist-html-macros.c new file mode 100644 index 0000000000..954aab0997 --- /dev/null +++ b/test/Analysis/plist-html-macros.c @@ -0,0 +1,31 @@ +// REQUIRES: shell +// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s +// (sanity check) + +// RUN: rm -rf %t.dir +// RUN: mkdir -p %t.dir +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-html -o %t.dir/index.plist %s +// RUN: ls %t.dir | grep \\.html | count 1 +// RUN: grep \\.html %t.dir/index.plist | count 1 + +// This tests two things: that the two calls to null_deref below are coalesced +// into a single bug by both the plist and HTML diagnostics, and that the plist +// diagnostics have a reference to the HTML diagnostics. (It would be nice to +// check more carefully that the two actually match, but that's hard to write +// in a lit RUN line.) + +#define CALL_FN(a) null_deref(a) + +void null_deref(int *a) { + if (a) + return; + *a = 1; // expected-warning{{null}} +} + +void test1() { + CALL_FN(0); +} + +void test2(int *p) { + CALL_FN(p); +} |