diff options
22 files changed, 805 insertions, 909 deletions
diff --git a/Driver/AnalysisConsumer.cpp b/Driver/AnalysisConsumer.cpp index 49ebde71c8..73ce3f6625 100644 --- a/Driver/AnalysisConsumer.cpp +++ b/Driver/AnalysisConsumer.cpp @@ -415,7 +415,7 @@ static void ActionGRExprEngine(AnalysisManager& mgr, GRTransferFuncs* tf, LiveVariables* L = mgr.getLiveVariables(); if (!L) return; - GRExprEngine Eng(*mgr.getCFG(), *mgr.getCodeDecl(), mgr.getContext(), *L, + GRExprEngine Eng(*mgr.getCFG(), *mgr.getCodeDecl(), mgr.getContext(), *L, mgr, PurgeDead, mgr.getStoreManagerCreator(), mgr.getConstraintManagerCreator()); @@ -442,7 +442,7 @@ static void ActionGRExprEngine(AnalysisManager& mgr, GRTransferFuncs* tf, ExplodedNodeImpl::SetAuditor(0); // Display warnings. - Eng.EmitWarnings(mgr); + Eng.getBugReporter().FlushReports(); // Visualize the exploded graph. if (mgr.shouldVisualizeGraphviz()) diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h index bcc28bdc5c..e0c3792323 100644 --- a/include/clang/Analysis/PathSensitive/BugReporter.h +++ b/include/clang/Analysis/PathSensitive/BugReporter.h @@ -23,6 +23,7 @@ #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/ImmutableSet.h" #include <list> namespace clang { @@ -36,100 +37,143 @@ class BugReporter; class GRExprEngine; class GRState; class Stmt; -class BugReport; +class BugType; class ParentMap; -class BugType { -public: - BugType() {} - virtual ~BugType(); +//===----------------------------------------------------------------------===// +// Interface for individual bug reports. +//===----------------------------------------------------------------------===// - virtual const char* getName() const = 0; - virtual const char* getDescription() const { return getName(); } - virtual const char* getCategory() const { return ""; } +// FIXME: Combine this with RangedBugReport and remove RangedBugReport. +class BugReport { + BugType& BT; + std::string Description; + const ExplodedNode<GRState> *EndNode; + SourceRange R; - virtual std::pair<const char**,const char**> getExtraDescriptiveText() { - return std::pair<const char**, const char**>(0, 0); +protected: + friend class BugReporter; + friend class BugReportEquivClass; + + virtual void Profile(llvm::FoldingSetNodeID& hash) const { + hash.AddInteger(getLocation().getRawEncoding()); } - - virtual void EmitWarnings(BugReporter& BR) {} - virtual void GetErrorNodes(std::vector<ExplodedNode<GRState>*>& Nodes) {} - virtual bool isCached(BugReport& R) = 0; -}; - -class BugTypeCacheLocation : public BugType { - llvm::SmallSet<ProgramPoint,10> CachedErrors; -public: - BugTypeCacheLocation() {} - virtual ~BugTypeCacheLocation() {} - virtual bool isCached(BugReport& R); - bool isCached(ProgramPoint P); -}; - - -class BugReport { - BugType& Desc; - const ExplodedNode<GRState> *EndNode; - SourceRange R; public: - BugReport(BugType& D, const ExplodedNode<GRState> *n) : Desc(D), EndNode(n) {} + BugReport(BugType& bt, const char* desc, const ExplodedNode<GRState> *n) + : BT(bt), Description(desc), EndNode(n) {} + virtual ~BugReport(); + + const BugType& getBugType() const { return BT; } + BugType& getBugType() { return BT; } - const BugType& getBugType() const { return Desc; } - BugType& getBugType() { return Desc; } - + // FIXME: Perhaps this should be moved into a subclass? const ExplodedNode<GRState>* getEndNode() const { return EndNode; } + // FIXME: Do we need this? Maybe getLocation() should return a ProgramPoint + // object. Stmt* getStmt(BugReporter& BR) const; - - const char* getName() const { return getBugType().getName(); } - - virtual const char* getDescription() const { - return getBugType().getDescription(); - } - virtual const char* getCategory() const { - return getBugType().getCategory(); - } + const std::string& getDescription() const { return Description; } + // FIXME: Is this needed? virtual std::pair<const char**,const char**> getExtraDescriptiveText() { - return getBugType().getExtraDescriptiveText(); + return std::make_pair((const char**)0,(const char**)0); } + // FIXME: Perhaps move this into a subclass. virtual PathDiagnosticPiece* getEndPath(BugReporter& BR, const ExplodedNode<GRState>* N); - virtual FullSourceLoc getLocation(SourceManager& Mgr); + /// getLocation - Return the "definitive" location of the reported bug. + /// While a bug can span an entire path, usually there is a specific + /// location that can be used to identify where the key issue occured. + /// This location is used by clients rendering diagnostics. + virtual SourceLocation getLocation() const; + /// getRanges - Returns the source ranges associated with this bug. virtual void getRanges(BugReporter& BR,const SourceRange*& beg, const SourceRange*& end); - + + // FIXME: Perhaps this should be moved into a subclass? virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N, const ExplodedNode<GRState>* PrevN, const ExplodedGraph<GRState>& G, BugReporter& BR); }; + +//===----------------------------------------------------------------------===// +// BugTypes (collections of related reports). +//===----------------------------------------------------------------------===// + +class BugReportEquivClass : public llvm::FoldingSetNode { + // List of *owned* BugReport objects. + std::list<BugReport*> Reports; + + friend class BugReporter; + void AddReport(BugReport* R) { Reports.push_back(R); } +public: + BugReportEquivClass(BugReport* R) { Reports.push_back(R); } + ~BugReportEquivClass(); + + void Profile(llvm::FoldingSetNodeID& ID) const { + assert(!Reports.empty()); + (*Reports.begin())->Profile(ID); + } + + class iterator { + std::list<BugReport*>::iterator impl; + public: + iterator(std::list<BugReport*>::iterator i) : impl(i) {} + iterator& operator++() { ++impl; return *this; } + bool operator==(const iterator& I) const { return I.impl == impl; } + bool operator!=(const iterator& I) const { return I.impl != impl; } + BugReport* operator*() const { return *impl; } + BugReport* operator->() const { return *impl; } + }; + + iterator begin() { return iterator(Reports.begin()); } + iterator end() { return iterator(Reports.end()); } +}; + +class BugType { +private: + const std::string Name; + const std::string Category; + llvm::FoldingSet<BugReportEquivClass> EQClasses; + friend class BugReporter; +public: + BugType(const char *name, const char* cat) : Name(name), Category(cat) {} + virtual ~BugType(); + // FIXME: Should these be made strings as well? + const std::string& getName() const { return Name; } + const std::string& getCategory() const { return Category; } + + virtual void FlushReports(BugReporter& BR); + void AddReport(BugReport* BR); +}; + +//===----------------------------------------------------------------------===// +// Specialized subclasses of BugReport. +//===----------------------------------------------------------------------===// + +// FIXME: Collapse this with the default BugReport class. class RangedBugReport : public BugReport { std::vector<SourceRange> Ranges; - const char* desc; public: - RangedBugReport(BugType& D, ExplodedNode<GRState> *n, - const char* description = 0) - : BugReport(D, n), desc(description) {} + RangedBugReport(BugType& D, const char* description, ExplodedNode<GRState> *n) + : BugReport(D, description, n) {} - virtual ~RangedBugReport(); + ~RangedBugReport(); - virtual const char* getDescription() const { - return desc ? desc : BugReport::getDescription(); - } - + // FIXME: Move this out of line. void addRange(SourceRange R) { Ranges.push_back(R); } - - virtual void getRanges(BugReporter& BR,const SourceRange*& beg, - const SourceRange*& end) { + // FIXME: Move this out of line. + void getRanges(BugReporter& BR,const SourceRange*& beg, + const SourceRange*& end) { if (Ranges.empty()) { beg = NULL; @@ -142,6 +186,10 @@ public: } }; +//===----------------------------------------------------------------------===// +// BugReporter and friends. +//===----------------------------------------------------------------------===// + class BugReporterData { public: virtual ~BugReporterData(); @@ -158,16 +206,25 @@ class BugReporter { public: enum Kind { BaseBRKind, GRBugReporterKind }; -protected: - Kind kind; +private: + typedef llvm::ImmutableSet<BugType*> BugTypesTy; + BugTypesTy::Factory F; + BugTypesTy BugTypes; + + const Kind kind; BugReporterData& D; - BugReporter(BugReporterData& d, Kind k) : kind(k), D(d) {} - + void FlushReport(BugReportEquivClass& EQ); + +protected: + BugReporter(BugReporterData& d, Kind k) : BugTypes(F.GetEmptySet()), kind(k), D(d) {} + public: - BugReporter(BugReporterData& d) : kind(BaseBRKind), D(d) {} + BugReporter(BugReporterData& d) : BugTypes(F.GetEmptySet()), kind(BaseBRKind), D(d) {} virtual ~BugReporter(); + void FlushReports(); + Kind getKind() const { return kind; } Diagnostic& getDiagnostic() { @@ -178,29 +235,22 @@ public: return D.getPathDiagnosticClient(); } - ASTContext& getContext() { - return D.getContext(); - } + ASTContext& getContext() { return D.getContext(); } - SourceManager& getSourceManager() { - return D.getSourceManager(); - } + SourceManager& getSourceManager() { return D.getSourceManager(); } - CFG* getCFG() { - return D.getCFG(); - } + CFG* getCFG() { return D.getCFG(); } - ParentMap& getParentMap() { - return D.getParentMap(); - } + ParentMap& getParentMap() { return D.getParentMap(); } - LiveVariables* getLiveVariables() { - return D.getLiveVariables(); - } + LiveVariables* getLiveVariables() { return D.getLiveVariables(); } - virtual void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R) {} + virtual void GeneratePathDiagnostic(PathDiagnostic& PD, + BugReportEquivClass& EQ) {} - void EmitWarning(BugReport& R); + void Register(BugType *BT); + + void EmitReport(BugReport *R); void EmitBasicReport(const char* BugName, const char* BugStr, SourceLocation Loc, @@ -234,11 +284,11 @@ public: static bool classof(const BugReporter* R) { return true; } }; +// FIXME: Get rid of GRBugReporter. It's the wrong abstraction. class GRBugReporter : public BugReporter { GRExprEngine& Eng; llvm::SmallSet<SymbolRef, 10> NotableSymbols; -public: - +public: GRBugReporter(BugReporterData& d, GRExprEngine& eng) : BugReporter(d, GRBugReporterKind), Eng(eng) {} @@ -246,9 +296,7 @@ public: /// getEngine - Return the analysis engine used to analyze a given /// function or method. - GRExprEngine& getEngine() { - return Eng; - } + GRExprEngine& getEngine() { return Eng; } /// getGraph - Get the exploded graph created by the analysis engine /// for the analyzed method or function. @@ -258,7 +306,8 @@ public: /// engine. GRStateManager& getStateManager(); - virtual void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R); + virtual void GeneratePathDiagnostic(PathDiagnostic& PD, + BugReportEquivClass& R); void addNotableSymbol(SymbolRef Sym) { NotableSymbols.insert(Sym); @@ -273,22 +322,18 @@ public: return R->getKind() == GRBugReporterKind; } }; - class DiagBugReport : public RangedBugReport { std::list<std::string> Strs; FullSourceLoc L; - const char* description; public: - DiagBugReport(const char* desc, BugType& D, FullSourceLoc l) : - RangedBugReport(D, NULL), L(l), description(desc) {} + DiagBugReport(BugType& D, const char* desc, FullSourceLoc l) : + RangedBugReport(D, desc, 0), L(l) {} virtual ~DiagBugReport() {} - virtual FullSourceLoc getLocation(SourceManager&) { return L; } - virtual const char* getDescription() const { - return description; - } + // FIXME: Move out-of-line (virtual function). + SourceLocation getLocation() const { return L; } void addString(const std::string& s) { Strs.push_back(s); } @@ -297,39 +342,6 @@ public: str_iterator str_end() const { return Strs.end(); } }; -class DiagCollector : public DiagnosticClient { - std::list<DiagBugReport> Reports; - BugType& D; -public: - DiagCollector(BugType& d) : D(d) {} - - virtual ~DiagCollector() {} - - bool IncludeInDiagnosticCounts() const { return false; } - - void HandleDiagnostic(Diagnostic::Level DiagLevel, - const DiagnosticInfo &Info); - - // Iterators. - typedef std::list<DiagBugReport>::iterator iterator; - iterator begin() { return Reports.begin(); } - iterator end() { return Reports.end(); } -}; - -class SimpleBugType : public BugTypeCacheLocation { - const char* name; - const char* category; - const char* desc; -public: - SimpleBugType(const char* n) : name(n), category(""), desc(0) {} - SimpleBugType(const char* n, const char* c, const char* d) - : name(n), category(c), desc(d) {} - - virtual const char* getName() const { return name; } - virtual const char* getDescription() const { return desc ? desc : name; } - virtual const char* getCategory() const { return category; } -}; - } // end clang namespace #endif diff --git a/include/clang/Analysis/PathSensitive/ExplodedGraph.h b/include/clang/Analysis/PathSensitive/ExplodedGraph.h index 954b096b8e..757efdc64c 100644 --- a/include/clang/Analysis/PathSensitive/ExplodedGraph.h +++ b/include/clang/Analysis/PathSensitive/ExplodedGraph.h @@ -39,6 +39,12 @@ class GRIndirectGotoNodeBuilderImpl; class GRSwitchNodeBuilderImpl; class GREndPathNodebuilderImpl; +//===----------------------------------------------------------------------===// +// ExplodedGraph "implementation" classes. These classes are not typed to +// contain a specific kind of state. Typed-specialized versions are defined +// on top of these classes. +//===----------------------------------------------------------------------===// + class ExplodedNodeImpl : public llvm::FoldingSetNode { protected: friend class ExplodedGraphImpl; @@ -204,6 +210,7 @@ public: } }; +class InterExplodedGraphMapImpl; class ExplodedGraphImpl { protected: @@ -289,9 +296,39 @@ public: return llvm::dyn_cast<FunctionDecl>(&CodeDecl); } + typedef llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*> NodeMap; + ExplodedGraphImpl* Trim(const ExplodedNodeImpl* const * NBeg, - const ExplodedNodeImpl* const * NEnd) const; + const ExplodedNodeImpl* const * NEnd, + InterExplodedGraphMapImpl *M) const; +}; + +class InterExplodedGraphMapImpl { + llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*> M; + friend class ExplodedGraphImpl; + void add(const ExplodedNodeImpl* From, ExplodedNodeImpl* To); + +protected: + ExplodedNodeImpl* getMappedImplNode(const ExplodedNodeImpl* N) const; + + InterExplodedGraphMapImpl(); +public: + virtual ~InterExplodedGraphMapImpl() {} +}; + +//===----------------------------------------------------------------------===// +// Type-specialized ExplodedGraph classes. +//===----------------------------------------------------------------------===// +template <typename STATE> +class InterExplodedGraphMap : public InterExplodedGraphMapImpl { +public: + InterExplodedGraphMap() {}; + ~InterExplodedGraphMap() {}; + + ExplodedNode<STATE>* getMappedNode(const ExplodedNode<STATE>* N) const { + return static_cast<ExplodedNode<STATE>*>(getMappedImplNode(N)); + } }; template <typename STATE> @@ -409,13 +446,12 @@ public: return const_cast<ExplodedGraph>(this)->eop_end(); } - // Utility. - - ExplodedGraph* + std::pair<ExplodedGraph*, InterExplodedGraphMap<STATE>*> Trim(const NodeTy* const* NBeg, const NodeTy* const* NEnd) const { if (NBeg == NEnd) - return NULL; + return std::make_pair((ExplodedGraph*) 0, + (InterExplodedGraphMap<STATE>*) 0); assert (NBeg < NEnd); @@ -424,12 +460,15 @@ public: const ExplodedNodeImpl* const* NEndImpl = (const ExplodedNodeImpl* const*) NEnd; - return static_cast<ExplodedGraph*>(ExplodedGraphImpl::Trim(NBegImpl, - NEndImpl)); + llvm::OwningPtr<InterExplodedGraphMap<STATE> > + M(new InterExplodedGraphMap<STATE>()); + + ExplodedGraphImpl* G = ExplodedGraphImpl::Trim(NBegImpl, NEndImpl, M.get()); + + return std::make_pair(static_cast<ExplodedGraph*>(G), M.take()); } }; - - + template <typename StateTy> class ExplodedNodeSet { diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index 2a72794dd7..9a4ed49ffd 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -20,18 +20,16 @@ #include "clang/Analysis/PathSensitive/GRState.h" #include "clang/Analysis/PathSensitive/GRSimpleAPICheck.h" #include "clang/Analysis/PathSensitive/GRTransferFuncs.h" +#include "clang/Analysis/PathSensitive/BugReporter.h" #include "clang/AST/Type.h" #include "clang/AST/ExprObjC.h" namespace clang { - class BugType; class PathDiagnosticClient; class Diagnostic; - class BugReporterData; -class GRExprEngine { - +class GRExprEngine { public: typedef GRState StateTy; typedef ExplodedGraph<StateTy> GraphTy; @@ -44,7 +42,6 @@ public: typedef GRSwitchNodeBuilder<StateTy> SwitchNodeBuilder; typedef GREndPathNodeBuilder<StateTy> EndPathNodeBuilder; typedef ExplodedNodeSet<StateTy> NodeSet; - protected: GRCoreEngine<GRExprEngine> CoreEngine; @@ -62,11 +59,7 @@ protected: /// StateMgr - Object that manages the data for all created states. GRStateManager StateMgr; - - /// BugTypes - Objects used for reporting bugs. - typedef std::vector<BugType*> BugTypeSet; - BugTypeSet BugTypes; - + /// SymMgr - Object that manages the symbol information. SymbolManager& SymMgr; @@ -92,6 +85,11 @@ protected: /// PurgeDead - Remove dead bindings before processing a statement. bool PurgeDead; + /// BR - The BugReporter associated with this engine. It is important that + // this object be placed at the very end of member variables so that its + // destructor is called before the rest of the GRExprEngine is destroyed. + GRBugReporter BR; + public: typedef llvm::SmallPtrSet<NodeTy*,2> ErrorNodes; typedef llvm::DenseMap<NodeTy*, Expr*> UndefArgsTy; @@ -177,10 +175,11 @@ public: public: GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx, LiveVariables& L, + BugReporterData& BRD, bool purgeDead, StoreManagerCreator SMC = CreateBasicStoreManager, ConstraintManagerCreator CMC = CreateBasicConstraintManager); - + ~GRExprEngine(); void ExecuteWorkList(unsigned Steps = 150000) { @@ -195,6 +194,8 @@ public: GRTransferFuncs& getTF() { return *StateMgr.TF; } + BugReporter& getBugReporter() { return BR; } + /// setTransferFunctions void setTransferFunctions(GRTransferFuncs* tf); @@ -219,27 +220,9 @@ public: GraphTy& getGraph() { return G; } const GraphTy& getGraph() const { return G; } - - typedef BugTypeSet::iterator bug_type_iterator; - typedef BugTypeSet::const_iterator const_bug_type_iterator; - - bug_type_iterator bug_types_begin() { return BugTypes.begin(); } - bug_type_iterator bug_types_end() { return BugTypes.end(); } - const_bug_type_iterator bug_types_begin() const { return BugTypes.begin(); } - const_bug_type_iterator bug_types_end() const { return BugTypes.end(); } - - /// Register - Register a BugType with the analyzer engine. A registered - /// BugType object will have its 'EmitWarnings' method called when the - /// the analyzer finishes analyzing a method or function. - void Register(BugType* B) { - BugTypes.push_back(B); - } - void RegisterInternalChecks(); - void EmitWarnings(BugReporterData& BRData); - bool isRetStackAddr(const NodeTy* N) const { return N->isSink() && RetsStackAddr.count(const_cast<NodeTy*>(N)) != 0; } diff --git a/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h b/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h index 66b2fb1ae8..e54b31dfe8 100644 --- a/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h +++ b/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h @@ -33,7 +33,6 @@ class GRSimpleAPICheck : public GRAuditor<GRState> { public: GRSimpleAPICheck() {} virtual ~GRSimpleAPICheck() {} - virtual void EmitWarnings(BugReporter& BR) = 0; }; } // end namespace clang diff --git a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h index 6f4266b9df..dba42c2515 100644 --- a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h +++ b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h @@ -23,28 +23,24 @@ namespace clang { class GRExprEngine; + class BugReporter; class ObjCMessageExpr; class GRTransferFuncs { - - friend class GRExprEngine; - + friend class GRExprEngine; protected: - - virtual SVal DetermEvalBinOpNN(GRExprEngine& Eng, BinaryOperator::Opcode Op, NonLoc L, NonLoc R) { return UnknownVal(); } - public: GRTransferFuncs() {} virtual ~GRTransferFuncs() {} virtual void RegisterPrinters(std::vector<GRState::Printer*>& Printers) {} - virtual void RegisterChecks(GRExprEngine& Eng); + virtual void RegisterChecks(BugReporter& BR) {} // Casts. diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp index be729bee6c..8a5c3e0a84 100644 --- a/lib/Analysis/BasicObjCFoundationChecks.cpp +++ b/lib/Analysis/BasicObjCFoundationChecks.cpp @@ -23,15 +23,12 @@ #include "clang/Analysis/PathSensitive/MemRegion.h" #include "clang/Analysis/PathDiagnostic.h" #include "clang/Analysis/LocalCheckers.h" - #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/ASTContext.h" #include "llvm/Support/Compiler.h" -#include <sstream> - using namespace clang; static ObjCInterfaceType* GetReceiverType(ObjCMessageExpr* ME) { @@ -60,64 +57,17 @@ static const char* GetReceiverNameType(ObjCMessageExpr* ME) { namespace { -class VISIBILITY_HIDDEN APIMisuse : public BugTypeCacheLocation { -public: - const char* getCategory() const { - return "API Misuse (Apple)"; - } -}; - -class VISIBILITY_HIDDEN NilArg : public APIMisuse { +class VISIBILITY_HIDDEN APIMisuse : public BugType { public: - virtual ~NilArg() {} - - virtual const char* getName() const { - return "nil argument"; - } - - class Report : public BugReport { - std::string Msg; - const char* s; - SourceRange R; - public: - - Report(NilArg& Desc, ExplodedNode<GRState>* N, - ObjCMessageExpr* ME, unsigned Arg) - : BugReport(Desc, N) { - - Expr* E = ME->getArg(Arg); - R = E->getSourceRange(); - - std::ostringstream os; - - os << "Argument to '" << GetReceiverNameType(ME) << "' method '" - << ME->getSelector().getAsString() << "' cannot be nil."; - - Msg = os.str(); - s = Msg.c_str(); - } - - virtual ~Report() {} - - virtual const char* getDescription() const { return s; } - - virtual void getRanges(BugReporter& BR, - const SourceRange*& B, const SourceRange*& E) { - B = &R; - E = B+1; - } - }; + APIMisuse(const char* name) : BugType(name, "API Misuse (Apple)") {} }; - class VISIBILITY_HIDDEN BasicObjCFoundationChecks : public GRSimpleAPICheck { - NilArg Desc; + APIMisuse *BT; + BugReporter& BR; ASTContext &Ctx; GRStateManager* VMgr; - - typedef std::vector<BugReport*> ErrorsTy; - ErrorsTy Errors; - + SVal GetSVal(const GRState* St, Expr* E) { return VMgr->GetSVal(St, E); } bool isNSString(ObjCInterfaceType* T, const char* suffix); @@ -129,26 +79,26 @@ class VISIBILITY_HIDDEN BasicObjCFoundationChecks : public GRSimpleAPICheck { bool CheckNilArg(NodeTy* N, unsigned Arg); public: - BasicObjCFoundationChecks(ASTContext& ctx, GRStateManager* vmgr) - : Ctx(ctx), VMgr(vmgr) {} - - virtual ~BasicObjCFoundationChecks() { - for (ErrorsTy::iterator I = Errors.begin(), E = Errors.end(); I!=E; ++I) - delete *I; - } - - virtual bool Audit(ExplodedNode<GRState>* N, GRStateManager&); - - virtual void EmitWarnings(BugReporter& BR); - -private: - - void AddError(BugReport* R) { - Errors.push_back(R); - } - - void WarnNilArg(NodeTy* N, ObjCMessageExpr* ME, unsigned Arg) { - AddError(new NilArg::Report(Desc, N, ME, Arg)); + BasicObjCFoundationChecks(ASTContext& ctx, GRStateManager* vmgr, + BugReporter& br) + : BT(0), BR(br), Ctx(ctx), VMgr(vmgr) {} + + bool Audit(ExplodedNode<GRState>* N, GRStateManager&); + +private: + void WarnNilArg(NodeTy* N, ObjCMessageExpr* ME, unsigned Arg) { + std::string sbuf; + llvm::raw_string_ostream os(sbuf); + os << "Argument to '" << GetReceiverNameType(ME) << "' method '" + << ME->getSelector().getAsString() << "' cannot be nil."; + + // Lazily create the BugType object for NilArg. This will be owned + // by the BugReporter object 'BR' once we call BR.EmitWarning. + if (!BT) BT = new APIMisuse("nil argument"); + + RangedBugReport *R = new RangedBugReport(*BT, os.str().c_str(), N); + R->addRange(ME->getArg(Arg)->getSourceRange()); + BR.EmitReport(R); } }; @@ -157,9 +107,9 @@ private: GRSimpleAPICheck* clang::CreateBasicObjCFoundationChecks(ASTContext& Ctx, - GRStateManager* VMgr) { + GRStateManager* VMgr, BugReporter& BR) { - return new BasicObjCFoundationChecks(Ctx, VMgr); + return new BasicObjCFoundationChecks(Ctx, VMgr, BR); } @@ -201,13 +151,6 @@ static inline bool isNil(SVal X) { // Error reporting. //===----------------------------------------------------------------------===// - -void BasicObjCFoundationChecks::EmitWarnings(BugReporter& BR) { - - for (ErrorsTy::iterator I=Errors.begin(), E=Errors.end(); I!=E; ++I) - BR.EmitWarning(**I); -} - bool BasicObjCFoundationChecks::CheckNilArg(NodeTy* N, unsigned Arg) { ObjCMessageExpr* ME = cast<ObjCMessageExpr>(cast<PostStmt>(N->getLocation()).getStmt()); @@ -307,41 +250,9 @@ bool BasicObjCFoundationChecks::AuditNSString(NodeTy* N, //===----------------------------------------------------------------------===// namespace { - -class VISIBILITY_HIDDEN BadCFNumberCreate : public APIMisuse{ -public: - typedef std::vector<BugReport*> AllErrorsTy; - AllErrorsTy AllErrors; - - virtual const char* getName() const { - return "Bad use of CFNumberCreate"; - } - - virtual void EmitWarnings(BugReporter& BR) { - // FIXME: Refactor this method. - for (AllErrorsTy::iterator I=AllErrors.begin(), E=AllErrors.end(); I!=E;++I) - BR.EmitWarning(**I); - } -}; - // FIXME: This entire class should be refactored into the common - // BugReporter classes. -class VISIBILITY_HIDDEN StrBugReport : public RangedBugReport { - std::string str; - const char* cstr; -public: - StrBugReport(BugType& D, ExplodedNode<GRState>* N, std::string s) - : RangedBugReport(D, N), str(s) { - cstr = str.c_str(); - } - - virtual const char* getDescription() const { return cstr; } -}; - - class VISIBILITY_HIDDEN AuditCFNumberCreate : public GRSimpleAPICheck { - // FIXME: Who should own this? - BadCFNumberCreate Desc; + APIMisuse* BT; // FIXME: Either this should be refactored into GRSimpleAPICheck, or // it should always be passed with a call to Audit. The latter @@ -349,24 +260,19 @@ class VISIBILITY_HIDDEN AuditCFNumberCreate : public GRSimpleAPICheck { ASTContext& Ctx; IdentifierInfo* II; GRStateManager* VMgr; + BugReporter& BR; SVal GetSVal(const GRState* St, Expr* E) { return VMgr->GetSVal(St, E); } public: - - AuditCFNumberCreate(ASTContext& ctx, GRStateManager* vmgr) - : Ctx(ctx), II(&Ctx.Idents.get("CFNumberCreate")), VMgr(vmgr) {} - - virtual ~AuditCFNumberCreate() {} + AuditCFNumberCreate(ASTContext& ctx, GRStateManager* vmgr, BugReporter& br) + : BT(0), Ctx(ctx), II(&Ctx.Idents.get("CFNumberCreate")), VMgr(vmgr), BR(br){} - virtual bool Audit(ExplodedNode<GRState>* N, GRStateManager&); + ~AuditCFNumberCreate() {} - virtual void EmitWarnings(BugReporter& BR) { - Desc.EmitWarnings(BR); - } + bool Audit(ExplodedNode<GRState>* N, GRStateManager&); private: - void AddError(const TypedRegion* R, Expr* Ex, ExplodedNode<GRState> *N, uint64_t SourceSize, uint64_t TargetSize, uint64_t NumberKind); }; @@ -537,8 +443,9 @@ void AuditCFNumberCreate::AddError(const TypedRegion* R, Expr* Ex, ExplodedNode<GRState> *N, uint64_t SourceSize, uint64_t TargetSize, uint64_t NumberKind) { - - std::ostringstream os; + + std::string sbuf; + llvm::raw_string_ostream os(sbuf); os << (SourceSize == 8 ? "An " : "A ") << SourceSize << " bit integer is used to initialize a CFNumber " @@ -553,16 +460,18 @@ void AuditCFNumberCreate::AddError(const TypedRegion* R, Expr* Ex, os << (SourceSize - TargetSize) << " bits of the input integer will be lost."; - StrBugReport* B = new StrBugReport(Desc, N, os.str()); - B->addRange(Ex->getSourceRange()); - Desc.AllErrors.push_back(B); + // Lazily create the BugType object. This will be owned + // by the BugReporter object 'BR' once we call BR.EmitWarning. + if (!BT) BT = new APIMisuse("Bad use of CFNumberCreate"); + RangedBugReport *report = new RangedBugReport(*BT, os.str().c_str(), N); + report->addRange(Ex->getSourceRange()); + BR.EmitReport(report); } GRSimpleAPICheck* clang::CreateAuditCFNumberCreate(ASTContext& Ctx, - GRStateManager* VMgr) { - - return new AuditCFNumberCreate(Ctx, VMgr); + GRStateManager* VMgr, BugReporter& BR) { + return new AuditCFNumberCreate(Ctx, VMgr, BR); } //===----------------------------------------------------------------------===// @@ -571,12 +480,13 @@ clang::CreateAuditCFNumberCreate(ASTContext& Ctx, void clang::RegisterAppleChecks(GRExprEngine& Eng) { ASTContext& Ctx = Eng.getContext(); GRStateManager* VMgr = &Eng.getStateManager(); + BugReporter &BR = Eng.getBugReporter(); - Eng.AddCheck(CreateBasicObjCFoundationChecks(Ctx, VMgr), + Eng.AddCheck(CreateBasicObjCFoundationChecks(Ctx, VMgr, BR), Stmt::ObjCMessageExprClass); - Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr), + Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr, BR), Stmt::CallExprClass); - Eng.Register(CreateNSErrorCheck()); + RegisterNSErrorChecks(BR, Eng); } diff --git a/lib/Analysis/BasicObjCFoundationChecks.h b/lib/Analysis/BasicObjCFoundationChecks.h index 3ad2ca57aa..6c594ea721 100644 --- a/lib/Analysis/BasicObjCFoundationChecks.h +++ b/lib/Analysis/BasicObjCFoundationChecks.h @@ -29,15 +29,18 @@ namespace clang { class GRSimpleAPICheck; class ASTContext; class GRStateManager; -class BugType; +class BugReporter; +class GRExprEngine; GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx, - GRStateManager* VMgr); + GRStateManager* VMgr, + BugReporter& BR); GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx, - GRStateManager* VMgr); + GRStateManager* VMgr, + BugReporter& BR); -BugType* CreateNSErrorCheck(); +void RegisterNSErrorChecks(BugReporter& BR, GRExprEngine &Eng); } // end clang namespace diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp index f59f89a1a8..c42218e506 100644 --- a/lib/Analysis/BugReporter.cpp +++ b/lib/Analysis/BugReporter.cpp @@ -23,24 +23,14 @@ #include "clang/Analysis/PathDiagnostic.h" #include "llvm/Support/raw_ostream.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/STLExtras.h" #include <sstream> using namespace clang; -BugReporter::~BugReporter() {} -GRBugReporter::~GRBugReporter() {} -BugReporterData::~BugReporterData() {} -BugType::~BugType() {} -BugReport::~BugReport() {} -RangedBugReport::~RangedBugReport() {} - -ExplodedGraph<GRState>& GRBugReporter::getGraph() { - return Eng.getGraph(); -} - -GRStateManager& GRBugReporter::getStateManager() { - return Eng.getStateManager(); -} +//===----------------------------------------------------------------------===// +// static functions. +//===----------------------------------------------------------------------===// static inline Stmt* GetStmt(const ProgramPoint& P) { if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) { @@ -85,7 +75,6 @@ static inline Stmt* GetStmt(const ExplodedNode<GRState>* N) { return isa<BlockEntrance>(ProgP) ? GetLastStmt(N) : GetStmt(ProgP); } - static void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr, const Stmt* S) { @@ -97,36 +86,42 @@ static void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr, os << ' '; os << "Execution continues on line " - << SMgr.getInstantiationLineNumber(S->getLocStart()) << '.'; + << SMgr.getInstantiationLineNumber(S->getLocStart()) << '.'; } - static inline void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr, const ExplodedNode<GRState>* N) { - ExecutionContinues(os, SMgr, GetStmt(N->getLocation())); } static inline void ExecutionContinues(std::ostringstream& os, SourceManager& SMgr, const CFGBlock* B) { - ExecutionContinues(os, SMgr, GetStmt(B)); } +//===----------------------------------------------------------------------===// +// Methods for BugType and subclasses. +//===----------------------------------------------------------------------===// +BugType::~BugType() {} +void BugType::FlushReports(BugReporter &BR) {} -Stmt* BugReport::getStmt(BugReporter& BR) const { - +//===----------------------------------------------------------------------===// +// Methods for BugReport and subclasses. +//===----------------------------------------------------------------------===// +BugReport::~BugReport() {} +RangedBugReport::~RangedBugReport() {} + +Stmt* BugReport::getStmt(BugReporter& BR) const { ProgramPoint ProgP = EndNode->getLocation(); Stmt *S = NULL; - if (BlockEntrance* BE = dyn_cast<BlockEntrance>(&ProgP)) - if (BE->getBlock() == &BR.getCFG()->getExit()) - S = GetLastStmt(EndNode); - if (!S) - S = GetStmt(ProgP); - + if (BlockEntrance* BE = dyn_cast<BlockEntrance>(&ProgP)) { + if (BE->getBlock() == &BR.getCFG()->getExit()) S = GetLastStmt(EndNode); + } + if (!S) S = GetStmt(ProgP); + return S; } @@ -144,7 +139,7 @@ BugReport::getEndPath(BugReporter& BR, const SourceRange *Beg, *End; getRanges(BR, Beg, End); - + for (; Beg != End; ++Beg) P->addRange(*Beg); @@ -163,17 +158,12 @@ void BugReport::getRanges(BugReporter& BR, const SourceRange*& beg, beg = end = 0; } -FullSourceLoc BugReport::getLocation(SourceManager& Mgr) { - - if (!EndNode) - return FullSourceLoc(); - - Stmt* S = GetStmt(EndNode); - - if (!S) - return FullSourceLoc(); - - return FullSourceLoc(S->getLocStart(), Mgr); +SourceLocation BugReport::getLocation() const { + if (EndNode) + if (Stmt* S = GetStmt(EndNode)) + return S->getLocStart(); + + return FullSourceLoc(); } PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N, @@ -183,35 +173,101 @@ PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N, return NULL; } -static std::pair<ExplodedGraph<GRState>*, ExplodedNode<GRState>*> -MakeReportGraph(const ExplodedGraph<GRState>* G, - const ExplodedNode<GRState>* N) { - - llvm::OwningPtr< ExplodedGraph<GRState> > GTrim(G->Trim(&N, &N+1)); - - // Find the error node in the trimmed graph. - - const ExplodedNode<GRState>* NOld = N; - N = 0; - - for (ExplodedGraph<GRState>::node_iterator - I = GTrim->nodes_begin(), E = GTrim->nodes_end(); I != E; ++I) { +//===----------------------------------------------------------------------===// +// Methods for BugReporter and subclasses. +//===----------------------------------------------------------------------===// + +BugReportEquivClass::~BugReportEquivClass() { + for (iterator I=begin(), E=end(); I!=E; ++I) delete *I; +} + +GRBugReporter::~GRBugReporter() { FlushReports(); } +BugReporterData::~BugReporterData() {} + +ExplodedGraph<GRState>& +GRBugReporter::getGraph() { return Eng.getGraph(); } + +GRStateManager& +GRBugReporter::getStateManager() { return Eng.getStateManager(); } + +BugReporter::~BugReporter() { FlushReports(); } + +void BugReporter::FlushReports() { + if (BugTypes.isEmpty()) + return; + + // First flush the warnings for each BugType. This may end up creating new + // warnings and new BugTypes. Because ImmutableSet is a functional data + // structure, we do not need to worry about the iterators being invalidated. + for (BugTypesTy::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I) + const_cast<BugType*>(*I)->FlushReports(*this); + + // Iterate through BugTypes a second time. BugTypes may have been updated + // with new BugType objects and new warnings. + for (BugTypesTy::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I) { + BugType *BT = const_cast<BugType*>(*I); + + typedef llvm::FoldingSet<BugReportEquivClass> SetTy; + SetTy& EQClasses = BT->EQClasses; + + for (SetTy::iterator EI=EQClasses.begin(), EE=EQClasses.end(); EI!=EE;++EI){ + BugReportEquivClass& EQ = *EI; + FlushReport(EQ); + } - if (I->getState() == NOld->getState() && - I->getLocation() == NOld->getLocation()) { - N = &*I; - break; - } + // Delete the BugType object. This will also delete the equivalence + // classes. + delete BT; } + + // Remove all references to the BugType objects. + BugTypes = F.GetEmptySet(); +} + +//===----------------------------------------------------------------------===// +// PathDiagnostics generation. +//===----------------------------------------------------------------------===// + +static std::pair<ExplodedGraph<GRState>*, + std::pair<ExplodedNode<GRState>*, unsigned> > +MakeReportGraph(const ExplodedGraph<GRState>* G, + const ExplodedNode<GRState>** NStart, + const ExplodedNode<GRState>** NEnd) { + + // Create the trimmed graph. It will contain the shortest paths from the + // error nodes to the root. In the new graph we should only have one + // error node unless there are two or more error nodes with the same minimum + // path length. + ExplodedGraph<GRState>* GTrim; + InterExplodedGraphMap<GRState>* NMap; + llvm::tie(GTrim, NMap) = G->Trim(NStart, NEnd); + + // Create owning pointers for GTrim and NMap just to ensure that they are + // released when this function exists. + llvm::OwningPtr<ExplodedGraph<GRState> > AutoReleaseGTrim(GTrim); + llvm::OwningPtr<InterExplodedGraphMap<GRState> > AutoReleaseNMap(NMap); + + // Find the (first) error node in the trimmed graph. We just need to consult + // the node map (NMap) which maps from nodes in the original graph to nodes + // in the new graph. + const ExplodedNode<GRState>* N = 0; + unsigned NodeIndex = 0; + + for (const ExplodedNode<GRState>** I = NStart; I != NEnd; ++I) + if ((N = NMap->getMappedNode(*I))) { + NodeIndex = (I - NStart) / sizeof(*I); + break; + } - assert(N); - - // Create a new graph with a single path. + assert(N && "No error node found in the trimmed graph."); + + // Create a new (third!) graph with a single path. This is the graph + // that will be returned to the caller. ExplodedGraph<GRState> *GNew = - new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(), - GTrim->getContext()); - - // Sometimes TrimGraph can contain a cycle. Perform a reverse DFS + new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(), + GTrim->getContext()); + + // Sometimes the trimmed graph can contain a cycle. Perform a reverse DFS // to the root node, and then construct a new graph that contains only // a single path. llvm::DenseMap<const void*,unsigned> Visited; @@ -238,13 +294,13 @@ MakeReportGraph(const ExplodedGraph<GRState>* G, E=Node->pred_end(); I!=E; ++I) WS.push_back(*I); } - + assert (Root); // Now walk from the root down the DFS path, always taking the successor // with the lowest number. ExplodedNode<GRState> *Last = 0, *First = 0; - + for ( N = Root ;;) { // Lookup the number associated with the current node. llvm::DenseMap<const void*,unsigned>::iterator I = Visited.find(N); @@ -253,20 +309,20 @@ MakeReportGraph(const ExplodedGraph<GRState>* G, // Create the equivalent node in the new graph with the same state // and location. ExplodedNode<GRState>* NewN = - GNew->getNode(N->getLocation(), N->getState()); - + GNew->getNode(N->getLocation(), N->getState()); + // Link up the new node with the previous node. if (Last) NewN->addPredecessor(Last); Last = NewN; - + // Are we at the final node? if (I->second == 0) { First = NewN; break; } - + // Find the next successor node. We choose the node that is marked // with the lowest DFS number. ExplodedNode<GRState>::const_succ_iterator SI = N->succ_begin(); @@ -274,7 +330,7 @@ MakeReportGraph(const ExplodedGraph<GRState>* G, N = 0; for (unsigned MinVal = 0; SI != SE; ++SI) { - + I = Visited.find(*SI); if (I == Visited.end()) @@ -285,12 +341,12 @@ MakeReportGraph(const ExplodedGraph<GRState>* G, MinVal = I->second; } } - + assert (N); } - + assert (First); - return std::make_pair(GNew, First); + return std::make_pair(GNew, std::make_pair(First, NodeIndex)); } static const VarDecl* @@ -300,12 +356,12 @@ GetMostRecentVarDeclBinding(const ExplodedNode<GRState>* N, for ( ; N ; N = N->pred_empty() ? 0 : *N->pred_begin()) { ProgramPoint P = N->getLocation(); - + if (!isa<PostStmt>(P)) continue; DeclRefExpr* DR = dyn_cast<DeclRefExpr>(cast<PostStmt>(P).getStmt()); - + if (!DR) continue; @@ -474,25 +530,37 @@ public: } // end anonymous namespace void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, - BugReport& R) { - - const ExplodedNode<GRState>* N = R.getEndNode(); + BugReportEquivClass& EQ) { + + std::vector<const ExplodedNode<GRState>*> Nodes; - if (!N) return; + for (BugReportEquivClass::iterator I=EQ.begin(), E=EQ.end(); I!=E; ++I) { + const ExplodedNode<GRState>* N = I->getEndNode(); + if (N) Nodes.push_back(N); + } + + if (Nodes.empty()) + return; // Construct a new graph that contains only a single path from the error - // node to a root. + // node to a root. + const std::pair<ExplodedGraph<GRState>*, + std::pair<ExplodedNode<GRState>*, unsigned> >& + GPair = MakeReportGraph(&getGraph(), &Nodes[0], &Nodes[0] + Nodes.size()); + + // Find the BugReport with the original location. + BugReport *R = 0; + unsigned i = 0; + for (BugReportEquivClass::iterator I=EQ.begin(), E=EQ.end(); I!=E; ++I, ++i) + if (i == GPair.second.second) { R = *I; break; } - const std::pair<ExplodedGraph<GRState>*, ExplodedNode<GRState>*> - GPair = MakeReportGraph(&getGraph(), N); + assert(R && "No original report found for sliced graph."); llvm::OwningPtr<ExplodedGraph<GRState> > ReportGraph(GPair.first); - assert(GPair.second->getLocation() == N->getLocation()); - N = GPair.second; + const ExplodedNode<GRState> *N = GPair.second.first; - // Start building the path diagnostic... - - if (PathDiagnosticPiece* Piece = R.getEndPath(*this, N)) + // Start building the path diagnostic... + if (PathDiagnosticPiece* Piece = R->getEndPath(*this, N)) PD.push_back(Piece); else return; @@ -686,7 +754,7 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, } } - if (PathDiagnosticPiece* p = R.VisitNode(N, NextNode, *ReportGraph, *this)) + if (PathDiagnosticPiece* p = R->VisitNode(N, NextNode, *ReportGraph, *this)) PD.push_front(p); if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) { @@ -699,37 +767,41 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, } -bool BugTypeCacheLocation::isCached(BugReport& R) { - - const ExplodedNode<GRState>* N = R.getEndNode(); - - if (!N) - return false; - - // Cache the location of the error. Don't emit the same - // warning for the same error type that occurs at the same program - // location but along a different path. - - return isCached(N->getLocation()); +void BugReporter::Register(BugType *BT) { + BugTypes = F.Add(BugTypes, BT); } -bool BugTypeCacheLocation::isCached(ProgramPoint P) { - if (CachedErrors.count(P)) - return true; +void BugReporter::EmitReport(BugReport* R) { + // Compute the bug report's hash to determine its equivalence class. + llvm::FoldingSetNodeID ID; + R->Profile(ID); - CachedErrors.insert(P); - return false; + // Lookup the equivance class. If there isn't one, create it. + BugType& BT = R->getBugType(); + Register(&BT); + void *InsertPos; + BugReportEquivClass* EQ = BT.EQClasses.FindNodeOrInsertPos(ID, InsertPos); + + if (!EQ) { + EQ = new BugReportEquivClass(R); + BT.EQClasses.InsertNode(EQ, InsertPos); + } + else + EQ->AddReport(R); } -void BugReporter::EmitWarning(BugReport& R) { - - if (R.getBugType().isCached(R)) - return; - - llvm::OwningPtr<PathDiagnostic> D(new PathDiagnostic(R.getName(), - R.getDescription(), - R.getCategory())); - GeneratePathDiagnostic(*D.get(), R); +void BugReporter::FlushReport(BugReportEquivClass& EQ) { + assert(!EQ.Reports.empty()); + BugReport &R = **EQ.begin(); + + // FIXME: Make sure we use the 'R' for the path that was actually used. + // Probably doesn't make a difference in practice. + BugType& BT = R.getBugType(); + + llvm::OwningPtr<PathDiagnostic> D(new PathDiagnostic(R.getBugType().getName(), + R.getDescription(), + BT.getCategory())); + GeneratePathDiagnostic(*D.get(), EQ); // Get the meta data. std::pair<const char**, const char**> Meta = R.getExtraDescriptiveText(); @@ -740,9 +812,9 @@ void BugReporter::EmitWarning(BugReport& R) { const SourceRange *Beg = 0, *End = 0; R.getRanges(*this, Beg, End); Diagnostic& Diag = getDiagnostic(); - FullSourceLoc L = R.getLocation(getSourceManager()); - const char *msg = PD ? R.getBugType().getName() : R.getDescription(); - unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning, msg); + FullSourceLoc L(R.getLocation(), getSourceManager()); + const std::string &msg = PD ? R.getBugType().getName() : R.getDescription(); + unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning, msg.c_str()); switch (End-Beg) { default: assert(0 && "Don't handle this many ranges yet!"); @@ -770,73 +842,15 @@ void BugReporter::EmitBasicReport(const char* name, const char* str, SourceRange* RBeg, unsigned NumRanges) { EmitBasicReport(name, "", str, Loc, RBeg, NumRanges); } - + void BugReporter::EmitBasicReport(const char* name, const char* category, const char* str, SourceLocation Loc, SourceRange* RBeg, unsigned NumRanges) { - SimpleBugType BT(name, category, 0); - DiagCollector C(BT); - Diagnostic& Diag = getDiagnostic(); - - DiagnosticClient *OldClient = Diag.getClient(); - Diag.setClient(&C); + // 'BT' will be owned by BugReporter as soon as we call 'EmitReport'. + BugType *BT = new BugType(name, category); FullSourceLoc L = getContext().getFullLoc(Loc); - unsigned DiagID = Diag.getCustomDiagID(Diagnostic::Warning, str); - - switch (NumRanges) { - default: assert(0 && "Don't handle this many ranges yet!"); - case 0: Diag.Report(L, DiagID); break; - case 1: Diag.Report(L, DiagID) << RBeg[0]; break; - case 2: Diag.Report(L, DiagID) << RBeg[0] << RBeg[1]; break; - case 3: Diag.Report(L, DiagID) << RBeg[0] << RBeg[1] << RBeg[2]; break; - } - - Diag.setClient(OldClient); - - for (DiagCollector::iterator I = C.begin(), E = C.end(); I != E; ++I) - EmitWarning(*I); -} - -void DiagCollector::HandleDiagnostic(Diagnostic::Level DiagLevel, - const DiagnosticInfo &Info) { - - // FIXME: Use a map from diag::kind to BugType, instead of having just - // one BugType. - const char *Desc = Info.getDiags()->getDescription(Info.getID()); - Reports.push_back(DiagBugReport(Desc, D, Info.getLocation())); - DiagBugReport& R = Reports.back(); - - for (unsigned i = 0, e = Info.getNumRanges(); i != e; ++i) - R.addRange(Info.getRange(i)); - - // FIXME: This is losing/ignoring formatting. - for (unsigned i = 0, e = Info.getNumArgs(); i != e; ++i) { - switch (Info.getArgKind(i)) { - case Diagnostic::ak_std_string: - R.addString(Info.getArgStdStr(i)); - break; - case Diagnostic::ak_c_string: - R.addString(Info.getArgCStr(i)); - break; - case Diagnostic::ak_sint: - R.addString(llvm::itostr(Info.getArgSInt(i))); - break; - case Diagnostic::ak_uint: - R.addString(llvm::utostr_32(Info.getArgUInt(i))); - break; - case Diagnostic::ak_identifierinfo: - R.addString(Info.getArgIdentifier(i)->getName()); - break; - case Diagnostic::ak_qualtype: - case Diagnostic::ak_declarationname: { - llvm::SmallString<64> Str; - Info.getDiags()->ConvertArgToString(Info.getArgKind(i), - Info.getRawArg(i), 0, 0, 0, 0, Str); - R.addString(std::string(Str.begin(), Str.end())); - break; - } - } - } + RangedBugReport *R = new DiagBugReport(*BT, str, L); + for ( ; NumRanges > 0 ; --NumRanges, ++RBeg) R->addRange(*RBeg); + EmitReport(R); } - diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp index ab9d409dd2..e42fa8614c 100644 --- a/lib/Analysis/CFRefCount.cpp +++ b/lib/Analysis/CFRefCount.cpp @@ -1333,7 +1333,7 @@ public: delete I->second; } - virtual void RegisterChecks(GRExprEngine& Eng); + void RegisterChecks(BugReporter &BR); virtual void RegisterPrinters(std::vector<GRState::Printer*>& Printers) { Printers.push_back(new BindingsPrinter()); @@ -2154,91 +2154,73 @@ namespace { // Bug Descriptions. // //===-------------===// - class VISIBILITY_HIDDEN CFRefBug : public BugTypeCacheLocation { + class VISIBILITY_HIDDEN CFRefBug : public BugType { protected: CFRefCount& TF; - + + CFRefBug(CFRefCount* tf, const char* name) + : BugType(name, "Memory (Core Foundation/Objective-C)"), TF(*tf) {} public: - CFRefBug(CFRefCount& tf) : TF(tf) {} CFRefCount& getTF() { return TF; } const CFRefCount& getTF() const { return TF; } + // FIXME: Eventually remove. + virtual const char* getDescription() const = 0; + virtual bool isLeak() const { return false; } - - const char* getCategory() const { - return "Memory (Core Foundation/Objective-C)"; - } }; class VISIBILITY_HIDDEN UseAfterRelease : public CFRefBug { public: - UseAfterRelease(CFRefCount& tf) : CFRefBug(tf) {} + UseAfterRelease(CFRefCount* tf) + : CFRefBug(tf, "use-after-release") {} - virtual const char* getName() const { - return "use-after-release"; - } - virtual const char* getDescription() const { + const char* getDescription() const { return "Reference-counted object is used after it is released."; } - virtual void EmitWarnings(BugReporter& BR); + virtual void FlushReports(BugReporter& BR); }; class VISIBILITY_HIDDEN BadRelease : public CFRefBug { public: - BadRelease(CFRefCount& tf) : CFRefBug(tf) {} - - virtual const char* getName() const { - return "bad release"; - } - virtual const char* getDescription() const { + BadRelease(CFRefCount* tf) : CFRefBug(tf, "bad release") {} + + const char* getDescription() const { return "Incorrect decrement of the reference count of a " "CoreFoundation object: " "The object is not owned at this point by the caller."; } - virtual void EmitWarnings(BugReporter& BR); + void FlushReports(BugReporter& BR); }; class VISIBILITY_HIDDEN Leak : public CFRefBug { - bool isReturn; + const bool isReturn; + protected: + Leak(CFRefCount* tf, const char* name, bool isRet) + : CFRefBug(tf, name), isReturn(isRet) {} public: - Leak(CFRefCount& tf) : CFRefBug(tf) {} - void setIsReturn(bool x) { isReturn = x; } - - virtual const char* getName() const { - - if (!isReturn) { - if (getTF().isGCEnabled()) - return "leak (GC)"; - - if (getTF().getLangOptions().getGCMode() == LangOptions::HybridGC) - return "leak (hybrid MM, non-GC)"; - - assert (getTF().getLangOptions().getGCMode() == LangOptions::NonGC); - return "leak"; - } - else { - if (getTF().isGCEnabled()) - return "[naming convention] leak of returned object (GC)"; - - if (getTF().getLangOptions().getGCMode() == LangOptions::HybridGC) - return "[naming convention] leak of returned object (hybrid MM, " - "non-GC)"; - - assert (getTF().getLangOptions().getGCMode() == LangOptions::NonGC); - return "[naming convention] leak of returned object"; - } - } + // FIXME: Remove once reports have better descriptions. + const char* getDescription() const { return "leak"; } - virtual void EmitWarnings(BugReporter& BR); - virtual void GetErrorNodes(std::vector<ExplodedNode<GRState>*>& Nodes); - virtual bool isLeak() const { return true; } - virtual bool isCached(BugReport& R); + void FlushReports(BugReporter &BR); + }; + + class VISIBILITY_HIDDEN LeakAtReturn : public Leak { + public: + LeakAtReturn(CFRefCount* tf, const char* name) + : Leak(tf, name, true) {} }; + class VISIBILITY_HIDDEN LeakWithinFunction : public Leak { + public: + LeakWithinFunction(CFRefCount* tf, const char* name) + : Leak(tf, name, false) {} + }; + //===---------===// // Bug Reports. // //===---------===// @@ -2247,7 +2229,7 @@ namespace { SymbolRef Sym; public: CFRefReport(CFRefBug& D, ExplodedNode<GRState> *n, SymbolRef sym) - : RangedBugReport(D, n), Sym(sym) {} + : RangedBugReport(D, D.getDescription(), n), Sym(sym) {} virtual ~CFRefReport() {} @@ -2278,17 +2260,49 @@ namespace { const ExplodedNode<GRState>* PrevN, const ExplodedGraph<GRState>& G, BugReporter& BR); + }; - + class VISIBILITY_HIDDEN CFRefLeakReport : public CFRefReport { + public: + CFRefLeakReport(CFRefBug& D, ExplodedNode<GRState> *n, SymbolRef sym) + : CFRefReport(D, n, sym) {} + + SourceLocation getLocation() const; + }; } // end anonymous namespace -void CFRefCount::RegisterChecks(GRExprEngine& Eng) { - Eng.Register(new UseAfterRelease(*this)); - Eng.Register(new BadRelease(*this)); - Eng.Register(new Leak(*this)); -} +void CFRefCount::RegisterChecks(BugReporter& BR) { + BR.Register(new UseAfterRelease(this)); + BR.Register(new BadRelease(this)); + + // First register "return" leaks. + const char* name = 0; + + if (isGCEnabled()) + name = "[naming convention] leak of returned object (GC)"; + else if (getLangOptions().getGCMode() == LangOptions::HybridGC) + name = "[naming convention] leak of returned object (hybrid MM, " + "non-GC)"; + else { + assert(getLangOptions().getGCMode() == LangOptions::NonGC); + name = "[naming convention] leak of returned object"; + } + + BR.Register(new LeakAtReturn(this, name)); + // Second, register leaks within a function/method. + if (isGCEnabled()) + name = "leak (GC)"; + else if (getLangOptions().getGCMode() == LangOptions::HybridGC) + name = "leak (hybrid MM, non-GC)"; + else { + assert(getLangOptions().getGCMode() == LangOptions::NonGC); + name = "leak"; + } + + BR.Register(new LeakWithinFunction(this, name)); +} static const char* Msgs[] = { "Code is compiled in garbage collection only mode" // GC only @@ -2621,30 +2635,25 @@ CFRefReport::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN) { return new PathDiagnosticPiece(L, os.str(), Hint); } -void UseAfterRelease::EmitWarnings(BugReporter& BR) { - +void UseAfterRelease::FlushReports(BugReporter& BR) { for (CFRefCount::use_after_iterator I = TF.use_after_begin(), - E = TF.use_after_end(); I != E; ++I) { - - CFRefReport report(*this, I->first, I->second.second); - report.addRange(I->second.first->getSourceRange()); - BR.EmitWarning(report); + E = TF.use_after_end(); I != E; ++I) { + CFRefReport *report = new CFRefReport(*this, I->first, I->second.second); + report->addRange(I->second.first->getSourceRange()); + BR.EmitReport(report); } } -void BadRelease::EmitWarnings(BugReporter& BR) { - +void BadRelease::FlushReports(BugReporter& BR) { for (CFRefCount::bad_release_iterator I = TF.bad_release_begin(), E = TF.bad_release_end(); I != E; ++I) { - - CFRefReport report(*this, I->first, I->second.second); - report.addRange(I->second.first->getSourceRange()); - BR.EmitWarning(report); + CFRefReport *report = new CFRefReport(*this, I->first, I->second.second); + report->addRange(I->second.first->getSourceRange()); + BR.EmitReport(report); } } -void Leak::EmitWarnings(BugReporter& BR) { - +void Leak::FlushReports(BugReporter& BR) { for (CFRefCount::leaks_iterator I = TF.leaks_begin(), E = TF.leaks_end(); I != E; ++I) { @@ -2652,34 +2661,20 @@ void Leak::EmitWarnings(BugReporter& BR) { unsigned n = SymV.size(); for (unsigned i = 0; i < n; ++i) { - setIsReturn(SymV[i].second); - CFRefReport report(*this, I->first, SymV[i].first); - BR.EmitWarning(report); + if (isReturn != SymV[i].second) continue; + CFRefReport* report = new CFRefLeakReport(*this, I->first, SymV[i].first); + BR.EmitReport(report); } } } -void Leak::GetErrorNodes(std::vector<ExplodedNode<GRState>*>& Nodes) { - for (CFRefCount::leaks_iterator I=TF.leaks_begin(), E=TF.leaks_end(); - I!=E; ++I) - Nodes.push_back(I->first); -} - -bool Leak::isCached(BugReport& R) { - +SourceLocation CFRefLeakReport::getLocation() const { // Most bug reports are cached at the location where they occured. // With leaks, we want to unique them by the location where they were // allocated, and only report a single path. - - SymbolRef Sym = static_cast<CFRefReport&>(R).getSymbol(); - - const ExplodedNode<GRState>* AllocNode = - GetAllocationSite(0, R.getEndNode(), Sym).first; - - if (!AllocNode) - return false; - - return BugTypeCacheLocation::isCached(AllocNode->getLocation()); + ProgramPoint P = + GetAllocationSite(0, getEndNode(), getSymbol()).first->getLocation(); + return cast<PostStmt>(P).getStmt()->getLocStart(); } //===----------------------------------------------------------------------===// diff --git a/lib/Analysis/CheckNSError.cpp b/lib/Analysis/CheckNSError.cpp index 919c17d064..a35497a6bf 100644 --- a/lib/Analysis/CheckNSError.cpp +++ b/lib/Analysis/CheckNSError.cpp @@ -27,50 +27,45 @@ using namespace clang; namespace { -class VISIBILITY_HIDDEN NSErrorCheck : public BugTypeCacheLocation { - - void EmitGRWarnings(GRBugReporter& BR); +class VISIBILITY_HIDDEN NSErrorCheck : public BugType { + const bool isNSErrorWarning; + IdentifierInfo * const II; + GRExprEngine &Eng; void CheckSignature(ObjCMethodDecl& MD, QualType& ResultTy, - llvm::SmallVectorImpl<VarDecl*>& NSErrorParams, - llvm::SmallVectorImpl<VarDecl*>& CFErrorParams, - IdentifierInfo* NSErrorII, - IdentifierInfo* CFErrorII); + llvm::SmallVectorImpl<VarDecl*>& ErrorParams); void CheckSignature(FunctionDecl& MD, QualType& ResultTy, - llvm::SmallVectorImpl<VarDecl*>& NSErrorParams, - llvm::SmallVectorImpl<VarDecl*>& CFErrorParams, - IdentifierInfo* NSErrorII, - IdentifierInfo* CFErrorII); + llvm::SmallVectorImpl<VarDecl*>& ErrorParams); - bool CheckNSErrorArgument(QualType ArgTy, IdentifierInfo* NSErrorII); - bool CheckCFErrorArgument(QualType ArgTy, IdentifierInfo* CFErrorII); + bool CheckNSErrorArgument(QualType ArgTy); + bool CheckCFErrorArgument(QualType ArgTy); - void CheckParamDeref(VarDecl* V, GRStateRef state, GRExprEngine& Eng, - GRBugReporter& BR, bool isNErrorWarning); + void CheckParamDeref(VarDecl* V, GRStateRef state, BugReporter& BR); - void EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl, bool isNSErrorWarning); + void EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl); - const char* desc; - const char* name; public: - NSErrorCheck() : desc(0) {} - - void EmitWarnings(BugReporter& BR) { EmitGRWarnings(cast<GRBugReporter>(BR));} - const char* getName() const { return name; } - const char* getDescription() const { return desc; } - const char* getCategory() const { return "Coding Conventions (Apple)"; } + NSErrorCheck(bool isNSError, GRExprEngine& eng) + : BugType(isNSError ? "NSError** null dereference" + : "CFErrorRef* null dereference", + "Coding Conventions (Apple)"), + isNSErrorWarning(isNSError), + II(&eng.getContext().Idents.get(isNSErrorWarning ? "NSError":"CFErrorRef")), + Eng(eng) {} + + void FlushReports(BugReporter& BR); }; } // end anonymous namespace -BugType* clang::CreateNSErrorCheck() { - return new NSErrorCheck(); +void clang::RegisterNSErrorChecks(BugReporter& BR, GRExprEngine &Eng) { + BR.Register(new NSErrorCheck(true, Eng)); + BR.Register(new NSErrorCheck(false, Eng)); } -void NSErrorCheck::EmitGRWarnings(GRBugReporter& BR) { +void NSErrorCheck::FlushReports(BugReporter& BR) { // Get the analysis engine and the exploded analysis graph. - GRExprEngine& Eng = BR.getEngine(); GRExprEngine::GraphTy& G = Eng.getGraph(); // Get the declaration of the method/function that was analyzed. @@ -80,50 +75,34 @@ void NSErrorCheck::EmitGRWarnings(GRBugReporter& BR) { ASTContext &Ctx = BR.getContext(); QualType ResultTy; - llvm::SmallVector<VarDecl*, 5> NSErrorParams; - llvm::SmallVector<VarDecl*, 5> CFErrorParams; + llvm::SmallVector<VarDecl*, 5> ErrorParams; if (ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(&CodeDecl)) - CheckSignature(*MD, ResultTy, NSErrorParams, CFErrorParams, - &Ctx.Idents.get("NSError"), &Ctx.Idents.get("CFErrorRef")); + CheckSignature(*MD, ResultTy, ErrorParams); else if (FunctionDecl* FD = dyn_cast<FunctionDecl>(&CodeDecl)) - CheckSignature(*FD, ResultTy, NSErrorParams, CFErrorParams, - &Ctx.Idents.get("NSError"), &Ctx.Idents.get("CFErrorRef")); + CheckSignature(*FD, ResultTy, ErrorParams); else return; - if (NSErrorParams.empty() && CFErrorParams.empty()) + if (ErrorParams.empty()) return; - if (ResultTy == Ctx.VoidTy) { - if (!NSErrorParams.empty()) - EmitRetTyWarning(BR, CodeDecl, true); - if (!CFErrorParams.empty()) - EmitRetTyWarning(BR, CodeDecl, false); - } + if (ResultTy == Ctx.VoidTy) EmitRetTyWarning(BR, CodeDecl); for (GRExprEngine::GraphTy::roots_iterator RI=G.roots_begin(), RE=G.roots_end(); RI!=RE; ++RI) { + // Scan the parameters for an implicit null dereference. + for (llvm::SmallVectorImpl<VarDecl*>::iterator I=ErrorParams.begin(), + E=ErrorParams.end(); I!=E; ++I) + CheckParamDeref(*I, GRStateRef((*RI)->getState(),Eng.getStateManager()), + BR); - // Scan the NSError** parameters for an implicit null dereference. - for (llvm::SmallVectorImpl<VarDecl*>::iterator I=NSErrorParams.begin(), - E=NSErrorParams.end(); I!=E; ++I) - CheckParamDeref(*I, GRStateRef((*RI)->getState(), Eng.getStateManager()), - Eng, BR, true); - - // Scan the CFErrorRef* parameters for an implicit null dereference. - for (llvm::SmallVectorImpl<VarDecl*>::iterator I=CFErrorParams.begin(), - E=CFErrorParams.end(); I!=E; ++I) - CheckParamDeref(*I, GRStateRef((*RI)->getState(), Eng.getStateManager()), - Eng, BR, false); } } -void NSErrorCheck::EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl, - bool isNSErrorWarning) { - - std::string msg; - llvm::raw_string_ostream os(msg); +void NSErrorCheck::EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl) { + std::string sbuf; + llvm::raw_string_ostream os(sbuf); if (isa<ObjCMethodDecl>(CodeDecl)) os << "Method"; @@ -138,15 +117,13 @@ void NSErrorCheck::EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl, BR.EmitBasicReport(isNSErrorWarning ? "Bad return type when passing NSError**" : "Bad return type when passing CFError*", - getCategory(), os.str().c_str(), CodeDecl.getLocation()); + getCategory().c_str(), os.str().c_str(), + CodeDecl.getLocation()); } void NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy, - llvm::SmallVectorImpl<VarDecl*>& NSErrorParams, - llvm::SmallVectorImpl<VarDecl*>& CFErrorParams, - IdentifierInfo* NSErrorII, - IdentifierInfo* CFErrorII) { + llvm::SmallVectorImpl<VarDecl*>& ErrorParams) { ResultTy = M.getResultType(); @@ -155,19 +132,17 @@ NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy, QualType T = (*I)->getType(); - if (CheckNSErrorArgument(T, NSErrorII)) - NSErrorParams.push_back(*I); - else if (CheckCFErrorArgument(T, CFErrorII)) - CFErrorParams.push_back(*I); + if (isNSErrorWarning) { + if (CheckNSErrorArgument(T)) ErrorParams.push_back(*I); + } + else if (CheckCFErrorArgument(T)) + ErrorParams.push_back(*I); } } void NSErrorCheck::CheckSignature(FunctionDecl& F, QualType& ResultTy, - llvm::SmallVectorImpl<VarDecl*>& NSErrorParams, - llvm::SmallVectorImpl<VarDecl*>& CFErrorParams, - IdentifierInfo* NSErrorII, - IdentifierInfo* CFErrorII) { + llvm::SmallVectorImpl<VarDecl*>& ErrorParams) { ResultTy = F.getResultType(); @@ -175,17 +150,17 @@ NSErrorCheck::CheckSignature(FunctionDecl& F, QualType& ResultTy, E=F.param_end(); I!=E; ++I) { QualType T = (*I)->getType(); - - if (CheckNSErrorArgument(T, NSErrorII)) - NSErrorParams.push_back(*I); - else if (CheckCFErrorArgument(T, CFErrorII)) - CFErrorParams.push_back(*I); + + if (isNSErrorWarning) { + if (CheckNSErrorArgument(T)) ErrorParams.push_back(*I); + } + else if (CheckCFErrorArgument(T)) + ErrorParams.push_back(*I); } } -bool NSErrorCheck::CheckNSErrorArgument(QualType ArgTy, - IdentifierInfo* NSErrorII) { +bool NSErrorCheck::CheckNSErrorArgument(QualType ArgTy) { const PointerType* PPT = ArgTy->getAsPointerType(); if (!PPT) return false; @@ -197,11 +172,10 @@ bool NSErrorCheck::CheckNSErrorArgument(QualType ArgTy, PT->getPointeeType()->getAsObjCInterfaceType(); if (!IT) return false; - return IT->getDecl()->getIdentifier() == NSErrorII; + return IT->getDecl()->getIdentifier() == II; } -bool NSErrorCheck::CheckCFErrorArgument(QualType ArgTy, - IdentifierInfo* CFErrorII) { +bool NSErrorCheck::CheckCFErrorArgument(QualType ArgTy) { const PointerType* PPT = ArgTy->getAsPointerType(); if (!PPT) return false; @@ -209,12 +183,11 @@ bool NSErrorCheck::CheckCFErrorArgument(QualType ArgTy, const TypedefType* TT = PPT->getPointeeType()->getAsTypedefType(); if (!TT) return false; - return TT->getDecl()->getIdentifier() == CFErrorII; + return TT->getDecl()->getIdentifier() == II; } void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState, - GRExprEngine& Eng, GRBugReporter& BR, - bool isNSErrorWarning) { + BugReporter& BR) { SVal ParamL = rootState.GetLValue(Param); const MemRegion* ParamR = cast<loc::MemRegionVal>(ParamL).getRegionAs<VarRegion>(); @@ -237,13 +210,11 @@ void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState, if (!SVX || SVX->getSymbol() != SV->getSymbol()) continue; // Emit an error. - BugReport R(*this, *I); + - name = isNSErrorWarning ? "NSError** null dereference" - : "CFErrorRef* null dereference"; - std::string msg; - llvm::raw_string_ostream os(msg); + std::string sbuf; + llvm::raw_string_ostream os(sbuf); os << "Potential null dereference. According to coding standards "; if (isNSErrorWarning) @@ -252,9 +223,11 @@ void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState, os << "documented in CoreFoundation/CFError.h the parameter '"; os << Param->getNameAsString() << "' may be null."; - desc = os.str().c_str(); - - BR.addNotableSymbol(SV->getSymbol()); - BR.EmitWarning(R); + + BugReport *report = new BugReport(*this, os.str().c_str(), *I); + // FIXME: Notable symbols are now part of the report. We should + // add support for notable symbols in BugReport. + // BR.addNotableSymbol(SV->getSymbol()); + BR.EmitReport(report); } } diff --git a/lib/Analysis/ExplodedGraph.cpp b/lib/Analysis/ExplodedGraph.cpp index 4e41b60da3..864a83b884 100644 --- a/lib/Analysis/ExplodedGraph.cpp +++ b/lib/Analysis/ExplodedGraph.cpp @@ -122,13 +122,17 @@ ExplodedNodeImpl::NodeGroup::~NodeGroup() { ExplodedGraphImpl* ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources, - const ExplodedNodeImpl* const* EndSources) const{ - - typedef llvm::DenseMap<const ExplodedNodeImpl*, const ExplodedNodeImpl*> Pass1Ty; - typedef llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*> Pass2Ty; + const ExplodedNodeImpl* const* EndSources, + InterExplodedGraphMapImpl* M) const { + typedef llvm::DenseMap<const ExplodedNodeImpl*, + const ExplodedNodeImpl*> Pass1Ty; Pass1Ty Pass1; - Pass2Ty Pass2; + + typedef llvm::DenseMap<const ExplodedNodeImpl*, + ExplodedNodeImpl*> Pass2Ty; + + Pass2Ty& Pass2 = M->M; llvm::SmallVector<const ExplodedNodeImpl*, 10> WL2; @@ -139,23 +143,28 @@ ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources, std::list<std::pair<const ExplodedNodeImpl*, const ExplodedNodeImpl*> > WL1, WL1_Loops; - for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources; ++I) + for (const ExplodedNodeImpl* const* I = BeginSources; I != EndSources; ++I){ + assert(*I); WL1.push_back(std::make_pair(*I, *I)); + } // Process the worklist. - while (! (WL1.empty() && WL1_Loops.empty())) { + while (!WL1.empty() || !WL1_Loops.empty()) { // Only dequeue from the "loops" worklist if WL1 has no items. // Thus we prioritize for paths that don't span loop boundaries. - const ExplodedNodeImpl *N, *Src; + const ExplodedNodeImpl *N = 0, *Src = 0; if (WL1.empty()) { + assert(!WL1_Loops.empty()); N = WL1_Loops.back().first; + assert(N); Src = WL1_Loops.back().second; WL1_Loops.pop_back(); } else { N = WL1.back().first; + assert(N); Src = WL1.back().second; WL1.pop_back(); } @@ -207,19 +216,21 @@ ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources, case Stmt::ForStmtClass: case Stmt::WhileStmtClass: case Stmt::DoStmtClass: + assert(*I); WL1_Loops.push_front(std::make_pair(*I, Src)); continue; } + assert(*I); WL1.push_front(std::make_pair(*I, Src)); } } } if (WL2.empty()) - return NULL; - + return 0; + ExplodedGraphImpl* G = MakeEmptyGraph(); // ===- Pass 2 (forward DFS to construct the new graph) -=== @@ -285,3 +296,14 @@ ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources, return G; } + +ExplodedNodeImpl* +InterExplodedGraphMapImpl::getMappedImplNode(const ExplodedNodeImpl* N) const { + llvm::DenseMap<const ExplodedNodeImpl*, ExplodedNodeImpl*>::iterator I = + M.find(N); + + return I == M.end() ? 0 : I->second; +} + +InterExplodedGraphMapImpl::InterExplodedGraphMapImpl() {} + diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index bb34763df7..ea2ae3d92b 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -69,22 +69,7 @@ public: MapTy::iterator I = M.find(key); M[key] = F.Concat(A, I == M.end() ? F.GetEmptyList() : I->second); } - - virtual void EmitWarnings(BugReporter& BR) { - llvm::DenseSet<GRSimpleAPICheck*> AlreadyVisited; - - for (MapTy::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) - for (Checks::iterator I=MI->second.begin(), E=MI->second.end(); I!=E;++I){ - - GRSimpleAPICheck* check = *I; - - if (AlreadyVisited.count(check)) - continue; - - check->EmitWarnings(BR); - } - } - + virtual bool Audit(NodeTy* N, GRStateManager& VMgr) { Stmt* S = cast<PostStmt>(N->getLocation()).getStmt(); void* key = reinterpret_cast<void*>((uintptr_t) S->getStmtClass()); @@ -115,7 +100,8 @@ static inline Selector GetNullarySelector(const char* name, ASTContext& Ctx) { GRExprEngine::GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx, - LiveVariables& L, bool purgeDead, + LiveVariables& L, BugReporterData& BRD, + bool purgeDead, StoreManagerCreator SMC, ConstraintManagerCreator CMC) : CoreEngine(cfg, CD, Ctx, *this), @@ -127,13 +113,11 @@ GRExprEngine::GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx, CurrentStmt(NULL), NSExceptionII(NULL), NSExceptionInstanceRaiseSelectors(NULL), RaiseSel(GetNullarySelector("raise", G.getContext())), - PurgeDead(purgeDead){} + PurgeDead(purgeDead), + BR(BRD, *this) {} GRExprEngine::~GRExprEngine() { - for (BugTypeSet::iterator I = BugTypes.begin(), E = BugTypes.end(); I!=E; ++I) - delete *I; - - + BR.FlushReports(); delete [] NSExceptionInstanceRaiseSelectors; } @@ -164,22 +148,9 @@ struct VISIBILITY_HIDDEN SaveOr { bool old_value; }; - -void GRExprEngine::EmitWarnings(BugReporterData& BRData) { - for (bug_type_iterator I = bug_types_begin(), E = bug_types_end(); I!=E; ++I){ - GRBugReporter BR(BRData, *this); - (*I)->EmitWarnings(BR); - } - - if (BatchAuditor) { - GRBugReporter BR(BRData, *this); - BatchAuditor->EmitWarnings(BR); - } -} - void GRExprEngine::setTransferFunctions(GRTransferFuncs* tf) { StateMgr.TF = tf; - tf->RegisterChecks(*this); + tf->RegisterChecks(getBugReporter()); tf->RegisterPrinters(getStateManager().Printers); } @@ -2918,7 +2889,7 @@ void GRExprEngine::ViewGraph(bool trim) { if (trim) { std::vector<NodeTy*> Src; - // Fixme: Migrate over to the new way of adding nodes. + // FIXME: Migrate over to the new way of adding nodes. AddSources(Src, null_derefs_begin(), null_derefs_end()); AddSources(Src, undef_derefs_begin(), undef_derefs_end()); AddSources(Src, explicit_bad_divides_begin(), explicit_bad_divides_end()); @@ -2927,10 +2898,8 @@ void GRExprEngine::ViewGraph(bool trim) { AddSources(Src, undef_arg_begin(), undef_arg_end()); AddSources(Src, undef_branches_begin(), undef_branches_end()); - // The new way. - for (BugTypeSet::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I) - (*I)->GetErrorNodes(Src); - + // FIXME: Enhance BugReporter to have a clean way to query if a node + // is involved in an error... and what kind. ViewGraph(&Src[0], &Src[0]+Src.size()); } @@ -2951,14 +2920,12 @@ void GRExprEngine::ViewGraph(NodeTy** Beg, NodeTy** End) { GraphPrintCheckerState = this; GraphPrintSourceManager = &getContext().getSourceManager(); - GRExprEngine::GraphTy* TrimmedG = G.Trim(Beg, End); + std::auto_ptr<GRExprEngine::GraphTy> TrimmedG(G.Trim(Beg, End).first); - if (!TrimmedG) + if (!TrimmedG.get()) llvm::cerr << "warning: Trimmed ExplodedGraph is empty.\n"; - else { + else llvm::ViewGraph(*TrimmedG->roots_begin(), "TrimmedGRExprEngine"); - delete TrimmedG; - } GraphPrintCheckerState = NULL; GraphPrintSourceManager = NULL; diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp index e7a644ce19..76286f4dbf 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/lib/Analysis/GRExprEngineInternalChecks.cpp @@ -39,154 +39,143 @@ ExplodedNode<GRState>* GetNode(GRExprEngine::undef_arg_iterator I) { //===----------------------------------------------------------------------===// namespace { -class VISIBILITY_HIDDEN BuiltinBug : public BugTypeCacheLocation { +class VISIBILITY_HIDDEN BuiltinBug : public BugType { + GRExprEngine &Eng; protected: - const char* name; - const char* desc; + const std::string desc; public: - BuiltinBug(const char* n, const char* d = 0) : name(n), desc(d) {} - virtual const char* getName() const { return name; } - virtual const char* getDescription() const { - return desc ? desc : name; - } + BuiltinBug(GRExprEngine *eng, const char* n, const char* d) + : BugType(n, "Logic Errors"), Eng(*eng), desc(d) {} + + BuiltinBug(GRExprEngine *eng, const char* n) + : BugType(n, "Logic Errors"), Eng(*eng), desc(n) {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) = 0; - virtual void EmitWarnings(BugReporter& BR) { - EmitBuiltinWarnings(BR, cast<GRBugReporter>(BR).getEngine()); - } + virtual void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) = 0; + + void FlushReports(BugReporter& BR) { FlushReportsImpl(BR, Eng); } template <typename ITER> void Emit(BugReporter& BR, ITER I, ITER E) { - for (; I != E; ++I) { - BugReport R(*this, GetNode(I)); - BR.EmitWarning(R); - } - } - - virtual const char* getCategory() const { return "Logic Errors"; } + for (; I != E; ++I) BR.EmitReport(new BugReport(*this, desc.c_str(), + GetNode(I))); + } }; class VISIBILITY_HIDDEN NullDeref : public BuiltinBug { public: - NullDeref() : BuiltinBug("null dereference", - "Dereference of null pointer.") {} + NullDeref(GRExprEngine* eng) + : BuiltinBug(eng,"null dereference", "Dereference of null pointer.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.null_derefs_begin(), Eng.null_derefs_end()); } }; class VISIBILITY_HIDDEN UndefinedDeref : public BuiltinBug { public: - UndefinedDeref() : BuiltinBug("uninitialized pointer dereference", - "Dereference of undefined value.") {} + UndefinedDeref(GRExprEngine* eng) + : BuiltinBug(eng,"uninitialized pointer dereference", + "Dereference of undefined value.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.undef_derefs_begin(), Eng.undef_derefs_end()); } }; class VISIBILITY_HIDDEN DivZero : public BuiltinBug { public: - DivZero() : BuiltinBug("divide-by-zero", - "Division by zero/undefined value.") {} + DivZero(GRExprEngine* eng) + : BuiltinBug(eng,"divide-by-zero", "Division by zero/undefined value.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.explicit_bad_divides_begin(), Eng.explicit_bad_divides_end()); } }; class VISIBILITY_HIDDEN UndefResult : public BuiltinBug { public: - UndefResult() : BuiltinBug("undefined result", + UndefResult(GRExprEngine* eng) : BuiltinBug(eng,"undefined result", "Result of operation is undefined.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.undef_results_begin(), Eng.undef_results_end()); } }; class VISIBILITY_HIDDEN BadCall : public BuiltinBug { public: - BadCall() - : BuiltinBug("invalid function call", + BadCall(GRExprEngine *eng) + : BuiltinBug(eng,"invalid function call", "Called function is a NULL or undefined function pointer value.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.bad_calls_begin(), Eng.bad_calls_end()); } }; class VISIBILITY_HIDDEN BadArg : public BuiltinBug { public: - BadArg() : BuiltinBug("uninitialized argument", + BadArg(GRExprEngine* eng) : BuiltinBug(eng,"uninitialized argument", "Pass-by-value argument in function is undefined.") {} - BadArg(const char* d) : BuiltinBug("uninitialized argument", d) {} + BadArg(GRExprEngine* eng, const char* d) + : BuiltinBug(eng,"uninitialized argument", d) {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { for (GRExprEngine::UndefArgsTy::iterator I = Eng.undef_arg_begin(), E = Eng.undef_arg_end(); I!=E; ++I) { - // Generate a report for this bug. - RangedBugReport report(*this, I->first); - report.addRange(I->second->getSourceRange()); - - // Emit the warning. - BR.EmitWarning(report); + RangedBugReport *report = new RangedBugReport(*this, desc.c_str(), + I->first); + report->addRange(I->second->getSourceRange()); + BR.EmitReport(report); } } }; class VISIBILITY_HIDDEN BadMsgExprArg : public BadArg { public: - BadMsgExprArg() - : BadArg("Pass-by-value argument in message expression is undefined.") {} + BadMsgExprArg(GRExprEngine* eng) + : BadArg(eng,"Pass-by-value argument in message expression is undefined."){} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { for (GRExprEngine::UndefArgsTy::iterator I=Eng.msg_expr_undef_arg_begin(), - E = Eng.msg_expr_undef_arg_end(); I!=E; ++I) { - + E = Eng.msg_expr_undef_arg_end(); I!=E; ++I) { // Generate a report for this bug. - RangedBugReport report(*this, I->first); - report.addRange(I->second->getSourceRange()); - - // Emit the warning. - BR.EmitWarning(report); + RangedBugReport *report = new RangedBugReport(*this, desc.c_str(), I->first); + report->addRange(I->second->getSourceRange()); + BR.EmitReport(report); } } }; class VISIBILITY_HIDDEN BadReceiver : public BuiltinBug { public: - BadReceiver() - : BuiltinBug("uninitialized receiver", + BadReceiver(GRExprEngine* eng) + : BuiltinBug(eng,"uninitialized receiver", "Receiver in message expression is an uninitialized value.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { for (GRExprEngine::ErrorNodes::iterator I=Eng.undef_receivers_begin(), End = Eng.undef_receivers_end(); I!=End; ++I) { // Generate a report for this bug. - RangedBugReport report(*this, *I); - + RangedBugReport *report = new RangedBugReport(*this, desc.c_str(), *I); ExplodedNode<GRState>* N = *I; Stmt *S = cast<PostStmt>(N->getLocation()).getStmt(); Expr* E = cast<ObjCMessageExpr>(S)->getReceiver(); assert (E && "Receiver cannot be NULL"); - report.addRange(E->getSourceRange()); - - // Emit the warning. - BR.EmitWarning(report); + report->addRange(E->getSourceRange()); + BR.EmitReport(report); } } }; class VISIBILITY_HIDDEN RetStack : public BuiltinBug { public: - RetStack() : BuiltinBug("return of stack address") {} + RetStack(GRExprEngine* eng) : BuiltinBug(eng, "return of stack address") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { for (GRExprEngine::ret_stackaddr_iterator I=Eng.ret_stackaddr_begin(), End = Eng.ret_stackaddr_end(); I!=End; ++I) { @@ -232,22 +221,20 @@ public: << V.getRegion()->getString() << "' returned."; } - RangedBugReport report(*this, N, os.str().c_str()); - report.addRange(E->getSourceRange()); - if (R.isValid()) report.addRange(R); - - // Emit the warning. - BR.EmitWarning(report); + RangedBugReport *report = new RangedBugReport(*this, os.str().c_str(), N); + report->addRange(E->getSourceRange()); + if (R.isValid()) report->addRange(R); + BR.EmitReport(report); } } }; class VISIBILITY_HIDDEN RetUndef : public BuiltinBug { public: - RetUndef() : BuiltinBug("uninitialized return value", + RetUndef(GRExprEngine* eng) : BuiltinBug(eng,"uninitialized return value", "Uninitialized or undefined return value returned to caller.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.ret_undef_begin(), Eng.ret_undef_end()); } }; @@ -276,11 +263,11 @@ class VISIBILITY_HIDDEN UndefBranch : public BuiltinBug { }; public: - UndefBranch() - : BuiltinBug("uninitialized value", + UndefBranch(GRExprEngine *eng) + : BuiltinBug(eng,"uninitialized value", "Branch condition evaluates to an uninitialized value.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { for (GRExprEngine::undef_branch_iterator I=Eng.undef_branches_begin(), E=Eng.undef_branches_end(); I!=E; ++I) { @@ -289,7 +276,6 @@ public: // branch condition." We do a recursive walk of the condition's // subexpressions and roughly look for the most nested subexpression // that binds to Undefined. We then highlight that expression's range. - BlockEdge B = cast<BlockEdge>((*I)->getLocation()); Expr* Ex = cast<Expr>(B.getSrc()->getTerminatorCondition()); assert (Ex && "Block must have a terminator."); @@ -298,7 +284,6 @@ public: // being the terminator condition. We want to inspect the state // of that node instead because it will contain main information about // the subexpressions. - assert (!(*I)->pred_empty()); // Note: any predecessor will do. They should have identical state, @@ -306,7 +291,6 @@ public: // had to already be undefined. ExplodedNode<GRState> *N = *(*I)->pred_begin(); ProgramPoint P = N->getLocation(); - const GRState* St = (*I)->getState(); if (PostStmt* PS = dyn_cast<PostStmt>(&P)) @@ -316,31 +300,29 @@ public: FindUndefExpr FindIt(Eng.getStateManager(), St); Ex = FindIt.FindExpr(Ex); - RangedBugReport R(*this, *I); - R.addRange(Ex->getSourceRange()); - - BR.EmitWarning(R); + RangedBugReport *R = new RangedBugReport(*this, desc.c_str(), *I); + R->addRange(Ex->getSourceRange()); + BR.EmitReport(R); } } }; class VISIBILITY_HIDDEN OutOfBoundMemoryAccess : public BuiltinBug { public: - OutOfBoundMemoryAccess() : BuiltinBug("out-of-bound memory access", - "Load or store into an out-of-bound memory position.") {} + OutOfBoundMemoryAccess(GRExprEngine* eng) + : BuiltinBug(eng,"out-of-bound memory access", + "Load or store into an out-of-bound memory position.") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.explicit_oob_memacc_begin(), Eng.explicit_oob_memacc_end()); } }; class VISIBILITY_HIDDEN BadSizeVLA : public BuiltinBug { - public: - BadSizeVLA() : BuiltinBug("Zero-sized VLA", - "VLAs with zero-size are undefined.") {} + BadSizeVLA(GRExprEngine* eng) : BuiltinBug(eng, "bad VLA size") {} - virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) { + void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { for (GRExprEngine::ErrorNodes::iterator I = Eng.ExplicitBadSizedVLA.begin(), E = Eng.ExplicitBadSizedVLA.end(); I!=E; ++I) { @@ -358,24 +340,16 @@ public: std::string buf; llvm::raw_string_ostream os(buf); os << "The expression used to specify the number of elements in the VLA '" - << VD->getNameAsString() << "' evaluates to "; + << VD->getNameAsString() << "' evaluates to "; - SVal X = Eng.getStateManager().GetSVal(N->getState(), SizeExpr); - if (X.isUndef()) { - name = "Undefined size for VLA"; + if (Eng.getStateManager().GetSVal(N->getState(), SizeExpr).isUndef()) os << "an undefined or garbage value."; - } - else { - name = "Zero-sized VLA"; - os << " to 0. VLAs with no elements have undefined behavior."; - } + else + os << "0. VLAs with no elements have undefined behavior."; - desc = os.str().c_str(); - RangedBugReport report(*this, N); - report.addRange(SizeExpr->getSourceRange()); - - // Emit the warning. - BR.EmitWarning(report); + RangedBugReport *report = new RangedBugReport(*this, os.str().c_str(), N); + report->addRange(SizeExpr->getSourceRange()); + BR.EmitReport(report); } } }; @@ -384,13 +358,11 @@ public: // __attribute__(nonnull) checking class VISIBILITY_HIDDEN CheckAttrNonNull : public GRSimpleAPICheck { - SimpleBugType BT; - std::list<RangedBugReport> Reports; + BugType *BT; + BugReporter &BR; public: - CheckAttrNonNull() : - BT("'nonnull' argument passed null", "API", - "Null pointer passed as an argument to a 'nonnull' parameter") {} + CheckAttrNonNull(BugReporter &br) : BT(0), BR(br) {} virtual bool Audit(ExplodedNode<GRState>* N, GRStateManager& VMgr) { CallExpr* CE = cast<CallExpr>(cast<PostStmt>(N->getLocation()).getStmt()); @@ -408,7 +380,6 @@ public: return false; // Iterate through the arguments of CE and check them for null. - unsigned idx = 0; bool hasError = false; @@ -417,40 +388,54 @@ public: if (!VMgr.isEqual(state, *I, 0) || !Att->isNonNull(idx)) continue; + + // Lazily allocate the BugType object if it hasn't already been created. + // Ownership is transferred to the BugReporter object once the BugReport + // is passed to 'EmitWarning'. + if (!BT) BT = new BugType("'nonnull' argument passed null", "API"); - RangedBugReport R(BT, N); - R.addRange((*I)->getSourceRange()); - Reports.push_back(R); + RangedBugReport *R = new RangedBugReport(*BT, + "Null pointer passed as an argument to a " + "'nonnull' parameter", N); + + R->addRange((*I)->getSourceRange()); + BR.EmitReport(R); hasError = true; } return hasError; } - - virtual void EmitWarnings(BugReporter& BR) { - for (std::list<RangedBugReport>::iterator I=Reports.begin(), - E=Reports.end(); I!=E; ++I) - BR.EmitWarning(*I); - } }; } // end anonymous namespace //===----------------------------------------------------------------------===// // Check registration. +//===----------------------------------------------------------------------===// void GRExprEngine::RegisterInternalChecks() { - Register(new NullDeref()); - Register(new UndefinedDeref()); - Register(new UndefBranch()); - Register(new DivZero()); - Register(new UndefResult()); - Register(new BadCall()); - Register(new RetStack()); - Register(new RetUndef()); - Register(new BadArg()); - Register(new BadMsgExprArg()); - Register(new BadReceiver()); - Register(new OutOfBoundMemoryAccess()); - Register(new BadSizeVLA()); - AddCheck(new CheckAttrNonNull(), Stmt::CallExprClass); + // Register internal "built-in" BugTypes with the BugReporter. These BugTypes + // are different than what probably many checks will do since they don't + // create BugReports on-the-fly but instead wait until GRExprEngine finishes + // analyzing a function. Generation of BugReport objects is done via a call + // to 'FlushReports' from BugReporter. + BR.Register(new NullDeref(this)); + BR.Register(new UndefinedDeref(this)); + BR.Register(new UndefBranch(this)); + BR.Register(new DivZero(this)); + BR.Register(new UndefResult(this)); + BR.Register(new BadCall(this)); + BR.Register(new RetStack(this)); + BR.Register(new RetUndef(this)); + BR.Register(new BadArg(this)); + BR.Register(new BadMsgExprArg(this)); + BR.Register(new BadReceiver(this)); + BR.Register(new OutOfBoundMemoryAccess(this)); + BR.Register(new BadSizeVLA(this)); + + // The following checks do not need to have their associated BugTypes + // explicitly registered with the BugReporter. If they issue any BugReports, + // their associated BugType will get registered with the BugReporter + // automatically. Note that the check itself is owned by the GRExprEngine + // object. + AddCheck(new CheckAttrNonNull(BR), Stmt::CallExprClass); } diff --git a/lib/Analysis/GRTransferFuncs.cpp b/lib/Analysis/GRTransferFuncs.cpp index 16d083c50f..d621edddc6 100644 --- a/lib/Analysis/GRTransferFuncs.cpp +++ b/lib/Analysis/GRTransferFuncs.cpp @@ -17,8 +17,6 @@ using namespace clang; -void GRTransferFuncs::RegisterChecks(GRExprEngine& Eng) {} - void GRTransferFuncs::EvalStore(ExplodedNodeSet<GRState>& Dst, GRExprEngine& Eng, GRStmtNodeBuilder<GRState>& Builder, diff --git a/test/Analysis/CFDateGC.m b/test/Analysis/CFDateGC.m index c405fe24e5..f4f75e996f 100644 --- a/test/Analysis/CFDateGC.m +++ b/test/Analysis/CFDateGC.m @@ -70,8 +70,8 @@ CFAbsoluteTime f2_noleak() { } void f3_leak_with_gc() { - CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); - [[(id) date retain] release]; // expected-warning{{leak}} + CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); // expected-warning{{leak}} + [[(id) date retain] release]; } // The following test case verifies that we "stop tracking" a retained object diff --git a/test/Analysis/CGColorSpace.c b/test/Analysis/CGColorSpace.c index a46448c917..b229bd2e24 100644 --- a/test/Analysis/CGColorSpace.c +++ b/test/Analysis/CGColorSpace.c @@ -7,8 +7,8 @@ extern CGColorSpaceRef CGColorSpaceRetain(CGColorSpaceRef space); extern void CGColorSpaceRelease(CGColorSpaceRef space); void f() { - CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); - CGColorSpaceRetain(X); // expected-warning{{leak}} + CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // expected-warning{{leak}} + CGColorSpaceRetain(X); } void fb() { diff --git a/test/Analysis/NSPanel.m b/test/Analysis/NSPanel.m index 5f9f26c88b..7e2d0a807f 100644 --- a/test/Analysis/NSPanel.m +++ b/test/Analysis/NSPanel.m @@ -80,9 +80,9 @@ extern NSString *NSWindowDidBecomeKeyNotification; } - (void)myMethod2 { - NSPanel *panel = [[NSPanel alloc] initWithContentRect:NSMakeRect(0, 0, 200, 200) styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:(BOOL)1]; + NSPanel *panel = [[NSPanel alloc] initWithContentRect:NSMakeRect(0, 0, 200, 200) styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:(BOOL)1]; // expected-warning{{leak}} - [panels addObject:panel]; // expected-warning{{leak}} + [panels addObject:panel]; } @end diff --git a/test/Analysis/NSString.m b/test/Analysis/NSString.m index b123d62337..297c63d3e9 100644 --- a/test/Analysis/NSString.m +++ b/test/Analysis/NSString.m @@ -109,12 +109,12 @@ NSArray *f6(NSString* s) { NSString* f7(NSString* s1, NSString* s2, NSString* s3) { NSString* s4 = (NSString*) - CFStringCreateWithFormat(kCFAllocatorDefault, 0, + CFStringCreateWithFormat(kCFAllocatorDefault, 0, // expected-warning{{leak}} (CFStringRef) __builtin___CFStringMakeConstantString("%@ %@ (%@)"), s1, s2, s3); CFRetain(s4); - return s4; // expected-warning{{leak}} + return s4; } NSMutableArray* f8() { diff --git a/test/Analysis/NSWindow.m b/test/Analysis/NSWindow.m index bcb6c41aba..33d77528f9 100644 --- a/test/Analysis/NSWindow.m +++ b/test/Analysis/NSWindow.m @@ -67,7 +67,7 @@ void f2() { } void f2b() { - NSWindow *window = [[NSWindow alloc] + NSWindow *window = [[NSWindow alloc] // expected-warning{{leak}} initWithContentRect:NSMakeRect(0,0,100,100) styleMask:NSTitledWindowMask|NSClosableWindowMask backing:NSBackingStoreBuffered @@ -76,7 +76,7 @@ void f2b() { [window orderFrontRegardless]; - [window retain]; // expected-warning{{leak}} + [window retain]; } diff --git a/test/Analysis/refcnt_naming.m b/test/Analysis/refcnt_naming.m index 44ccf27a56..0fa95ebc8b 100644 --- a/test/Analysis/refcnt_naming.m +++ b/test/Analysis/refcnt_naming.m @@ -23,8 +23,8 @@ typedef signed char BOOL; - (NSURL *)myMethod:(NSString *)inString { - NSURL *url = (NSURL *)CFURLCreateWithString(0, (CFStringRef)inString, 0); - return url; // expected-warning{{leak}} + NSURL *url = (NSURL *)CFURLCreateWithString(0, (CFStringRef)inString, 0); // expected-warning{{leak}} + return url; } - (NSURL *)getMethod:(NSString *)inString diff --git a/test/Analysis/retain-release.m b/test/Analysis/retain-release.m index 66ff991b09..9a5a855947 100644 --- a/test/Analysis/retain-release.m +++ b/test/Analysis/retain-release.m @@ -130,28 +130,28 @@ CFAbsoluteTime f3() { CFAbsoluteTime f5(int x) { CFAbsoluteTime t = CFAbsoluteTimeGetCurrent(); - CFDateRef date = CFDateCreate(0, t); + CFDateRef date = CFDateCreate(0, t); // expected-warning{{leak}} if (x) CFRelease(date); - return t; // expected-warning{{leak}} + return t; } // Test a leak involving the return. CFDateRef f6(int x) { - CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); + CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); // expected-warning{{leak}} CFRetain(date); - return date; // expected-warning{{leak}} + return date; } // Test a leak involving an overwrite. CFDateRef f7() { - CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); + CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); //expected-warning{{leak}} CFRetain(date); - date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); //expected-warning{{leak}} + date = CFDateCreate(0, CFAbsoluteTimeGetCurrent()); return date; } @@ -160,9 +160,9 @@ CFDateRef f7() { CFDateRef MyDateCreate(); CFDateRef f8() { - CFDateRef date = MyDateCreate(); + CFDateRef date = MyDateCreate(); // expected-warning{{leak}} CFRetain(date); - return date; // expected-warning{{leak}} + return date; } CFDateRef f9() { @@ -179,24 +179,24 @@ CFDateRef f9() { // http://developer.apple.com/DOCUMENTATION/DARWIN/Reference/DiscArbitrationFramework/ // void f10(io_service_t media, DADiskRef d, CFStringRef s) { - DADiskRef disk = DADiskCreateFromBSDName(kCFAllocatorDefault, 0, "hello"); - if (disk) NSLog(@"ok"); // expected-warning{{leak}} + DADiskRef disk = DADiskCreateFromBSDName(kCFAllocatorDefault, 0, "hello"); // expected-warning{{leak}} + if (disk) NSLog(@"ok"); - disk = DADiskCreateFromIOMedia(kCFAllocatorDefault, 0, media); - if (disk) NSLog(@"ok"); // expected-warning{{leak}} + disk = DADiskCreateFromIOMedia(kCFAllocatorDefault, 0, media); // expected-warning{{leak}} + if (disk) NSLog(@"ok"); - CFDictionaryRef dict = DADiskCopyDescription(d); - if (dict) NSLog(@"ok"); // expected-warning{{leak}} + CFDictionaryRef dict = DADiskCopyDescription(d); // expected-warning{{leak}} + if (dict) NSLog(@"ok"); - disk = DADiskCopyWholeDisk(d); - if (disk) NSLog(@"ok"); // expected-warning{{leak}} + disk = DADiskCopyWholeDisk(d); // expected-warning{{leak}} + if (disk) NSLog(@"ok"); - DADissenterRef dissenter = DADissenterCreate(kCFAllocatorDefault, + DADissenterRef dissenter = DADissenterCreate(kCFAllocatorDefault, // expected-warning{{leak}} kDAReturnSuccess, s); - if (dissenter) NSLog(@"ok"); // expected-warning{{leak}} + if (dissenter) NSLog(@"ok"); - DASessionRef session = DASessionCreate(kCFAllocatorDefault); - if (session) NSLog(@"ok"); // expected-warning{{leak}} + DASessionRef session = DASessionCreate(kCFAllocatorDefault); // expected-warning{{leak}} + if (session) NSLog(@"ok"); } // Test retain/release checker with CFString and CFMutableArray. |