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 @@ -23,6 +23,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/FoldingSet.h" @@ -65,6 +66,11 @@ // Interface for individual bug reports. //===----------------------------------------------------------------------===// +/// A mapping from diagnostic consumers to the diagnostics they should +/// consume. +using DiagnosticForConsumerMapTy = + llvm::DenseMap>; + /// This class provides an interface through which checkers can create /// individual bug reports. class BugReport : public llvm::ilist_node { @@ -130,10 +136,6 @@ /// Used for ensuring the visitors are only added once. llvm::FoldingSet CallbacksSet; - /// Used for clients to tell if the report's configuration has changed - /// since the last time they checked. - unsigned ConfigurationChangeToken = 0; - /// When set, this flag disables all callstack pruning from a diagnostic /// path. This is useful for some reports that want maximum fidelty /// when reporting an issue. @@ -229,10 +231,6 @@ bool isInteresting(SVal V); bool isInteresting(const LocationContext *LC); - unsigned getConfigurationChangeToken() const { - return ConfigurationChangeToken; - } - /// Returns whether or not this report should be considered valid. /// /// Invalid reports are those that have been classified as likely false @@ -345,6 +343,9 @@ /// registerVarDeclsLastStore(). void addVisitor(std::unique_ptr visitor); + /// Remove all visitors attached to this bug report. + void clearVisitors(); + /// Iterators through the custom diagnostic visitors. visitor_iterator visitor_begin() { return Callbacks.begin(); } visitor_iterator visitor_end() { return Callbacks.end(); } @@ -406,6 +407,8 @@ /// BugReporter is a utility class for generating PathDiagnostics for analysis. /// It collects the BugReports and BugTypes and knows how to generate /// and flush the corresponding diagnostics. +/// +/// The base class is used for generating path-insensitive class BugReporter { public: enum Kind { BaseBRKind, GRBugReporterKind }; @@ -422,11 +425,11 @@ /// Generate and flush the diagnostics for the given bug report. void FlushReport(BugReportEquivClass& EQ); - /// Generate and flush the diagnostics for the given bug report - /// and PathDiagnosticConsumer. - void FlushReport(BugReport *exampleReport, - PathDiagnosticConsumer &PD, - ArrayRef BugReports); + /// Generate the diagnostics for the given bug report. + std::unique_ptr + generateDiagnosticForConsumerMap(BugReport *exampleReport, + ArrayRef consumers, + ArrayRef bugReports); /// The set of bug reports tracked by the BugReporter. llvm::FoldingSet EQClasses; @@ -472,10 +475,10 @@ AnalyzerOptions &getAnalyzerOptions() { return D.getAnalyzerOptions(); } - virtual bool generatePathDiagnostic(PathDiagnostic& pathDiagnostic, - PathDiagnosticConsumer &PC, - ArrayRef &bugReports) { - return true; + virtual std::unique_ptr + generatePathDiagnostics(ArrayRef consumers, + ArrayRef &bugReports) { + return {}; } void Register(BugType *BT); @@ -506,7 +509,7 @@ StringRef category); }; -// FIXME: Get rid of GRBugReporter. It's the wrong abstraction. +/// GRBugReporter is used for generating path-sensitive reports. class GRBugReporter : public BugReporter { ExprEngine& Eng; @@ -528,16 +531,14 @@ /// engine. ProgramStateManager &getStateManager(); - /// Generates a path corresponding to one of the given bug reports. + /// \p bugReports A set of bug reports within a *single* equivalence class /// - /// Which report is used for path generation is not specified. The - /// bug reporter will try to pick the shortest path, but this is not - /// guaranteed. - /// - /// \return True if the report was valid and a path was generated, - /// false if the reports should be considered invalid. - bool generatePathDiagnostic(PathDiagnostic &PD, PathDiagnosticConsumer &PC, - ArrayRef &bugReports) override; + /// \return A mapping from consumers to the corresponding diagnostics. + /// Iterates through the bug reports within a single equivalence class, + /// stops at a first non-invalidated report. + std::unique_ptr + generatePathDiagnostics(ArrayRef consumers, + ArrayRef &bugReports) override; /// classof - Used by isa<>, cast<>, and dyn_cast<>. static bool classof(const BugReporter* R) { @@ -545,13 +546,27 @@ } }; + +class NodeMapClosure : public BugReport::NodeResolver { + InterExplodedGraphMap &M; + +public: + NodeMapClosure(InterExplodedGraphMap &m) : M(m) {} + + const ExplodedNode *getOriginalNode(const ExplodedNode *N) override { + return M.lookup(N); + } +}; + class BugReporterContext { GRBugReporter &BR; + NodeMapClosure NMC; virtual void anchor(); public: - BugReporterContext(GRBugReporter& br) : BR(br) {} + BugReporterContext(GRBugReporter &br, InterExplodedGraphMap &Backmap) + : BR(br), NMC(Backmap) {} virtual ~BugReporterContext() = default; @@ -575,7 +590,7 @@ return BR.getSourceManager(); } - virtual BugReport::NodeResolver& getNodeResolver() = 0; + NodeMapClosure& getNodeResolver() { return NMC; } }; } // namespace ento Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -40,12 +40,6 @@ class PathDiagnosticPiece; /// BugReporterVisitors are used to add custom diagnostics along a path. -/// -/// Custom visitors should subclass the BugReporterVisitorImpl class for a -/// default implementation of the clone() method. -/// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the -/// default implementation of clone() will NOT do the right thing, and you -/// will have to provide your own implementation.) class BugReporterVisitor : public llvm::FoldingSetNode { public: BugReporterVisitor() = default; @@ -53,19 +47,13 @@ BugReporterVisitor(BugReporterVisitor &&) {} virtual ~BugReporterVisitor(); - /// Returns a copy of this BugReporter. - /// - /// Custom BugReporterVisitors should not override this method directly. - /// Instead, they should inherit from BugReporterVisitorImpl and provide - /// a protected or public copy constructor. - /// - /// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the - /// default implementation of clone() will NOT do the right thing, and you - /// will have to provide your own implementation.) - virtual std::unique_ptr clone() const = 0; - /// Return a diagnostic piece which should be associated with the /// given node. + /// Note that this function does *not* get run on the very last node + /// of the report, as the PathDiagnosticPiece associated with the + /// last node should be unique. + /// Use {@code getEndPath} to customize the note associated with the report + /// end instead. /// /// The last parameter can be used to register a new visitor with the given /// BugReport while processing a node. @@ -84,34 +72,18 @@ /// /// NOTE that this function can be implemented on at most one used visitor, /// and otherwise it crahes at runtime. - virtual std::unique_ptr + virtual std::shared_ptr getEndPath(BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR); virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0; /// Generates the default final diagnostic piece. - static std::unique_ptr + static std::shared_ptr getDefaultEndPath(BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR); }; -/// This class provides a convenience implementation for clone() using the -/// Curiously-Recurring Template Pattern. If you are implementing a custom -/// BugReporterVisitor, subclass BugReporterVisitorImpl and provide a public -/// or protected copy constructor. -/// -/// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the -/// default implementation of clone() will NOT do the right thing, and you -/// will have to provide your own implementation.) -template -class BugReporterVisitorImpl : public BugReporterVisitor { - std::unique_ptr clone() const override { - return llvm::make_unique(*static_cast(this)); - } -}; - -class FindLastStoreBRVisitor final - : public BugReporterVisitorImpl { +class FindLastStoreBRVisitor final : public BugReporterVisitor { const MemRegion *R; SVal V; bool Satisfied = false; @@ -138,8 +110,7 @@ BugReport &BR) override; }; -class TrackConstraintBRVisitor final - : public BugReporterVisitorImpl { +class TrackConstraintBRVisitor final : public BugReporterVisitor { DefinedSVal Constraint; bool Assumption; bool IsSatisfied = false; @@ -172,8 +143,7 @@ /// \class NilReceiverBRVisitor /// Prints path notes when a message is sent to a nil receiver. -class NilReceiverBRVisitor final - : public BugReporterVisitorImpl { +class NilReceiverBRVisitor final : public BugReporterVisitor { public: void Profile(llvm::FoldingSetNodeID &ID) const override { static int x = 0; @@ -191,8 +161,7 @@ }; /// Visitor that tries to report interesting diagnostics from conditions. -class ConditionBRVisitor final - : public BugReporterVisitorImpl { +class ConditionBRVisitor final : public BugReporterVisitor { // FIXME: constexpr initialization isn't supported by MSVC2013. static const char *const GenericTrueMessage; static const char *const GenericFalseMessage; @@ -255,7 +224,7 @@ /// /// Currently this suppresses reports based on locations of bugs. class LikelyFalsePositiveSuppressionBRVisitor final - : public BugReporterVisitorImpl { + : public BugReporterVisitor { public: static void *getTag() { static int Tag = 0; @@ -282,8 +251,7 @@ /// /// As a result, BugReporter will not prune the path through the function even /// if the region's contents are not modified/accessed by the call. -class UndefOrNullArgVisitor final - : public BugReporterVisitorImpl { +class UndefOrNullArgVisitor final : public BugReporterVisitor { /// The interesting memory region this visitor is tracking. const MemRegion *R; @@ -302,8 +270,7 @@ BugReport &BR) override; }; -class SuppressInlineDefensiveChecksVisitor final - : public BugReporterVisitorImpl { +class SuppressInlineDefensiveChecksVisitor final : public BugReporterVisitor { /// The symbolic value for which we are tracking constraints. /// This value is constrained to null in the end of path. DefinedSVal V; @@ -333,8 +300,7 @@ BugReport &BR) override; }; -class CXXSelfAssignmentBRVisitor final - : public BugReporterVisitorImpl { +class CXXSelfAssignmentBRVisitor final : public BugReporterVisitor { bool Satisfied = false; public: @@ -350,7 +316,7 @@ /// The bug visitor prints a diagnostic message at the location where a given /// variable was tainted. -class TaintBugVisitor final : public BugReporterVisitorImpl { +class TaintBugVisitor final : public BugReporterVisitor { private: const SVal V; @@ -367,8 +333,7 @@ /// The bug visitor will walk all the nodes in a path and collect all the /// constraints. When it reaches the root node, will create a refutation /// manager and check if the constraints are satisfiable -class FalsePositiveRefutationBRVisitor final - : public BugReporterVisitorImpl { +class FalsePositiveRefutationBRVisitor final : public BugReporterVisitor { private: /// Holds the constraints in a given path // TODO: should we use a set? 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 @@ -811,7 +811,7 @@ bool isWithinCall() const { return !pathStack.empty(); } - void setEndOfPath(std::unique_ptr EndPiece) { + void setEndOfPath(std::shared_ptr EndPiece) { assert(!Loc.isValid() && "End location already set!"); Loc = EndPiece->getLocation(); assert(Loc.isValid() && "Invalid location for end-of-path piece"); @@ -824,12 +824,6 @@ VerboseDesc += S; } - void resetPath() { - pathStack.clear(); - pathImpl.clear(); - Loc = PathDiagnosticLocation(); - } - /// If the last piece of the report point to the header file, resets /// the location of the report to be the last location in the main source /// file. @@ -865,7 +859,6 @@ filesmap_iterator executedLines_end() const { return ExecutedLines->end(); } PathDiagnosticLocation getLocation() const { - assert(Loc.isValid() && "No report location set yet!"); return Loc; } Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp @@ -39,7 +39,7 @@ : public Checker> { mutable std::unique_ptr BT; - class DeleteBugVisitor : public BugReporterVisitorImpl { + class DeleteBugVisitor : public BugReporterVisitor { public: DeleteBugVisitor() : Satisfied(false) {} void Profile(llvm::FoldingSetNodeID &ID) const override { Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp @@ -38,8 +38,7 @@ new BugType(this, "Dynamic and static type mismatch", "Type Error")); } - class DynamicTypeBugVisitor - : public BugReporterVisitorImpl { + class DynamicTypeBugVisitor : public BugReporterVisitor { public: DynamicTypeBugVisitor(const MemRegion *Reg) : Reg(Reg) {} Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -74,7 +74,7 @@ new BugType(this, "Generics", categories::CoreFoundationObjectiveC)); } - class GenericsBugVisitor : public BugReporterVisitorImpl { + class GenericsBugVisitor : public BugReporterVisitor { public: GenericsBugVisitor(SymbolRef S) : Sym(S) {} Index: cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -113,8 +113,7 @@ } namespace { -class NonLocalizedStringBRVisitor final - : public BugReporterVisitorImpl { +class NonLocalizedStringBRVisitor final : public BugReporterVisitor { const MemRegion *NonLocalizedString; bool Satisfied; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h @@ -78,7 +78,7 @@ /// Bug visitor class to find the node where the request region was previously /// used in order to include it into the BugReport path. - class RequestNodeVisitor : public BugReporterVisitorImpl { + class RequestNodeVisitor : public BugReporterVisitor { public: RequestNodeVisitor(const MemRegion *const MemoryRegion, const std::string &ErrText) Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -120,8 +120,7 @@ /// The bug visitor which allows us to print extra diagnostics along the /// BugReport path. For example, showing the allocation site of the leaked /// region. - class SecKeychainBugVisitor - : public BugReporterVisitorImpl { + class SecKeychainBugVisitor : public BugReporterVisitor { protected: // The allocated region symbol tracked by the main analysis. SymbolRef Sym; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -431,8 +431,7 @@ /// The bug visitor which allows us to print extra diagnostics along the /// BugReport path. For example, showing the allocation site of the leaked /// region. - class MallocBugVisitor final - : public BugReporterVisitorImpl { + class MallocBugVisitor final : public BugReporterVisitor { protected: enum NotificationMode { Normal, @@ -511,7 +510,7 @@ BugReporterContext &BRC, BugReport &BR) override; - std::unique_ptr + std::shared_ptr getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) override { if (!IsLeak) @@ -521,7 +520,7 @@ PathDiagnosticLocation::createEndOfPath(EndPathNode, BRC.getSourceManager()); // Do not add the statement itself as a range in case of leak. - return llvm::make_unique(L, BR.getDescription(), + return std::make_shared(L, BR.getDescription(), false); } Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp @@ -61,7 +61,7 @@ private: enum MisuseKind {MK_FunCall, MK_Copy, MK_Move}; - class MovedBugVisitor : public BugReporterVisitorImpl { + class MovedBugVisitor : public BugReporterVisitor { public: MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {} Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -128,8 +128,7 @@ DefaultBool NeedTracking; private: - class NullabilityBugVisitor - : public BugReporterVisitorImpl { + class NullabilityBugVisitor : public BugReporterVisitor { public: NullabilityBugVisitor(const MemRegion *M) : Region(M) {} Index: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp @@ -62,9 +62,7 @@ REGISTER_SET_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef) namespace { -class SuperDeallocBRVisitor final - : public BugReporterVisitorImpl { - +class SuperDeallocBRVisitor final : public BugReporterVisitor { SymbolRef ReceiverSymbol; bool Satisfied; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -1788,8 +1788,7 @@ //===---------===// // Bug Reports. // //===---------===// - - class CFRefReportVisitor : public BugReporterVisitorImpl { + class CFRefReportVisitor : public BugReporterVisitor { protected: SymbolRef Sym; const SummaryLogTy &SummaryLog; @@ -1810,7 +1809,7 @@ BugReporterContext &BRC, BugReport &BR) override; - std::unique_ptr getEndPath(BugReporterContext &BRC, + std::shared_ptr getEndPath(BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR) override; }; @@ -1821,18 +1820,9 @@ const SummaryLogTy &log) : CFRefReportVisitor(sym, GCEnabled, log) {} - std::unique_ptr getEndPath(BugReporterContext &BRC, + std::shared_ptr getEndPath(BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR) override; - - std::unique_ptr clone() const override { - // The curiously-recurring template pattern only works for one level of - // subclassing. Rather than make a new template base for - // CFRefReportVisitor, we simply override clone() to do the right thing. - // This could be trouble someday if BugReporterVisitorImpl is ever - // used for something else besides a convenient implementation of clone(). - return llvm::make_unique(*this); - } }; class CFRefReport : public BugReport { @@ -2365,14 +2355,14 @@ InterestingMethodContext); } -std::unique_ptr +std::shared_ptr CFRefReportVisitor::getEndPath(BugReporterContext &BRC, const ExplodedNode *EndN, BugReport &BR) { BR.markInteresting(Sym); return BugReporterVisitor::getDefaultEndPath(BRC, EndN, BR); } -std::unique_ptr +std::shared_ptr CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, const ExplodedNode *EndN, BugReport &BR) { @@ -2459,7 +2449,7 @@ os << " is not referenced later in this execution path and has a retain " "count of +" << RV->getCount(); - return llvm::make_unique(L, os.str()); + return std::make_shared(L, os.str()); } void CFRefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) { Index: cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp @@ -55,7 +55,7 @@ } }; -class DivisionBRVisitor : public BugReporterVisitorImpl { +class DivisionBRVisitor : public BugReporterVisitor { private: SymbolRef ZeroSymbol; const StackFrameContext *SFC; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -69,7 +69,7 @@ bool IsCopy) const; void checkVAListEndCall(const CallEvent &Call, CheckerContext &C) const; - class ValistBugVisitor : public BugReporterVisitorImpl { + class ValistBugVisitor : public BugReporterVisitor { public: ValistBugVisitor(const MemRegion *Reg, bool IsLeak = false) : Reg(Reg), IsLeak(IsLeak) {} @@ -78,7 +78,7 @@ ID.AddPointer(&X); ID.AddPointer(Reg); } - std::unique_ptr + std::shared_ptr getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) override { if (!IsLeak) @@ -87,8 +87,7 @@ PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath( EndPathNode, BRC.getSourceManager()); // Do not add the statement itself as a range in case of leak. - return llvm::make_unique(L, BR.getDescription(), - false); + return std::make_shared(L, BR.getDescription(), false); } std::shared_ptr VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -57,7 +57,7 @@ void reportBug(StringRef Msg, bool PureError, const MemRegion *Reg, CheckerContext &C) const; - class VirtualBugVisitor : public BugReporterVisitorImpl { + class VirtualBugVisitor : public BugReporterVisitor { private: const MemRegion *ObjectRegion; bool Found; Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -343,21 +343,9 @@ namespace { -class NodeMapClosure : public BugReport::NodeResolver { - InterExplodedGraphMap &M; - -public: - NodeMapClosure(InterExplodedGraphMap &m) : M(m) {} - - const ExplodedNode *getOriginalNode(const ExplodedNode *N) override { - return M.lookup(N); - } -}; - class PathDiagnosticBuilder : public BugReporterContext { BugReport *R; PathDiagnosticConsumer *PDC; - NodeMapClosure NMC; public: const LocationContext *LC; @@ -365,7 +353,7 @@ PathDiagnosticBuilder(GRBugReporter &br, BugReport *r, InterExplodedGraphMap &Backmap, PathDiagnosticConsumer *pdc) - : BugReporterContext(br), R(r), PDC(pdc), NMC(Backmap), + : BugReporterContext(br, Backmap), R(r), PDC(pdc), LC(r->getErrorNode()->getLocationContext()) {} PathDiagnosticLocation ExecutionContinues(const ExplodedNode *N); @@ -383,8 +371,6 @@ return getParentMap().getParent(S); } - NodeMapClosure& getNodeResolver() override { return NMC; } - PathDiagnosticLocation getEnclosingStmtLocation(const Stmt *S); PathDiagnosticConsumer::PathGenerationScheme getGenerationScheme() const { @@ -1021,6 +1007,9 @@ static const char StrLoopCollectionEmpty[] = "Loop body skipped when collection is empty"; +static std::unique_ptr +findExecutedLines(SourceManager &SM, const ExplodedNode *N); + /// Generate diagnostics for the node \p N, /// and write it into \p PD. /// \p AddPathEdges Whether diagnostic consumer can generate path arrows @@ -1259,83 +1248,15 @@ } } -/// There are two path diagnostics generation modes: with adding edges (used -/// for plists) and without (used for HTML and text). -/// When edges are added (\p ActiveScheme is Extensive), -/// the path is modified to insert artificially generated -/// edges. -/// Otherwise, more detailed diagnostics is emitted for block edges, explaining -/// the transitions in words. -static bool generatePathDiagnostics( - PathDiagnostic &PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, - LocationContextMap &LCM, - ArrayRef> visitors, - BugReport *R, - PathDiagnosticConsumer::PathGenerationScheme ActiveScheme) { - const ExplodedNode *LastNode = N; - BugReport *report = PDB.getBugReport(); - StackDiagVector CallStack; - InterestingExprs IE; - bool AddPathEdges = (ActiveScheme == PathDiagnosticConsumer::Extensive); - bool GenerateDiagnostics = (ActiveScheme != PathDiagnosticConsumer::None); - - PathDiagnosticLocation PrevLoc = GenerateDiagnostics ? - PD.getLocation() : PathDiagnosticLocation(); - - const ExplodedNode *NextNode = N->getFirstPred(); - while (NextNode) { - N = NextNode; - NextNode = N->getFirstPred(); - - if (GenerateDiagnostics) - generatePathDiagnosticsForNode( - N, PD, PrevLoc, PDB, LCM, CallStack, IE, AddPathEdges); - - if (!NextNode) { - for (auto &V : visitors) { - V->finalizeVisitor(PDB, LastNode, *R); - } - continue; - } - - // Add pieces from custom visitors. - llvm::FoldingSet DeduplicationSet; - for (auto &V : visitors) { - if (auto p = V->VisitNode(N, NextNode, PDB, *report)) { - - if (!GenerateDiagnostics) - continue; - - if (DeduplicationSet.GetOrInsertNode(p.get()) != p.get()) - continue; - - if (AddPathEdges) - addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), PDB.LC); - updateStackPiecesWithMessage(*p, CallStack); - PD.getActivePath().push_front(std::move(p)); - } - } - } - - if (AddPathEdges) { - // Add an edge to the start of the function. - // We'll prune it out later, but it helps make diagnostics more uniform. - const StackFrameContext *CalleeLC = PDB.LC->getCurrentStackFrame(); - const Decl *D = CalleeLC->getDecl(); - addEdgeToPath(PD.getActivePath(), PrevLoc, - PathDiagnosticLocation::createBegin(D, PDB.getSourceManager()), - CalleeLC); - } - - if (!report->isValid()) - return false; - - // After constructing the full PathDiagnostic, do a pass over it to compact - // PathDiagnosticPieces that occur within a macro. - if (!AddPathEdges && GenerateDiagnostics) - CompactPathDiagnostic(PD.getMutablePieces(), PDB.getSourceManager()); - - return true; +static std::unique_ptr +generateEmptyDiagnosticForReport(BugReport *R, SourceManager &SM) { + BugType &BT = R->getBugType(); + return llvm::make_unique( + R->getBugType().getCheckName(), R->getDeclWithIssue(), + R->getBugType().getName(), R->getDescription(), + R->getShortDescription(/*Fallback=*/false), BT.getCategory(), + R->getUniqueingLocation(), R->getUniqueingDecl(), + findExecutedLines(SM, R->getErrorNode())); } static const Stmt *getStmtParent(const Stmt *S, const ParentMap &PM) { @@ -1957,6 +1878,129 @@ Path.pop_front(); } +using VisitorsDiagnosticsTy = llvm::DenseMap>>; + +/// This function is responsible for generating diagnostic pieces that are +/// *not* provided by bug report visitors. +/// These diagnostics may differ depending on the consumer's settings, +/// and are therefore constructed separately for each consumer. +/// +/// There are two path diagnostics generation modes: with adding edges (used +/// for plists) and without (used for HTML and text). +/// When edges are added (\p ActiveScheme is Extensive), +/// the path is modified to insert artificially generated +/// edges. +/// Otherwise, more detailed diagnostics is emitted for block edges, explaining +/// the transitions in words. +static std::unique_ptr generatePathDiagnosticForConsumer( + PathDiagnosticConsumer::PathGenerationScheme ActiveScheme, + PathDiagnosticBuilder &PDB, + const ExplodedNode *ErrorNode, + const VisitorsDiagnosticsTy &VisitorsDiagnostics) { + + bool GenerateDiagnostics = (ActiveScheme != PathDiagnosticConsumer::None); + bool AddPathEdges = (ActiveScheme == PathDiagnosticConsumer::Extensive); + SourceManager &SM = PDB.getSourceManager(); + BugReport *R = PDB.getBugReport(); + AnalyzerOptions &Opts = PDB.getBugReporter().getAnalyzerOptions(); + StackDiagVector CallStack; + InterestingExprs IE; + LocationContextMap LCM; + std::unique_ptr PD = generateEmptyDiagnosticForReport(R, SM); + + if (GenerateDiagnostics) { + auto EndNotes = VisitorsDiagnostics.find(ErrorNode); + std::shared_ptr LastPiece; + if (EndNotes != VisitorsDiagnostics.end()) { + assert(!EndNotes->second.empty()); + LastPiece = EndNotes->second[0]; + } else { + LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, ErrorNode, *R); + } + PD->setEndOfPath(LastPiece); + } + + PathDiagnosticLocation PrevLoc = PD->getLocation(); + const ExplodedNode *NextNode = ErrorNode->getFirstPred(); + while (NextNode) { + if (GenerateDiagnostics) + generatePathDiagnosticsForNode( + NextNode, *PD, PrevLoc, PDB, LCM, CallStack, IE, AddPathEdges); + + auto VisitorNotes = VisitorsDiagnostics.find(NextNode); + NextNode = NextNode->getFirstPred(); + if (!GenerateDiagnostics || VisitorNotes == VisitorsDiagnostics.end()) + continue; + + // This is a workaround due to inability to put shared PathDiagnosticPiece + // into a FoldingSet. + std::set DeduplicationSet; + + // Add pieces from custom visitors. + for (const auto &Note : VisitorNotes->second) { + llvm::FoldingSetNodeID ID; + Note->Profile(ID); + auto P = DeduplicationSet.insert(ID); + if (!P.second) + continue; + + if (AddPathEdges) + addEdgeToPath(PD->getActivePath(), PrevLoc, Note->getLocation(), + PDB.LC); + updateStackPiecesWithMessage(*Note, CallStack); + PD->getActivePath().push_front(Note); + } + } + + if (AddPathEdges) { + // Add an edge to the start of the function. + // We'll prune it out later, but it helps make diagnostics more uniform. + const StackFrameContext *CalleeLC = PDB.LC->getCurrentStackFrame(); + const Decl *D = CalleeLC->getDecl(); + addEdgeToPath(PD->getActivePath(), PrevLoc, + PathDiagnosticLocation::createBegin(D, SM), CalleeLC); + } + + if (!AddPathEdges && GenerateDiagnostics) + CompactPathDiagnostic(PD->getMutablePieces(), SM); + + // Finally, prune the diagnostic path of uninteresting stuff. + if (!PD->path.empty()) { + if (R->shouldPrunePath() && Opts.shouldPrunePaths()) { + bool stillHasNotes = + removeUnneededCalls(PD->getMutablePieces(), R, LCM); + assert(stillHasNotes); + (void)stillHasNotes; + } + + // Redirect all call pieces to have valid locations. + adjustCallLocations(PD->getMutablePieces()); + removePiecesWithInvalidLocations(PD->getMutablePieces()); + + if (AddPathEdges) { + + // Reduce the number of edges from a very conservative set + // to an aesthetically pleasing subset that conveys the + // necessary information. + OptimizedCallsSet OCS; + while (optimizeEdges(PD->getMutablePieces(), SM, OCS, LCM)) {} + + // Drop the very first function-entry edge. It's not really necessary + // for top-level functions. + dropFunctionEntryEdge(PD->getMutablePieces(), LCM, SM); + } + + // Remove messages that are basically the same, and edges that may not + // make sense. + // We have to do this after edge optimization in the Extensive mode. + removeRedundantMsgs(PD->getMutablePieces()); + removeEdgesToDefaultInitializers(PD->getMutablePieces()); + } + return PD; +} + + //===----------------------------------------------------------------------===// // Methods for BugType and subclasses. //===----------------------------------------------------------------------===// @@ -1979,14 +2023,17 @@ llvm::FoldingSetNodeID ID; visitor->Profile(ID); - void *InsertPos; - if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos)) + void *InsertPos = nullptr; + if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos)) { return; + } - CallbacksSet.InsertNode(visitor.get(), InsertPos); Callbacks.push_back(std::move(visitor)); - ++ConfigurationChangeToken; +} + +void BugReport::clearVisitors() { + Callbacks.clear(); } BugReport::~BugReport() { @@ -2032,9 +2079,7 @@ if (!sym) return; - // If the symbol wasn't already in our set, note a configuration change. - if (getInterestingSymbols().insert(sym).second) - ++ConfigurationChangeToken; + getInterestingSymbols().insert(sym); if (const auto *meta = dyn_cast(sym)) getInterestingRegions().insert(meta->getRegion()); @@ -2044,10 +2089,8 @@ if (!R) return; - // If the base region wasn't already in our set, note a configuration change. R = R->getBaseRegion(); - if (getInterestingRegions().insert(R).second) - ++ConfigurationChangeToken; + getInterestingRegions().insert(R); if (const auto *SR = dyn_cast(R)) getInterestingSymbols().insert(SR->getSymbol()); @@ -2487,36 +2530,70 @@ path.insert(path.end(), Pieces.begin(), Pieces.end()); } -bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, - PathDiagnosticConsumer &PC, - ArrayRef &bugReports) { - assert(!bugReports.empty()); +/// Generate notes from all visitors. +/// Notes associated with {@code ErrorNode} are generated using +/// {@code getEndPath}, and the rest are generated with {@code VisitNode}. +static std::unique_ptr +generateVisitorsDiagnostics(BugReport *R, const ExplodedNode *ErrorNode, + BugReporterContext &BRC) { + auto Notes = llvm::make_unique(); + BugReport::VisitorList visitors; + + // Run visitors on all nodes starting from the node *before* the last one. + // The last node is reserved for notes generated with {@code getEndPath}. + const ExplodedNode *NextNode = ErrorNode->getFirstPred(); + while (NextNode) { - bool HasValid = false; - bool HasInvalid = false; - SmallVector errorNodes; - for (const auto I : bugReports) { - if (I->isValid()) { - HasValid = true; - errorNodes.push_back(I->getErrorNode()); - } else { - // Keep the errorNodes list in sync with the bugReports list. - HasInvalid = true; - errorNodes.push_back(nullptr); + // At each iteration, move all visitors from report to visitor list. + for (BugReport::visitor_iterator I = R->visitor_begin(), + E = R->visitor_end(); + I != E; ++I) { + visitors.push_back(std::move(*I)); + } + R->clearVisitors(); + + const ExplodedNode *Pred = NextNode->getFirstPred(); + if (!Pred) { + std::shared_ptr LastPiece; + for (auto &V : visitors) { + V->finalizeVisitor(BRC, ErrorNode, *R); + + if (auto Piece = V->getEndPath(BRC, ErrorNode, *R)) { + assert(!LastPiece && + "There can only be one final piece in a diagnostic."); + LastPiece = std::move(Piece); + llvm::errs() << "Writing to last piece" << "\n"; + (*Notes)[ErrorNode].push_back(LastPiece); + } + } + break; } - } - // If all the reports have been marked invalid by a previous path generation, - // we're done. - if (!HasValid) - return false; + for (auto &V : visitors) { + auto P = V->VisitNode(NextNode, Pred, BRC, *R); + if (P) + (*Notes)[NextNode].push_back(std::move(P)); + } - using PathGenerationScheme = PathDiagnosticConsumer::PathGenerationScheme; + if (!R->isValid()) + break; - PathGenerationScheme ActiveScheme = PC.getGenerationScheme(); + NextNode = Pred; + } - TrimmedGraph TrimG(&getGraph(), errorNodes); - ReportGraph ErrorGraph; + return Notes; +} + +/// Find a non-invalidated report for a given equivalence class, +/// and return together with a cache of visitors notes. +/// If none found, return a nullptr paired with an empty cache. +static +std::pair> findValidReport( + TrimmedGraph &TrimG, + ReportGraph &ErrorGraph, + ArrayRef &bugReports, + AnalyzerOptions &Opts, + GRBugReporter &Reporter) { while (TrimG.popNextReportGraph(ErrorGraph)) { // Find the BugReport with the original location. @@ -2524,15 +2601,12 @@ BugReport *R = bugReports[ErrorGraph.Index]; assert(R && "No original report found for sliced graph."); assert(R->isValid() && "Report selected by trimmed graph marked invalid."); - - // Start building the path diagnostic... - PathDiagnosticBuilder PDB(*this, R, ErrorGraph.BackMap, &PC); - const ExplodedNode *N = ErrorGraph.ErrorNode; + const ExplodedNode *ErrorNode = ErrorGraph.ErrorNode; // Register refutation visitors first, if they mark the bug invalid no // further analysis is required R->addVisitor(llvm::make_unique()); - if (getAnalyzerOptions().shouldCrosscheckWithZ3()) + if (Opts.shouldCrosscheckWithZ3()) R->addVisitor(llvm::make_unique()); // Register additional node visitors. @@ -2540,101 +2614,59 @@ R->addVisitor(llvm::make_unique()); R->addVisitor(llvm::make_unique()); - BugReport::VisitorList visitors; - unsigned origReportConfigToken, finalReportConfigToken; - LocationContextMap LCM; - - // While generating diagnostics, it's possible the visitors will decide - // new symbols and regions are interesting, or add other visitors based on - // the information they find. If they do, we need to regenerate the path - // based on our new report configuration. - do { - // Get a clean copy of all the visitors. - for (BugReport::visitor_iterator I = R->visitor_begin(), - E = R->visitor_end(); I != E; ++I) - visitors.push_back((*I)->clone()); - - // Clear out the active path from any previous work. - PD.resetPath(); - origReportConfigToken = R->getConfigurationChangeToken(); - - // Generate the very last diagnostic piece - the piece is visible before - // the trace is expanded. - std::unique_ptr LastPiece; - for (BugReport::visitor_iterator I = visitors.begin(), E = visitors.end(); - I != E; ++I) { - if (std::unique_ptr Piece = - (*I)->getEndPath(PDB, N, *R)) { - assert(!LastPiece && - "There can only be one final piece in a diagnostic."); - LastPiece = std::move(Piece); - } - } - - if (ActiveScheme != PathDiagnosticConsumer::None) { - if (!LastPiece) - LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R); - assert(LastPiece); - PD.setEndOfPath(std::move(LastPiece)); - } - - // Make sure we get a clean location context map so we don't - // hold onto old mappings. - LCM.clear(); - - generatePathDiagnostics(PD, PDB, N, LCM, visitors, R, ActiveScheme); + BugReporterContext BRC(Reporter, ErrorGraph.BackMap); - // Clean up the visitors we used. - visitors.clear(); + // Run all visitors on a given graph, once. + std::unique_ptr visitorNotes = + generateVisitorsDiagnostics(R, ErrorNode, BRC); - // Did anything change while generating this path? - finalReportConfigToken = R->getConfigurationChangeToken(); - } while (finalReportConfigToken != origReportConfigToken); - - if (!R->isValid()) - continue; - - // Finally, prune the diagnostic path of uninteresting stuff. - if (!PD.path.empty()) { - if (R->shouldPrunePath() && getAnalyzerOptions().shouldPrunePaths()) { - bool stillHasNotes = removeUnneededCalls(PD.getMutablePieces(), R, LCM); - assert(stillHasNotes); - (void)stillHasNotes; - } + if (R->isValid()) + return std::make_pair(R, std::move(visitorNotes)); + } + return std::make_pair(nullptr, llvm::make_unique()); +} - // Redirect all call pieces to have valid locations. - adjustCallLocations(PD.getMutablePieces()); - removePiecesWithInvalidLocations(PD.getMutablePieces()); +std::unique_ptr +GRBugReporter::generatePathDiagnostics( + ArrayRef consumers, + ArrayRef &bugReports) { + assert(!bugReports.empty()); - if (ActiveScheme == PathDiagnosticConsumer::Extensive) { - SourceManager &SM = getSourceManager(); + auto Out = llvm::make_unique(); + bool HasValid = false; + SmallVector errorNodes; + for (const auto I : bugReports) { + if (I->isValid()) { + HasValid = true; + errorNodes.push_back(I->getErrorNode()); + } else { + // Keep the errorNodes list in sync with the bugReports list. + errorNodes.push_back(nullptr); + } + } - // Reduce the number of edges from a very conservative set - // to an aesthetically pleasing subset that conveys the - // necessary information. - OptimizedCallsSet OCS; - while (optimizeEdges(PD.getMutablePieces(), SM, OCS, LCM)) {} + // If all the reports have been marked invalid by a previous path generation, + // we're done. + if (!HasValid) + return Out; - // Drop the very first function-entry edge. It's not really necessary - // for top-level functions. - dropFunctionEntryEdge(PD.getMutablePieces(), LCM, SM); - } + TrimmedGraph TrimG(&getGraph(), errorNodes); + ReportGraph ErrorGraph; + auto ReportInfo = findValidReport(TrimG, ErrorGraph, bugReports, + getAnalyzerOptions(), *this); + BugReport *R = ReportInfo.first; - // Remove messages that are basically the same, and edges that may not - // make sense. - // We have to do this after edge optimization in the Extensive mode. - removeRedundantMsgs(PD.getMutablePieces()); - removeEdgesToDefaultInitializers(PD.getMutablePieces()); + if (R && R->isValid()) { + const ExplodedNode *ErrorNode = ErrorGraph.ErrorNode; + for (PathDiagnosticConsumer *PC : consumers) { + PathDiagnosticBuilder PDB(*this, R, ErrorGraph.BackMap, PC); + std::unique_ptr PD = generatePathDiagnosticForConsumer( + PC->getGenerationScheme(), PDB, ErrorNode, *ReportInfo.second); + (*Out)[PC] = std::move(PD); } - - // We found a report and didn't suppress it. - return true; } - // We suppressed all the reports in this equivalence class. - assert(!HasInvalid && "Inconsistent suppression"); - (void)HasInvalid; - return false; + return Out; } void BugReporter::Register(BugType *BT) { @@ -2893,11 +2925,55 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) { SmallVector bugReports; - BugReport *exampleReport = FindReportInEquivalenceClass(EQ, bugReports); - if (exampleReport) { - for (PathDiagnosticConsumer *PDC : getPathDiagnosticConsumers()) { - FlushReport(exampleReport, *PDC, bugReports); + BugReport *report = FindReportInEquivalenceClass(EQ, bugReports); + if (!report) + return; + + ArrayRef Consumers = getPathDiagnosticConsumers(); + std::unique_ptr Diagnostics = + generateDiagnosticForConsumerMap(report, Consumers, bugReports); + + for (auto &P : *Diagnostics) { + PathDiagnosticConsumer *Consumer = P.first; + std::unique_ptr &PD = P.second; + + // If the path is empty, generate a single step path with the location + // of the issue. + if (PD->path.empty()) { + PathDiagnosticLocation L = report->getLocation(getSourceManager()); + auto piece = llvm::make_unique( + L, report->getDescription()); + for (SourceRange Range : report->getRanges()) + piece->addRange(Range); + PD->setEndOfPath(std::move(piece)); + } + + PathPieces &Pieces = PD->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 = report->getNotes().rbegin(), + E = report->getNotes().rend(); I != E; ++I) { + PathDiagnosticNotePiece *Piece = I->get(); + auto ConvertedPiece = std::make_shared( + Piece->getLocation(), Piece->getString()); + for (const auto &R: Piece->getRanges()) + ConvertedPiece->addRange(R); + + Pieces.push_front(std::move(ConvertedPiece)); + } + } else { + for (auto I = report->getNotes().rbegin(), + E = report->getNotes().rend(); I != E; ++I) + Pieces.push_front(*I); } + + // Get the meta data. + const BugReport::ExtraTextList &Meta = report->getExtraText(); + for (const auto &i : Meta) + PD->addMeta(i); + + Consumer->HandlePathDiagnostic(std::move(PD)); } } @@ -2974,79 +3050,41 @@ return ExecutedLines; } -void BugReporter::FlushReport(BugReport *exampleReport, - PathDiagnosticConsumer &PD, - ArrayRef bugReports) { - // FIXME: Make sure we use the 'R' for the path that was actually used. - // Probably doesn't make a difference in practice. - BugType& BT = exampleReport->getBugType(); - - auto D = llvm::make_unique( - exampleReport->getBugType().getCheckName(), - exampleReport->getDeclWithIssue(), exampleReport->getBugType().getName(), - exampleReport->getDescription(), - exampleReport->getShortDescription(/*Fallback=*/false), BT.getCategory(), - exampleReport->getUniqueingLocation(), exampleReport->getUniqueingDecl(), - findExecutedLines(getSourceManager(), exampleReport->getErrorNode())); - - 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.updateMax(bugReports.size()); - - if (!generatePathDiagnostic(*D.get(), PD, bugReports)) - return; - - MaxValidBugClassSize.updateMax(bugReports.size()); - - // 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(); +std::unique_ptr +BugReporter::generateDiagnosticForConsumerMap( + BugReport *report, ArrayRef consumers, + ArrayRef bugReports) { + + if (!report->isPathSensitive()) { + auto Out = llvm::make_unique(); + for (auto *Consumer : consumers) + (*Out)[Consumer] = generateEmptyDiagnosticForReport(report, + getSourceManager()); + return Out; + } + + // Generate the full path sensitive 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.updateMax(bugReports.size()); + std::unique_ptr Out = + generatePathDiagnostics(consumers, bugReports); + + if (Out->empty()) + return Out; + + MaxValidBugClassSize.updateMax(bugReports.size()); + + // 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(); + for (auto const &P : *Out) if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll) - D->resetDiagnosticLocationToMainFile(); - } - - // If the path is empty, generate a single step path with the location - // of the issue. - if (D->path.empty()) { - PathDiagnosticLocation L = exampleReport->getLocation(getSourceManager()); - auto piece = llvm::make_unique( - L, exampleReport->getDescription()); - for (SourceRange Range : exampleReport->getRanges()) - piece->addRange(Range); - 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(); - auto ConvertedPiece = std::make_shared( - Piece->getLocation(), Piece->getString()); - for (const auto &R: Piece->getRanges()) - ConvertedPiece->addRange(R); - - Pieces.push_front(std::move(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 (const auto &i : Meta) - D->addMeta(i); + P.second->resetDiagnosticLocationToMainFile(); - PD.HandlePathDiagnostic(std::move(D)); + return Out; } void BugReporter::EmitBasicReport(const Decl *DeclWithIssue, Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -177,7 +177,7 @@ // Definitions for bug reporter visitors. //===----------------------------------------------------------------------===// -std::unique_ptr +std::shared_ptr BugReporterVisitor::getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) { return nullptr; @@ -188,7 +188,7 @@ const ExplodedNode *EndPathNode, BugReport &BR) {} -std::unique_ptr BugReporterVisitor::getDefaultEndPath( +std::shared_ptr BugReporterVisitor::getDefaultEndPath( BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) { PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager()); @@ -197,12 +197,12 @@ // Only add the statement itself as a range if we didn't specify any // special ranges for this report. - auto P = llvm::make_unique( + auto P = std::make_shared( L, BR.getDescription(), Ranges.begin() == Ranges.end()); for (SourceRange Range : Ranges) P->addRange(Range); - return std::move(P); + return P; } /// \return name of the macro inside the location \p Loc. @@ -234,8 +234,7 @@ /// for which the region of interest \p RegionOfInterest was passed into, /// but not written inside, and it has caused an undefined read or a null /// pointer dereference outside. -class NoStoreFuncVisitor final - : public BugReporterVisitorImpl { +class NoStoreFuncVisitor final : public BugReporterVisitor { const SubRegion *RegionOfInterest; static constexpr const char *DiagnosticsMsg = "Returning without writing to '"; @@ -526,8 +525,7 @@ } }; -class MacroNullReturnSuppressionVisitor final - : public BugReporterVisitorImpl { +class MacroNullReturnSuppressionVisitor final : public BugReporterVisitor { const SubRegion *RegionOfInterest; public: @@ -609,7 +607,7 @@ /// /// This visitor is intended to be used when another visitor discovers that an /// interesting value comes from an inlined function call. -class ReturnVisitor : public BugReporterVisitorImpl { +class ReturnVisitor : public BugReporterVisitor { const StackFrameContext *StackFrame; enum { Initial, Index: cfe/trunk/test/Analysis/null-deref-path-notes.c =================================================================== --- cfe/trunk/test/Analysis/null-deref-path-notes.c +++ cfe/trunk/test/Analysis/null-deref-path-notes.c @@ -24,14 +24,14 @@ } void f3(char *source) { - char *destination = 0; // FIXME: There should be a note here as well. + char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}} destination = destination + 0; // expected-note{{Null pointer value stored to 'destination'}} memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}} // expected-note@-1{{Null pointer argument in call to memory copy function}} } void f4(char *source) { - char *destination = 0; // FIXME: There should be a note here as well. + char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}} destination = destination - 0; // expected-note{{Null pointer value stored to 'destination'}} memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}} // expected-note@-1{{Null pointer argument in call to memory copy function}}