From e5716cbae7f67cabc64edccfdaf02ef64115fd4a Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Thu, 3 Dec 2009 03:27:11 +0000 Subject: [PATCH] Add batch version of 'StoreManager::InvalidateRegion()' for invalidating multiple regions as once. After adopting this in the CFRefCount::EvalCall(), we see a reduction in analysis time of 1.5% when analyzing all of SQLite3. llvm-svn: 90405 --- .../clang/Analysis/PathSensitive/Store.h | 6 +++ clang/lib/Analysis/CFRefCount.cpp | 49 ++++++++++++------- clang/lib/Analysis/RegionStore.cpp | 32 +++++++----- clang/lib/Analysis/Store.cpp | 11 +++++ 4 files changed, 70 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Analysis/PathSensitive/Store.h b/clang/include/clang/Analysis/PathSensitive/Store.h index 55fa83d9ecc3..51f09c30df5c 100644 --- a/clang/include/clang/Analysis/PathSensitive/Store.h +++ b/clang/include/clang/Analysis/PathSensitive/Store.h @@ -147,6 +147,12 @@ public: const MemRegion *R, const Expr *E, unsigned Count, InvalidatedSymbols *IS) = 0; + + virtual const GRState *InvalidateRegions(const GRState *state, + const MemRegion * const *Begin, + const MemRegion * const *End, + const Expr *E, unsigned Count, + InvalidatedSymbols *IS); // FIXME: Make out-of-line. virtual const GRState *setExtent(const GRState *state, diff --git a/clang/lib/Analysis/CFRefCount.cpp b/clang/lib/Analysis/CFRefCount.cpp index 0e4c1b1a0175..0b69a4c5ae3a 100644 --- a/clang/lib/Analysis/CFRefCount.cpp +++ b/clang/lib/Analysis/CFRefCount.cpp @@ -2786,6 +2786,8 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, Expr* ErrorExpr = NULL; SymbolRef ErrorSym = 0; + llvm::SmallVector RegionsToInvalidate; + for (ExprIterator I = arg_beg; I != arg_end; ++I, ++idx) { SVal V = state->getSValAsScalarOrLoc(*I); SymbolRef Sym = V.getAsLocSymbol(); @@ -2808,16 +2810,8 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, continue; // Invalidate the value of the variable passed by reference. - - // FIXME: We can have collisions on the conjured symbol if the - // expression *I also creates conjured symbols. We probably want - // to identify conjured symbols by an expression pair: the enclosing - // expression (the context) and the expression itself. This should - // disambiguate conjured symbols. - unsigned Count = Builder.getCurrentBlockCount(); - StoreManager& StoreMgr = Eng.getStateManager().getStoreManager(); - const MemRegion *R = MR->getRegion(); + // Are we dealing with an ElementRegion? If the element type is // a basic integer type (e.g., char, int) and the underying region // is a variable region then strip off the ElementRegion. @@ -2841,14 +2835,11 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, } // FIXME: What about layers of ElementRegions? } - - StoreManager::InvalidatedSymbols IS; - state = StoreMgr.InvalidateRegion(state, R, *I, Count, &IS); - for (StoreManager::InvalidatedSymbols::iterator I = IS.begin(), - E = IS.end(); I!=E; ++I) { - // Remove any existing reference-count binding. - state = state->remove(*I); - } + + // Mark this region for invalidation. We batch invalidate regions + // below for efficiency. + RegionsToInvalidate.push_back(R); + continue; } else { // Nuke all other arguments passed by reference. @@ -2864,6 +2855,30 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, goto tryAgain; } } + + // Invalidate regions we designed for invalidation use the batch invalidation + // API. + if (!RegionsToInvalidate.empty()) { + // FIXME: We can have collisions on the conjured symbol if the + // expression *I also creates conjured symbols. We probably want + // to identify conjured symbols by an expression pair: the enclosing + // expression (the context) and the expression itself. This should + // disambiguate conjured symbols. + unsigned Count = Builder.getCurrentBlockCount(); + StoreManager& StoreMgr = Eng.getStateManager().getStoreManager(); + + + StoreManager::InvalidatedSymbols IS; + state = StoreMgr.InvalidateRegions(state, RegionsToInvalidate.data(), + RegionsToInvalidate.data() + + RegionsToInvalidate.size(), + Ex, Count, &IS); + for (StoreManager::InvalidatedSymbols::iterator I = IS.begin(), + E = IS.end(); I!=E; ++I) { + // Remove any existing reference-count binding. + state = state->remove(*I); + } + } // Evaluate the effect on the message receiver. if (!ErrorExpr && Receiver) { diff --git a/clang/lib/Analysis/RegionStore.cpp b/clang/lib/Analysis/RegionStore.cpp index e645172a5b66..170abc8fe6f1 100644 --- a/clang/lib/Analysis/RegionStore.cpp +++ b/clang/lib/Analysis/RegionStore.cpp @@ -269,7 +269,15 @@ public: const GRState *InvalidateRegion(const GRState *state, const MemRegion *R, const Expr *E, unsigned Count, - InvalidatedSymbols *IS); + InvalidatedSymbols *IS) { + return RegionStoreManager::InvalidateRegions(state, &R, &R+1, E, Count, IS); + } + + const GRState *InvalidateRegions(const GRState *state, + const MemRegion * const *Begin, + const MemRegion * const *End, + const Expr *E, unsigned Count, + InvalidatedSymbols *IS); private: void RemoveSubRegionBindings(RegionBindings &B, const MemRegion *R, @@ -460,16 +468,14 @@ void RegionStoreManager::RemoveSubRegionBindings(RegionBindings &B, B = RBFactory.Remove(B, R); } -const GRState *RegionStoreManager::InvalidateRegion(const GRState *state, - const MemRegion *R, - const Expr *Ex, - unsigned Count, - InvalidatedSymbols *IS) { +const GRState *RegionStoreManager::InvalidateRegions(const GRState *state, + const MemRegion * const *I, + const MemRegion * const *E, + const Expr *Ex, + unsigned Count, + InvalidatedSymbols *IS) { ASTContext& Ctx = StateMgr.getContext(); - // Strip away casts. - R = R->StripCasts(); - // Get the mapping of regions -> subregions. llvm::OwningPtr SubRegions(getRegionStoreSubRegionMap(state->getStore())); @@ -478,10 +484,14 @@ const GRState *RegionStoreManager::InvalidateRegion(const GRState *state, llvm::DenseMap Visited; llvm::SmallVector WorkList; - WorkList.push_back(R); + + for ( ; I != E; ++I) { + // Strip away casts. + WorkList.push_back((*I)->StripCasts()); + } while (!WorkList.empty()) { - R = WorkList.back(); + const MemRegion *R = WorkList.back(); WorkList.pop_back(); // Have we visited this region before? diff --git a/clang/lib/Analysis/Store.cpp b/clang/lib/Analysis/Store.cpp index 2fd573c4764d..78eed4619be2 100644 --- a/clang/lib/Analysis/Store.cpp +++ b/clang/lib/Analysis/Store.cpp @@ -205,3 +205,14 @@ SVal StoreManager::CastRetrievedVal(SVal V, const TypedRegion *R, return V; } +const GRState *StoreManager::InvalidateRegions(const GRState *state, + const MemRegion * const *I, + const MemRegion * const *End, + const Expr *E, + unsigned Count, + InvalidatedSymbols *IS) { + for ( ; I != End ; ++I) + state = InvalidateRegion(state, *I, E, Count, IS); + + return state; +}