Index: clang/include/clang/Analysis/ProgramPoint.h =================================================================== --- clang/include/clang/Analysis/ProgramPoint.h +++ clang/include/clang/Analysis/ProgramPoint.h @@ -42,12 +42,11 @@ virtual ~ProgramPointTag(); virtual StringRef getTagDescription() const = 0; -protected: /// Used to implement 'isKind' in subclasses. - const void *getTagKind() { return TagKind; } + const void *getTagKind() const { return TagKind; } private: - const void *TagKind; + const void *const TagKind; }; class SimpleProgramPointTag : public ProgramPointTag { Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -592,6 +592,61 @@ NodeMapClosure& getNodeResolver() { return NMC; } }; + +/// The tag upon which the TagVisitor reacts. Add these in order to display +/// additional PathDiagnosticEventPieces along the path. +class NoteTag : public ProgramPointTag { +public: + using Callback = + std::function; + +private: + static int Kind; + + const Callback Cb; + + NoteTag(Callback &&Cb) : ProgramPointTag(&Kind), Cb(std::move(Cb)) {} + +public: + static bool classof(const ProgramPointTag *T) { + return T->getTagKind() == &Kind; + } + + Optional generateMessage(BugReporterContext &BRC, + BugReport &R) const { + std::string Msg = Cb(BRC, R); + if (Msg.empty()) + return None; + + return std::move(Msg); + } + + StringRef getTagDescription() const override { + // TODO: Remember a few examples of generated messages + // and display them in the ExplodedGraph dump by + // returning them from this function. + return "Note Tag"; + } + + // Manage memory for NoteTag objects. + class Factory { + llvm::BumpPtrAllocator &Alloc; + + public: + Factory(llvm::BumpPtrAllocator &Alloc) : Alloc(Alloc) {} + + const NoteTag *makeNoteTag(Callback &&Cb) { + // We cannot use make_unique because we cannot access the private + // constructor from inside it. + NoteTag *Tag = Alloc.Allocate(); + return new (Tag) NoteTag(std::move(Cb)); + } + }; + + friend class Factory; + friend class TagVisitor; +}; + } // namespace ento } // namespace clang Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H #define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H +#include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" @@ -328,6 +329,17 @@ BugReport &BR) override; }; + +/// The visitor detects NoteTags and displays the event notes they contain. +class TagVisitor : public BugReporterVisitor { +public: + void Profile(llvm::FoldingSetNodeID &ID) const override; + + std::shared_ptr VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + BugReport &R) override; +}; + namespace bugreporter { /// Attempts to add visitors to track expression value back to its point of Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -219,6 +219,24 @@ Eng.getBugReporter().emitReport(std::move(R)); } + + /// Produce a program point tag that displays an additional path note + /// to the user. This is a lightweight alternative to the + /// BugReporterVisitor mechanism: instead of visiting the bug report + /// node-by-node to restore the sequence of events that led to discovering + /// a bug, you can add notes as you add your transitions. + const NoteTag *getNoteTag(NoteTag::Callback &&Cb) { + return Eng.getNoteTags().makeNoteTag(std::move(Cb)); + } + + /// A shorthand version of getNoteTag that doesn't require you to accept + /// the BugReporterContext arguments when you don't need it. + const NoteTag *getNoteTag(std::function &&Cb) { + return getNoteTag( + [Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); }); + } + + /// Returns the word that should be used to refer to the declaration /// in the report. StringRef getDeclDescription(const Decl *D); Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -22,6 +22,7 @@ #include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h" #include "clang/StaticAnalyzer/Core/PathSensitive/FunctionSummary.h" @@ -155,6 +156,8 @@ /// The flag, which specifies the mode of inlining for the engine. InliningModes HowToInline; + NoteTag::Factory NoteTags; + public: ExprEngine(cross_tu::CrossTranslationUnitContext &CTU, AnalysisManager &mgr, SetOfConstDecls *VisitedCalleesIn, @@ -396,6 +399,8 @@ SymbolManager &getSymbolManager() { return SymMgr; } MemRegionManager &getRegionManager() { return MRMgr; } + NoteTag::Factory &getNoteTags() { return NoteTags; } + // Functions for external checking of whether we have unfinished work bool wasBlocksExhausted() const { return Engine.wasBlocksExhausted(); } Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -80,43 +80,10 @@ checkReturnAux(RS, C); } - class Visitor : public BugReporterVisitor { - public: - void Profile(llvm::FoldingSetNodeID &ID) const { - static int X = 0; - ID.AddPointer(&X); - } - - std::shared_ptr VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, BugReport &R); - }; }; } // end anonymous namespace -// FIXME: It's a 'const ParmVarDecl *' but there's no ready-made GDM traits -// specialization for this sort of types. -REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, const void *) - -std::shared_ptr -MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, - BugReport &R) { - const auto *NewPVD = static_cast( - N->getState()->get()); - const auto *OldPVD = static_cast( - N->getFirstPred()->getState()->get()); - if (OldPVD == NewPVD) - return nullptr; - - assert(NewPVD && "What is deallocated cannot be un-deallocated!"); - SmallString<64> Str; - llvm::raw_svector_ostream OS(Str); - OS << "Value passed through parameter '" << NewPVD->getName() - << "' is deallocated"; - - PathDiagnosticLocation Loc = - PathDiagnosticLocation::create(N->getLocation(), BRC.getSourceManager()); - return std::make_shared(Loc, OS.str()); -} +REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool) static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C) { SymbolRef Sym = V.getAsSymbol(); @@ -195,7 +162,16 @@ if (!PVD) return; - C.addTransition(C.getState()->set(PVD)); + const NoteTag *T = C.getNoteTag([this, PVD](BugReport &BR) -> std::string { + if (&BR.getBugType() != &BT) + return ""; + SmallString<64> Str; + llvm::raw_svector_ostream OS(Str); + OS << "Value passed through parameter '" << PVD->getName() + << "\' is deallocated"; + return OS.str(); + }); + C.addTransition(C.getState()->set(true), T); } // Returns true if V can potentially represent a "successful" kern_return_t. @@ -260,7 +236,6 @@ R->addRange(RS->getSourceRange()); bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); - R->addVisitor(llvm::make_unique()); C.emitReport(std::move(R)); } Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -2612,6 +2612,7 @@ R->addVisitor(llvm::make_unique()); R->addVisitor(llvm::make_unique()); R->addVisitor(llvm::make_unique()); + R->addVisitor(llvm::make_unique()); BugReporterContext BRC(Reporter, ErrorGraph.BackMap); Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -2480,6 +2480,30 @@ return nullptr; } +int NoteTag::Kind = 0; + +void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const { + static int Tag = 0; + ID.AddPointer(&Tag); +} + +std::shared_ptr +TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, + BugReport &R) { + ProgramPoint PP = N->getLocation(); + const NoteTag *T = dyn_cast_or_null(PP.getTag()); + if (!T) + return nullptr; + + if (Optional Msg = T->generateMessage(BRC, R)) { + PathDiagnosticLocation Loc = + PathDiagnosticLocation::create(PP, BRC.getSourceManager()); + return std::make_shared(Loc, *Msg); + } + + return nullptr; +} + void FalsePositiveRefutationBRVisitor::Profile( llvm::FoldingSetNodeID &ID) const { static int Tag = 0; Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -201,7 +201,9 @@ svalBuilder(StateMgr.getSValBuilder()), ObjCNoRet(mgr.getASTContext()), BR(mgr, *this), - VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) { + VisitedCallees(VisitedCalleesIn), + HowToInline(HowToInlineIn), + NoteTags(G.getAllocator()) { unsigned TrimInterval = mgr.options.GraphTrimInterval; if (TrimInterval != 0) { // Enable eager node reclamation when constructing the ExplodedGraph. Index: clang/test/Analysis/mig.mm =================================================================== --- clang/test/Analysis/mig.mm +++ clang/test/Analysis/mig.mm @@ -91,6 +91,14 @@ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} } +MIG_SERVER_ROUTINE +kern_return_t no_unrelated_notes(mach_port_name_t port, vm_address_t address, vm_size_t size) { + vm_deallocate(port, address, size); // no-note + 1 / 0; // expected-warning{{Division by zero}} + // expected-note@-1{{Division by zero}} + return KERN_SUCCESS; +} + // Make sure we find the bug when the object is destroyed within an // automatic destructor. MIG_SERVER_ROUTINE