From: Kristof Umann Date: Tue, 13 Aug 2019 22:03:08 +0000 (+0000) Subject: [analyzer][NFC] Make sure that the BugReport is not modified during the construction... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f1293c56cf60ab1ab5b963021fd8a8de99e1f68b;p=clang [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces I feel this is kinda important, because in a followup patch I'm adding different kinds of interestingness, and propagating the correct kind in BugReporter.cpp is just one less thing to worry about. Differential Revision: https://reviews.llvm.org/D65578 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@368755 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 7fb82a58a9..c3d7ba3120 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -105,6 +105,7 @@ protected: const ExplodedNode *ErrorNode = nullptr; SmallVector Ranges; + const SourceRange ErrorNodeRange; ExtraTextList ExtraText; NoteList Notes; @@ -155,16 +156,22 @@ protected: llvm::SmallSet TrackedConditions; public: - BugReport(const BugType& bt, StringRef desc, const ExplodedNode *errornode) - : BT(bt), Description(desc), ErrorNode(errornode) {} + BugReport(const BugType &bt, StringRef desc, const ExplodedNode *errornode) + : BT(bt), Description(desc), ErrorNode(errornode), + ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() + : SourceRange()) {} - BugReport(const BugType& bt, StringRef shortDesc, StringRef desc, + BugReport(const BugType &bt, StringRef shortDesc, StringRef desc, const ExplodedNode *errornode) : BT(bt), ShortDescription(shortDesc), Description(desc), - ErrorNode(errornode) {} + ErrorNode(errornode), + ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() + : SourceRange()) {} BugReport(const BugType &bt, StringRef desc, PathDiagnosticLocation l) - : BT(bt), Description(desc), Location(l) {} + : BT(bt), Description(desc), Location(l), + ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() + : SourceRange()) {} /// Create a BugReport with a custom uniqueing location. /// @@ -323,7 +330,7 @@ public: } /// Get the SourceRanges associated with the report. - virtual llvm::iterator_range getRanges(); + virtual llvm::iterator_range getRanges() const; /// Add custom or predefined bug report visitors to this report. /// diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h index 536ed757e6..b2927a08b6 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -82,7 +82,7 @@ public: /// Generates the default final diagnostic piece. static PathDiagnosticPieceRef getDefaultEndPath(const BugReporterContext &BRC, const ExplodedNode *N, - BugReport &BR); + const BugReport &BR); }; /// Finds last store into the given region, diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h index ef3c75f87a..6e2a613aed 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h @@ -67,7 +67,7 @@ public: ExplodedNode *n, SymbolRef sym, StringRef endText); - llvm::iterator_range getRanges() override { + llvm::iterator_range getRanges() const override { if (!isLeak) return BugReport::getRanges(); return llvm::make_range(ranges_iterator(), ranges_iterator()); diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 92d35b9669..f2983b7f4b 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -192,13 +192,17 @@ public: }; /// Contains every contextual information needed for constructing a -/// PathDiagnostic object for a given bug report. This class (and aside from -/// some caching BugReport does in the background) and its fields are immutable, -/// and passes a BugReportConstruct object around during the construction. +/// PathDiagnostic object for a given bug report. This class and its fields are +/// immutable, and passes a BugReportConstruct object around during the +/// construction. class PathDiagnosticBuilder : public BugReporterContext { /// A linear path from the error node to the root. std::unique_ptr BugPath; - BugReport *R; + /// The bug report we're describing. Visitors create their diagnostics with + /// them being the last entities being able to modify it (for example, + /// changing interestingness here would cause inconsistencies as to how this + /// file and visitors construct diagnostics), hence its const. + const BugReport *R; /// The leaf of the bug path. This isn't the same as the bug reports error /// node, which refers to the *original* graph, not the bug path. const ExplodedNode *const ErrorNode; @@ -257,7 +261,7 @@ private: ExecutionContinues(llvm::raw_string_ostream &os, const PathDiagnosticConstruct &C) const; - BugReport *getBugReport() const { return R; } + const BugReport *getBugReport() const { return R; } }; } // namespace @@ -2172,14 +2176,13 @@ const Stmt *BugReport::getStmt() const { return S; } -llvm::iterator_range BugReport::getRanges() { +llvm::iterator_range BugReport::getRanges() const { // If no custom ranges, add the range of the statement corresponding to // the error node. if (Ranges.empty()) { if (const auto *E = dyn_cast_or_null(getStmt())) - addRange(E->getSourceRange()); - else - return llvm::make_range(ranges_iterator(), ranges_iterator()); + return llvm::make_range(&ErrorNodeRange, &ErrorNodeRange + 1); + return llvm::make_range(ranges_iterator(), ranges_iterator()); } // User-specified absence of range info. diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 131678cb73..02bdf86f57 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -267,7 +267,7 @@ void BugReporterVisitor::finalizeVisitor(BugReporterContext &, PathDiagnosticPieceRef BugReporterVisitor::getDefaultEndPath(const BugReporterContext &BRC, const ExplodedNode *EndPathNode, - BugReport &BR) { + const BugReport &BR) { PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath( EndPathNode, BRC.getSourceManager());