From: Ted Kremenek Date: Fri, 11 Jul 2008 18:37:32 +0000 (+0000) Subject: Refactored auditor interface within GRExprEngine and GRCoreEngine to use a "batch... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bdb435ddaafd5069becd543d638112f68825b89d;p=clang Refactored auditor interface within GRExprEngine and GRCoreEngine to use a "batch auditor" to dispatch to specialized auditors instead of having a separate vector for each audited Expr*. This not only provides a much cleaner implementation, but also allows us to install auditors for any expression. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@53464 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/PathSensitive/GRCoreEngine.h b/include/clang/Analysis/PathSensitive/GRCoreEngine.h index 32fe9d3342..a618e9ffe6 100644 --- a/include/clang/Analysis/PathSensitive/GRCoreEngine.h +++ b/include/clang/Analysis/PathSensitive/GRCoreEngine.h @@ -165,36 +165,26 @@ public: template class GRStmtNodeBuilder { +public: typedef STATE StateTy; typedef ExplodedNode NodeTy; +private: GRStmtNodeBuilderImpl& NB; - const StateTy* CleanedState; - - GRAuditor **CallExprAuditBeg, **CallExprAuditEnd; - GRAuditor **ObjCMsgExprAuditBeg, **ObjCMsgExprAuditEnd; + const StateTy* CleanedState; + GRAuditor* Auditor; public: GRStmtNodeBuilder(GRStmtNodeBuilderImpl& nb) : NB(nb), - CallExprAuditBeg(0), CallExprAuditEnd(0), - ObjCMsgExprAuditBeg(0), ObjCMsgExprAuditEnd(0), - PurgingDeadSymbols(false), + Auditor(0), PurgingDeadSymbols(false), BuildSinks(false), HasGeneratedNode(false) { CleanedState = getLastNode()->getState(); } - - void setObjCMsgExprAuditors(GRAuditor **B, - GRAuditor **E) { - ObjCMsgExprAuditBeg = B; - ObjCMsgExprAuditEnd = E; + + void setAuditor(GRAuditor* A) { + Auditor = A; } - - void setCallExprAuditors(GRAuditor **B, - GRAuditor **E) { - CallExprAuditBeg = B; - CallExprAuditEnd = E; - } NodeTy* getLastNode() const { return static_cast(NB.getLastNode()); @@ -241,23 +231,9 @@ public: NodeTy* Pred, const StateTy* St) { const StateTy* PredState = GetState(Pred); - - GRAuditor **AB = NULL, **AE = NULL; - - switch (S->getStmtClass()) { - default: break; - case Stmt::CallExprClass: - AB = CallExprAuditBeg; - AE = CallExprAuditEnd; - break; - case Stmt::ObjCMessageExprClass: - AB = ObjCMsgExprAuditBeg; - AE = ObjCMsgExprAuditEnd; - break; - } - + // If the state hasn't changed, don't generate a new node. - if (!BuildSinks && St == PredState && AB == NULL) { + if (!BuildSinks && St == PredState && Auditor == 0) { Dst.Add(Pred); return NULL; } @@ -268,9 +244,8 @@ public: if (BuildSinks) N->markAsSink(); else { - for ( ; AB != AE; ++AB) - if ((*AB)->Audit(N)) - N->markAsSink(); + if (Auditor && Auditor->Audit(N)) + N->markAsSink(); Dst.Add(N); } diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index 97699ca902..1bae83b982 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -98,10 +98,7 @@ protected: Selector* NSExceptionInstanceRaiseSelectors; Selector RaiseSel; - typedef llvm::SmallVector SimpleChecksTy; - - SimpleChecksTy CallChecks; - SimpleChecksTy MsgExprChecks; + llvm::OwningPtr BatchAuditor; public: typedef llvm::SmallPtrSet UndefBranchesTy; @@ -347,21 +344,7 @@ public: return UndefReceivers.end(); } - typedef SimpleChecksTy::iterator simple_checks_iterator; - - simple_checks_iterator call_auditors_begin() { return CallChecks.begin(); } - simple_checks_iterator call_auditors_end() { return CallChecks.end(); } - - simple_checks_iterator msgexpr_auditors_begin() { - return MsgExprChecks.begin(); - } - simple_checks_iterator msgexpr_auditors_end() { - return MsgExprChecks.end(); - } - - void AddCallCheck(GRSimpleAPICheck* A); - - void AddObjCMessageExprCheck(GRSimpleAPICheck* A); + void AddCheck(GRSimpleAPICheck* A, Stmt::StmtClass C); /// ProcessStmt - Called by GRCoreEngine. Used to generate new successor /// nodes by processing the 'effects' of a block-level statement. diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index 96e7fae7e3..fa8009f3ef 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -13,12 +13,13 @@ // //===----------------------------------------------------------------------===// +#include "clang/Analysis/PathSensitive/BasicStore.h" #include "clang/Analysis/PathSensitive/GRExprEngine.h" #include "clang/Analysis/PathSensitive/BugReporter.h" #include "clang/Basic/SourceManager.h" #include "llvm/Support/Streams.h" - -#include "clang/Analysis/PathSensitive/BasicStore.h" +#include "llvm/ADT/ImmutableList.h" +#include "llvm/Support/Compiler.h" #ifndef NDEBUG #include "llvm/Support/GraphWriter.h" @@ -34,6 +35,79 @@ using llvm::APSInt; // Engine construction and deletion. //===----------------------------------------------------------------------===// +namespace { + +class VISIBILITY_HIDDEN MappedBatchAuditor : public GRSimpleAPICheck { + typedef llvm::ImmutableList Checks; + typedef llvm::DenseMap MapTy; + + MapTy M; + Checks::Factory F; + +public: + MappedBatchAuditor(llvm::BumpPtrAllocator& Alloc) : F(Alloc) {} + + virtual ~MappedBatchAuditor() { + llvm::DenseSet 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; + + AlreadyVisited.insert(check); + delete check; + } + } + + void AddCheck(GRSimpleAPICheck* A, Stmt::StmtClass C) { + assert (A && "Check cannot be null."); + void* key = reinterpret_cast((uintptr_t) C); + 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 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) { + Stmt* S = cast(N->getLocation()).getStmt(); + void* key = reinterpret_cast((uintptr_t) S->getStmtClass()); + MapTy::iterator MI = M.find(key); + + if (MI == M.end()) + return false; + + bool isSink = false; + + for (Checks::iterator I=MI->second.begin(), E=MI->second.end(); I!=E; ++I) + isSink |= (*I)->Audit(N); + + return isSink; + } +}; + +} // end anonymous namespace + +//===----------------------------------------------------------------------===// +// Engine construction and deletion. +//===----------------------------------------------------------------------===// + static inline Selector GetNullarySelector(const char* name, ASTContext& Ctx) { IdentifierInfo* II = &Ctx.Idents.get(name); return Ctx.Selectors.getSelector(0, &II); @@ -58,14 +132,7 @@ GRExprEngine::GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx, GRExprEngine::~GRExprEngine() { for (BugTypeSet::iterator I = BugTypes.begin(), E = BugTypes.end(); I!=E; ++I) delete *I; - - for (SimpleChecksTy::iterator I = CallChecks.begin(), E = CallChecks.end(); - I != E; ++I) - delete *I; - - for (SimpleChecksTy::iterator I=MsgExprChecks.begin(), E=MsgExprChecks.end(); - I != E; ++I) - delete *I; + delete [] NSExceptionInstanceRaiseSelectors; } @@ -104,16 +171,9 @@ void GRExprEngine::EmitWarnings(BugReporterData& BRData) { (*I)->EmitWarnings(BR); } - for (SimpleChecksTy::iterator I = CallChecks.begin(), E = CallChecks.end(); - I != E; ++I) { - GRBugReporter BR(BRData, *this); - (*I)->EmitWarnings(BR); - } - - for (SimpleChecksTy::iterator I=MsgExprChecks.begin(), E=MsgExprChecks.end(); - I != E; ++I) { + if (BatchAuditor) { GRBugReporter BR(BRData, *this); - (*I)->EmitWarnings(BR); + BatchAuditor->EmitWarnings(BR); } } @@ -122,12 +182,11 @@ void GRExprEngine::setTransferFunctions(GRTransferFuncs* tf) { TF->RegisterChecks(*this); } -void GRExprEngine::AddCallCheck(GRSimpleAPICheck* A) { - CallChecks.push_back(A); -} - -void GRExprEngine::AddObjCMessageExprCheck(GRSimpleAPICheck* A) { - MsgExprChecks.push_back(A); +void GRExprEngine::AddCheck(GRSimpleAPICheck* A, Stmt::StmtClass C) { + if (!BatchAuditor) + BatchAuditor.reset(new MappedBatchAuditor(getGraph().getAllocator())); + + ((MappedBatchAuditor*) BatchAuditor.get())->AddCheck(A, C); } const ValueState* GRExprEngine::getInitialState() { @@ -186,26 +245,14 @@ void GRExprEngine::ProcessStmt(Stmt* S, StmtNodeBuilder& builder) { CurrentStmt = S; // Set up our simple checks. + if (BatchAuditor) + Builder->setAuditor(BatchAuditor.get()); - // FIXME: This can probably be installed directly in GRCoreEngine, obviating - // the need to do a copy every time we hit a block-level statement. - - if (!MsgExprChecks.empty()) - Builder->setObjCMsgExprAuditors((GRAuditor**) &MsgExprChecks[0], - (GRAuditor**) (&MsgExprChecks[0] + MsgExprChecks.size())); - - - if (!CallChecks.empty()) - Builder->setCallExprAuditors((GRAuditor**) &CallChecks[0], - (GRAuditor**) (&CallChecks[0] + CallChecks.size())); - - // Create the cleaned state. - + // Create the cleaned state. CleanedState = StateMgr.RemoveDeadBindings(EntryNode->getState(), CurrentStmt, Liveness, DeadSymbols); // Process any special transfer function for dead symbols. - NodeSet Tmp; if (DeadSymbols.empty()) diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp index 7d7afe39af..860a786089 100644 --- a/lib/Analysis/GRSimpleVals.cpp +++ b/lib/Analysis/GRSimpleVals.cpp @@ -354,11 +354,11 @@ void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) { ASTContext& Ctx = Eng.getContext(); ValueStateManager* VMgr = &Eng.getStateManager(); - GRSimpleAPICheck* Check = CreateBasicObjCFoundationChecks(Ctx, VMgr); - Eng.AddObjCMessageExprCheck(Check); + GRSimpleAPICheck* Check = CreateBasicObjCFoundationChecks(Ctx, VMgr); + Eng.AddCheck(Check, Stmt::ObjCMessageExprClass); - Check = CreateAuditCFNumberCreate(Ctx, VMgr); - Eng.AddCallCheck(Check); + Check = CreateAuditCFNumberCreate(Ctx, VMgr); + Eng.AddCheck(Check, Stmt::CallExprClass); }