diff options
author | Ted Kremenek <kremenek@apple.com> | 2009-02-04 23:49:09 +0000 |
---|---|---|
committer | Ted Kremenek <kremenek@apple.com> | 2009-02-04 23:49:09 +0000 |
commit | fc5d067ea092a9e960a091f5f55b93ec41e7ad2a (patch) | |
tree | 07628d31b86c912e2b64f8f562f3d8fecd3dd600 /clang/lib/Analysis | |
parent | 400c040e01a5b946f50ec6e6395b14ed9a225477 (diff) | |
download | bcm5719-llvm-fc5d067ea092a9e960a091f5f55b93ec41e7ad2a.tar.gz bcm5719-llvm-fc5d067ea092a9e960a091f5f55b93ec41e7ad2a.zip |
Overhaul BugReporter interface and implementation. The new interface cleans up
the ownership of BugTypes and BugReports. Now BugReports are owned by BugTypes,
and BugTypes are owned by the BugReporter object.
The major functionality change in this patch is that reports are not immediately
emitted by a call to BugReporter::EmitWarning (now called EmitReport), but
instead of queued up in report "equivalence classes". When
BugReporter::FlushReports() is called, it emits one diagnostic per report
equivalence class. This provides a nice cleanup with the caching of reports as
well as enables the BugReporter engine to select the "best" path for reporting a
path-sensitive bug based on all the locations in the ExplodedGraph that the same
bug could occur.
Along with this patch, Leaks are now coalesced into a common equivalence class
by their allocation site, and the "summary" diagnostic for leaks now reports the
allocation site as the location of the bug (this may later be augmented to also
provide an example location where the leak occurs).
llvm-svn: 63796
Diffstat (limited to 'clang/lib/Analysis')
-rw-r--r-- | clang/lib/Analysis/BasicObjCFoundationChecks.cpp | 186 | ||||
-rw-r--r-- | clang/lib/Analysis/BasicObjCFoundationChecks.h | 11 | ||||
-rw-r--r-- | clang/lib/Analysis/BugReporter.cpp | 378 | ||||
-rw-r--r-- | clang/lib/Analysis/CFRefCount.cpp | 187 | ||||
-rw-r--r-- | clang/lib/Analysis/CheckNSError.cpp | 157 | ||||
-rw-r--r-- | clang/lib/Analysis/ExplodedGraph.cpp | 42 | ||||
-rw-r--r-- | clang/lib/Analysis/GRExprEngine.cpp | 59 | ||||
-rw-r--r-- | clang/lib/Analysis/GRExprEngineInternalChecks.cpp | 249 | ||||
-rw-r--r-- | clang/lib/Analysis/GRTransferFuncs.cpp | 2 |
9 files changed, 569 insertions, 702 deletions
diff --git a/clang/lib/Analysis/BasicObjCFoundationChecks.cpp b/clang/lib/Analysis/BasicObjCFoundationChecks.cpp index be729bee6c0..8a5c3e0a847 100644 --- a/clang/lib/Analysis/BasicObjCFoundationChecks.cpp +++ b/clang/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/clang/lib/Analysis/BasicObjCFoundationChecks.h b/clang/lib/Analysis/BasicObjCFoundationChecks.h index 3ad2ca57aa3..6c594ea7219 100644 --- a/clang/lib/Analysis/BasicObjCFoundationChecks.h +++ b/clang/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/clang/lib/Analysis/BugReporter.cpp b/clang/lib/Analysis/BugReporter.cpp index f59f89a1a82..c42218e506c 100644 --- a/clang/lib/Analysis/BugReporter.cpp +++ b/clang/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/clang/lib/Analysis/CFRefCount.cpp b/clang/lib/Analysis/CFRefCount.cpp index ab9d409dd21..e42fa8614cc 100644 --- a/clang/lib/Analysis/CFRefCount.cpp +++ b/clang/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/clang/lib/Analysis/CheckNSError.cpp b/clang/lib/Analysis/CheckNSError.cpp index 919c17d0646..a35497a6bfd 100644 --- a/clang/lib/Analysis/CheckNSError.cpp +++ b/clang/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/clang/lib/Analysis/ExplodedGraph.cpp b/clang/lib/Analysis/ExplodedGraph.cpp index 4e41b60da37..864a83b8847 100644 --- a/clang/lib/Analysis/ExplodedGraph.cpp +++ b/clang/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/clang/lib/Analysis/GRExprEngine.cpp b/clang/lib/Analysis/GRExprEngine.cpp index bb34763df7f..ea2ae3d92be 100644 --- a/clang/lib/Analysis/GRExprEngine.cpp +++ b/clang/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/clang/lib/Analysis/GRExprEngineInternalChecks.cpp b/clang/lib/Analysis/GRExprEngineInternalChecks.cpp index e7a644ce19e..76286f4dbf2 100644 --- a/clang/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/clang/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/clang/lib/Analysis/GRTransferFuncs.cpp b/clang/lib/Analysis/GRTransferFuncs.cpp index 16d083c50fb..d621edddc6d 100644 --- a/clang/lib/Analysis/GRTransferFuncs.cpp +++ b/clang/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, |