Author: dergachev Date: Fri Mar 29 15:21:00 2019 New Revision: 357323 URL: http://llvm.org/viewvc/llvm-project?rev=357323&view=rev Log: [analyzer] Introduce a simplified API for adding custom path notes.
Almost all path-sensitive checkers need to tell the user when something specific to that checker happens along the execution path but does not constitute a bug on its own. For instance, a call to operator delete in C++ has consequences that are specific to a use-after-free bug. Deleting an object is not a bug on its own, but when the Analyzer finds an execution path on which a deleted object is used, it'll have to explain to the user when exactly during that path did the deallocation take place. Historically such custom notes were added by implementing "bug report visitors". These visitors were post-processing bug reports by visiting every ExplodedNode along the path and emitting path notes whenever they noticed that a change that is relevant to a bug report occurs within the program state. For example, it emits a "memory is deallocated" note when it notices that a pointer changes its state from "allocated" to "deleted". The "visitor" approach is powerful and efficient but hard to use because such preprocessing implies that the developer first models the effects of the event (say, changes the pointer's state from "allocated" to "deleted" as part of operator delete()'s transfer function) and then forgets what happened and later tries to reverse-engineer itself and figure out what did it do by looking at the report. The proposed approach tries to avoid discarding the information that was available when the transfer function was evaluated. Instead, it allows the developer to capture all the necessary information into a closure that will be automatically invoked later in order to produce the actual note. This should reduce boilerplate and avoid very painful logic duplication. On the technical side, the closure is a lambda that's put into a special kind of a program point tag, and a special bug report visitor visits all nodes in the report and invokes all note-producing closures it finds along the path. For now it is up to the lambda to make sure that the note is actually relevant to the report. For instance, a memory deallocation note would be irrelevant when we're reporting a division by zero bug or if we're reporting a use-after-free of a different, unrelated chunk of memory. The lambda can figure these thing out by looking at the bug report object that's passed into it. A single checker is refactored to make use of the new functionality: MIGChecker. Its program state is trivial, making it an easy testing ground for the first version of the API. Differential Revision: https://reviews.llvm.org/D58367 Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp cfe/trunk/test/Analysis/mig.mm Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original) +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Fri Mar 29 15:21:00 2019 @@ -42,12 +42,11 @@ public: 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 { Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri Mar 29 15:21:00 2019 @@ -592,6 +592,60 @@ public: 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<std::string(BugReporterContext &, BugReport &)>; + +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<std::string> 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<NoteTag>(); + return new (Tag) NoteTag(std::move(Cb)); + } + }; + + friend class TagVisitor; +}; + } // namespace ento } // namespace clang Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Fri Mar 29 15:21:00 2019 @@ -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" @@ -342,6 +343,17 @@ public: 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<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + BugReport &R) override; +}; + namespace bugreporter { /// Attempts to add visitors to track expression value back to its point of Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Fri Mar 29 15:21:00 2019 @@ -219,6 +219,24 @@ public: 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<std::string(BugReport &)> &&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); Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri Mar 29 15:21:00 2019 @@ -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 @@ private: /// 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 @@ public: 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(); } Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp Fri Mar 29 15:21:00 2019 @@ -80,43 +80,10 @@ public: checkReturnAux(RS, C); } - class Visitor : public BugReporterVisitor { - public: - void Profile(llvm::FoldingSetNodeID &ID) const { - static int X = 0; - ID.AddPointer(&X); - } - - std::shared_ptr<PathDiagnosticPiece> 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<PathDiagnosticPiece> -MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, - BugReport &R) { - const auto *NewPVD = static_cast<const ParmVarDecl *>( - N->getState()->get<ReleasedParameter>()); - const auto *OldPVD = static_cast<const ParmVarDecl *>( - N->getFirstPred()->getState()->get<ReleasedParameter>()); - 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<PathDiagnosticEventPiece>(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 @@ void MIGChecker::checkPostCall(const Cal if (!PVD) return; - C.addTransition(C.getState()->set<ReleasedParameter>(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<ReleasedParameter>(true), T); } // Returns true if V can potentially represent a "successful" kern_return_t. @@ -260,7 +236,6 @@ void MIGChecker::checkReturnAux(const Re R->addRange(RS->getSourceRange()); bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); - R->addVisitor(llvm::make_unique<Visitor>()); C.emitReport(std::move(R)); } Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Mar 29 15:21:00 2019 @@ -2612,6 +2612,7 @@ std::pair<BugReport*, std::unique_ptr<Vi R->addVisitor(llvm::make_unique<NilReceiverBRVisitor>()); R->addVisitor(llvm::make_unique<ConditionBRVisitor>()); R->addVisitor(llvm::make_unique<CXXSelfAssignmentBRVisitor>()); + R->addVisitor(llvm::make_unique<TagVisitor>()); BugReporterContext BRC(Reporter, ErrorGraph.BackMap); Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Mar 29 15:21:00 2019 @@ -2492,6 +2492,30 @@ FalsePositiveRefutationBRVisitor::VisitN return nullptr; } +int NoteTag::Kind = 0; + +void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const { + static int Tag = 0; + ID.AddPointer(&Tag); +} + +std::shared_ptr<PathDiagnosticPiece> +TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, + BugReport &R) { + ProgramPoint PP = N->getLocation(); + const NoteTag *T = dyn_cast_or_null<NoteTag>(PP.getTag()); + if (!T) + return nullptr; + + if (Optional<std::string> Msg = T->generateMessage(BRC, R)) { + PathDiagnosticLocation Loc = + PathDiagnosticLocation::create(PP, BRC.getSourceManager()); + return std::make_shared<PathDiagnosticEventPiece>(Loc, *Msg); + } + + return nullptr; +} + void FalsePositiveRefutationBRVisitor::Profile( llvm::FoldingSetNodeID &ID) const { static int Tag = 0; Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Mar 29 15:21:00 2019 @@ -201,7 +201,9 @@ ExprEngine::ExprEngine(cross_tu::CrossTr 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. Modified: cfe/trunk/test/Analysis/mig.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/mig.mm?rev=357323&r1=357322&r2=357323&view=diff ============================================================================== --- cfe/trunk/test/Analysis/mig.mm (original) +++ cfe/trunk/test/Analysis/mig.mm Fri Mar 29 15:21:00 2019 @@ -91,6 +91,14 @@ kern_return_t release_twice(mach_port_na // 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits