diff --git a/clang/lib/Analysis/RegionStore.cpp b/clang/lib/Analysis/RegionStore.cpp index 3b27150baa1f..9fe41bed2a36 100644 --- a/clang/lib/Analysis/RegionStore.cpp +++ b/clang/lib/Analysis/RegionStore.cpp @@ -168,13 +168,20 @@ public: class VISIBILITY_HIDDEN RegionStoreManager : public StoreManager { const RegionStoreFeatures Features; RegionBindings::Factory RBFactory; + + typedef llvm::DenseMap SMCache; + SMCache SC; + public: RegionStoreManager(GRStateManager& mgr, const RegionStoreFeatures &f) : StoreManager(mgr), Features(f), RBFactory(mgr.getAllocator()) {} - virtual ~RegionStoreManager() {} + virtual ~RegionStoreManager() { + for (SMCache::iterator I = SC.begin(), E = SC.end(); I != E; ++I) + delete (*I).second; + } SubRegionMap *getSubRegionMap(const GRState *state); @@ -1570,211 +1577,210 @@ RegionStoreManager::CopyLazyBindings(nonloc::LazyCompoundVal V, // State pruning. //===----------------------------------------------------------------------===// -static void UpdateLiveSymbols(SVal X, SymbolReaper& SymReaper) { - if (loc::MemRegionVal *XR = dyn_cast(&X)) { - const MemRegion *R = XR->getRegion(); - - while (R) { - if (const SymbolicRegion *SR = dyn_cast(R)) { - SymReaper.markLive(SR->getSymbol()); - return; - } - - if (const SubRegion *SR = dyn_cast(R)) { - R = SR->getSuperRegion(); - continue; - } - - break; - } - - return; - } - - for (SVal::symbol_iterator SI=X.symbol_begin(), SE=X.symbol_end();SI!=SE;++SI) - SymReaper.markLive(*SI); -} - namespace { -class VISIBILITY_HIDDEN TreeScanner { - RegionBindings B; - RegionDefaultBindings DB; - SymbolReaper &SymReaper; - llvm::DenseSet &Marked; - llvm::DenseSet &ScannedLazyVals; - RegionStoreSubRegionMap &M; - RegionStoreManager &RS; - llvm::SmallVectorImpl &RegionRoots; - const bool MarkKeys; +class VISIBILITY_HIDDEN RBDNode + : public std::pair { public: - TreeScanner(RegionBindings b, RegionDefaultBindings db, - SymbolReaper &symReaper, - llvm::DenseSet &marked, - llvm::DenseSet &scannedLazyVals, - RegionStoreSubRegionMap &m, RegionStoreManager &rs, - llvm::SmallVectorImpl ®ionRoots, - bool markKeys = true) - : B(b), DB(db), SymReaper(symReaper), Marked(marked), - ScannedLazyVals(scannedLazyVals), M(m), - RS(rs), RegionRoots(regionRoots), MarkKeys(markKeys) {} + RBDNode(const GRState *st, const MemRegion *r) + : std::pair(st, r) {} + + const GRState *getState() const { return first; } + const MemRegion *getRegion() const { return second; } +}; - void scanTree(const MemRegion *R); +enum VisitFlag { NotVisited = 0, VisitedFromSubRegion, VisitedFromSuperRegion }; + +class RBDItem : public RBDNode { +private: + const VisitFlag VF; + +public: + RBDItem(const GRState *st, const MemRegion *r, VisitFlag vf) + : RBDNode(st, r), VF(vf) {} + + VisitFlag getVisitFlag() const { return VF; } }; } // end anonymous namespace - - -void TreeScanner::scanTree(const MemRegion *R) { - if (MarkKeys) { - if (Marked.count(R)) - return; - - Marked.insert(R); - } - - // Mark the symbol for any live SymbolicRegion as "live". This means we - // should continue to track that symbol. - if (const SymbolicRegion* SymR = dyn_cast(R)) - SymReaper.markLive(SymR->getSymbol()); - - // Get the data binding for R (if any). - const SVal* Xptr = B.lookup(R); - - // Check for lazy bindings. - if (const nonloc::LazyCompoundVal *V = - dyn_cast_or_null(Xptr)) { - - const LazyCompoundValData *D = V->getCVData(); - - if (!ScannedLazyVals.count(D)) { - // Scan the bindings in the LazyCompoundVal. - ScannedLazyVals.insert(D); - - // FIXME: Cache subregion maps. - const GRState *lazyState = D->getState(); - - llvm::OwningPtr - lazySM(RS.getRegionStoreSubRegionMap(lazyState)); - - Store lazyStore = lazyState->getStore(); - RegionBindings lazyB = RS.GetRegionBindings(lazyStore); - - RegionDefaultBindings lazyDB = lazyState->get(); - - // Scan the bindings. - TreeScanner scan(lazyB, lazyDB, SymReaper, Marked, ScannedLazyVals, - *lazySM.get(), RS, RegionRoots, false); - - scan.scanTree(D->getRegion()); - } - } - else { - // No direct binding? Get the default binding for R (if any). - if (!Xptr) - Xptr = DB.lookup(R); - - // Direct or default binding? - if (Xptr) { - SVal X = *Xptr; - UpdateLiveSymbols(X, SymReaper); // Update the set of live symbols. - - // If X is a region, then add it to the RegionRoots. - if (const MemRegion *RX = X.getAsRegion()) { - RegionRoots.push_back(RX); - // Mark the super region of the RX as live. - // e.g.: int x; char *y = (char*) &x; if (*y) ... - // 'y' => element region. 'x' is its super region. - if (const SubRegion *SR = dyn_cast(RX)) { - RegionRoots.push_back(SR->getSuperRegion()); - } - } - } - } - - RegionStoreSubRegionMap::iterator I, E; - - for (llvm::tie(I, E) = M.begin_end(R); I != E; ++I) - scanTree(*I); -} - + void RegionStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc, SymbolReaper& SymReaper, llvm::SmallVectorImpl& RegionRoots) { Store store = state.getStore(); RegionBindings B = GetRegionBindings(store); - - // Lazily constructed backmap from MemRegions to SubRegions. - typedef llvm::ImmutableSet SubRegionsTy; - typedef llvm::ImmutableMap SubRegionsMapTy; + RegionDefaultBindings DVM = state.get(); // The backmap from regions to subregions. llvm::OwningPtr SubRegions(getRegionStoreSubRegionMap(&state)); - - // Do a pass over the regions in the store. For VarRegions we check if - // the variable is still live and if so add it to the list of live roots. - // For other regions we populate our region backmap. + + // Do a pass over the regions in the store. For VarRegions we check if + // the variable is still live and if so add it to the list of live roots. + // For other regions we populate our region backmap. llvm::SmallVector IntermediateRoots; - + // Scan the direct bindings for "intermediate" roots. for (RegionBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) { const MemRegion *R = I.getKey(); IntermediateRoots.push_back(R); } - + // Scan the default bindings for "intermediate" roots. - RegionDefaultBindings DVM = state.get(); for (RegionDefaultBindings::iterator I = DVM.begin(), E = DVM.end(); I != E; ++I) { const MemRegion *R = I.getKey(); IntermediateRoots.push_back(R); } - + // Process the "intermediate" roots to find if they are referenced by // real roots. + llvm::SmallVector WorkList; + llvm::DenseMap IntermediateVisited; + while (!IntermediateRoots.empty()) { const MemRegion* R = IntermediateRoots.back(); IntermediateRoots.pop_back(); - + + unsigned &visited = IntermediateVisited[R]; + if (visited) + continue; + visited = 1; + if (const VarRegion* VR = dyn_cast(R)) { - if (SymReaper.isLive(Loc, VR->getDecl())) { - RegionRoots.push_back(VR); // This is a live "root". - } + if (SymReaper.isLive(Loc, VR->getDecl())) + WorkList.push_back(RBDItem(&state, VR, VisitedFromSuperRegion)); continue; } - + if (const SymbolicRegion* SR = dyn_cast(R)) { if (SymReaper.isLive(SR->getSymbol())) - RegionRoots.push_back(SR); + WorkList.push_back(RBDItem(&state, SR, VisitedFromSuperRegion)); continue; } - - // Add the super region for R to the worklist if it is a subregion. + + // Add the super region for R to the worklist if it is a subregion. if (const SubRegion* superR = - dyn_cast(cast(R)->getSuperRegion())) + dyn_cast(cast(R)->getSuperRegion())) IntermediateRoots.push_back(superR); } - // Process the worklist of RegionRoots. This performs a "mark-and-sweep" - // of the store. We want to find all live symbols and dead regions. - llvm::DenseSet Marked; - llvm::DenseSet LazyVals; - TreeScanner TS(B, DVM, SymReaper, Marked, LazyVals, *SubRegions.get(), - *this, RegionRoots); - - while (!RegionRoots.empty()) { - const MemRegion *R = RegionRoots.back(); - RegionRoots.pop_back(); - TS.scanTree(R); + // Enqueue the RegionRoots onto WorkList. + for (llvm::SmallVectorImpl::iterator I=RegionRoots.begin(), + E=RegionRoots.end(); I!=E; ++I) { + WorkList.push_back(RBDItem(&state, *I, VisitedFromSuperRegion)); } + RegionRoots.clear(); + + // Process the worklist. + typedef llvm::DenseMap, VisitFlag> + VisitMap; + + VisitMap Visited; + + while (!WorkList.empty()) { + RBDItem N = WorkList.back(); + WorkList.pop_back(); + + // Have we visited this node before? + VisitFlag &VF = Visited[N]; + if (VF >= N.getVisitFlag()) + continue; + + const MemRegion *R = N.getRegion(); + const GRState *state_N = N.getState(); + + // Enqueue subregions? + if (N.getVisitFlag() == VisitedFromSuperRegion) { + RegionStoreSubRegionMap *M; + + if (&state == state_N) + M = SubRegions.get(); + else { + RegionStoreSubRegionMap *& SM = SC[state_N]; + if (!SM) + SM = getRegionStoreSubRegionMap(state_N); + M = SM; + } + + RegionStoreSubRegionMap::iterator I, E; + for (llvm::tie(I, E) = M->begin_end(R); I != E; ++I) + WorkList.push_back(RBDItem(state_N, *I, VisitedFromSuperRegion)); + } + // At this point, if we have already visited this region before, we are + // done. + if (VF != NotVisited) { + VF = N.getVisitFlag(); + continue; + } + VF = N.getVisitFlag(); + + // Enqueue the super region. + if (const SubRegion *SR = dyn_cast(R)) { + const MemRegion *superR = SR->getSuperRegion(); + if (!isa(superR)) { + // If 'R' is a field or an element, we want to keep the bindings + // for the other fields and elements around. The reason is that + // pointer arithmetic can get us to the other fields or elements. + VisitFlag NewVisit = + isa(R) || isa(R) || isa(R) + ? VisitedFromSuperRegion : VisitedFromSubRegion; + + WorkList.push_back(RBDItem(state_N, superR, NewVisit)); + } + } + + // Mark the symbol for any live SymbolicRegion as "live". This means we + // should continue to track that symbol. + if (const SymbolicRegion* SymR = dyn_cast(R)) + SymReaper.markLive(SymR->getSymbol()); + + Store store_N = state_N->getStore(); + RegionBindings B_N = GetRegionBindings(store_N); + + // Get the data binding for R (if any). + const SVal* Xptr = B_N.lookup(R); + + // Check for lazy bindings. + if (const nonloc::LazyCompoundVal *V = + dyn_cast_or_null(Xptr)) { + + const LazyCompoundValData *D = V->getCVData(); + WorkList.push_back(RBDItem(D->getState(), D->getRegion(), + VisitedFromSuperRegion)); + } + else { + // No direct binding? Get the default binding for R (if any). + if (!Xptr) { + RegionDefaultBindings DVM_N = &state == state_N ? DVM + : state_N->get(); + + Xptr = DVM_N.lookup(R); + } + + // Direct or default binding? + if (Xptr) { + SVal X = *Xptr; + + // Update the set of live symbols. + for (SVal::symbol_iterator SI=X.symbol_begin(), SE=X.symbol_end(); + SI!=SE;++SI) + SymReaper.markLive(*SI); + + // If X is a region, then add it to the worklist. + if (const MemRegion *RX = X.getAsRegion()) + WorkList.push_back(RBDItem(state_N, RX, VisitedFromSuperRegion)); + } + } + } + // We have now scanned the store, marking reachable regions and symbols // as live. We now remove all the regions that are dead from the store // as well as update DSymbols with the set symbols that are now dead. for (RegionBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) { const MemRegion* R = I.getKey(); // If this region live? Is so, none of its symbols are dead. - if (Marked.count(R)) + if (Visited.find(std::make_pair(&state, R)) != Visited.end()) continue; // Remove this dead region from the store. @@ -1800,7 +1806,7 @@ void RegionStoreManager::RemoveDeadBindings(GRState &state, Stmt* Loc, const MemRegion *R = I.getKey(); // If this region live? Is so, none of its symbols are dead. - if (Marked.count(R)) + if (Visited.find(std::make_pair(&state, R)) != Visited.end()) continue; // Remove this dead region. diff --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m index 03e3f6f2dcef..66f01fc7b312 100644 --- a/clang/test/Analysis/misc-ps-region-store.m +++ b/clang/test/Analysis/misc-ps-region-store.m @@ -264,5 +264,17 @@ int test_invalidate_field_test_positive() { return y.x; // expected-warning{{garbage}} } +// This test case illustrates how a typeless array of bytes casted to a +// struct should be treated as initialized. RemoveDeadBindings previously +// had a bug that caused 'x' to lose its default symbolic value after the +// assignment to 'p', thus causing 'p->z' to evaluate to "undefined". +struct ArrayWrapper { unsigned char y[16]; }; +struct WrappedStruct { unsigned z; }; +int test_handle_array_wrapper() { + struct ArrayWrapper x; + test_handle_array_wrapper(&x); + struct WrappedStruct *p = (struct WrappedStruct*) x.y; + return p->z; // no-warning +}