Author: dergachev Date: Fri Apr 19 13:23:29 2019 New Revision: 358781 URL: http://llvm.org/viewvc/llvm-project?rev=358781&view=rev Log: Reapply "[analyzer] Introduce a simplified API for adding custom path notes."
This reapplies commit r357323, fixing memory leak found by LSan. 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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original) +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Fri Apr 19 13:23:29 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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri Apr 19 13:23:29 2019 @@ -592,6 +592,59 @@ 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 { + std::vector<std::unique_ptr<NoteTag>> Tags; + + public: + const NoteTag *makeNoteTag(Callback &&Cb) { + // We cannot use make_unique because we cannot access the private + // constructor from inside it. + std::unique_ptr<NoteTag> T(new NoteTag(std::move(Cb))); + Tags.push_back(std::move(T)); + return Tags.back().get(); + } + }; + + 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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Fri Apr 19 13:23:29 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" @@ -328,6 +329,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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Fri Apr 19 13:23:29 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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri Apr 19 13:23:29 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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp Fri Apr 19 13:23:29 2019 @@ -102,43 +102,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(); @@ -217,7 +184,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. @@ -282,7 +258,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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Apr 19 13:23:29 2019 @@ -2611,6 +2611,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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Apr 19 13:23:29 2019 @@ -2499,6 +2499,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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Apr 19 13:23:29 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) + { 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=358781&r1=358780&r2=358781&view=diff ============================================================================== --- cfe/trunk/test/Analysis/mig.mm (original) +++ cfe/trunk/test/Analysis/mig.mm Fri Apr 19 13:23:29 2019 @@ -100,6 +100,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
