From: Ted Kremenek Date: Wed, 3 Feb 2010 04:16:00 +0000 (+0000) Subject: Fix regression in RegionStore due to recent changes in X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5b290658c5af4cc186fe556311db2bfbb316c00a;p=clang Fix regression in RegionStore due to recent changes in RegionStoreManager::InvalidateRegions() by adjusting the worklist to iterate over BindingKeys instead of MemRegions. We also only need to do the actual invalidation work on base regions, and for non-base regions just blow away their bindings. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@95200 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp index 88e4eb6211..d829199d35 100644 --- a/lib/Checker/RegionStore.cpp +++ b/lib/Checker/RegionStore.cpp @@ -506,9 +506,10 @@ void RegionStoreManager::RemoveSubRegionBindings(RegionBindings &B, namespace { class InvalidateRegionsWorker { - typedef BumpVector RegionCluster; + typedef BumpVector RegionCluster; typedef llvm::DenseMap ClusterMap; - typedef llvm::SmallVector WorkList; + typedef llvm::SmallVector, 10> + WorkList; BumpVectorContext BVC; ClusterMap ClusterM; @@ -523,25 +524,30 @@ public: StoreManager::InvalidatedSymbols *IS); private: + void AddToWorkList(BindingKey K); void AddToWorkList(const MemRegion *R); - void AddToCluster(const MemRegion *R); + void AddToCluster(BindingKey K); RegionCluster **getCluster(const MemRegion *R); }; } -void InvalidateRegionsWorker::AddToCluster(const MemRegion *R) { +void InvalidateRegionsWorker::AddToCluster(BindingKey K) { + const MemRegion *R = K.getRegion(); const MemRegion *baseR = R->getBaseRegion(); RegionCluster **CPtr = getCluster(baseR); - if (R != baseR) { - assert(*CPtr); - (*CPtr)->push_back(R, BVC); - } + assert(*CPtr); + (*CPtr)->push_back(K, BVC); +} + +void InvalidateRegionsWorker::AddToWorkList(BindingKey K) { + AddToWorkList(K.getRegion()); } void InvalidateRegionsWorker::AddToWorkList(const MemRegion *R) { - RegionCluster **CPtr = getCluster( R->getBaseRegion()); + const MemRegion *baseR = R->getBaseRegion(); + RegionCluster **CPtr = getCluster(baseR); if (RegionCluster *C = *CPtr) { - WL.push_back(C); + WL.push_back(std::make_pair(baseR, C)); *CPtr = NULL; } } @@ -552,7 +558,6 @@ InvalidateRegionsWorker::getCluster(const MemRegion *R) { if (!CRef) { void *Mem = BVC.getAllocator().Allocate(); CRef = new (Mem) RegionCluster(BVC, 10); - CRef->push_back(R, BVC); } return &CRef; } @@ -571,9 +576,12 @@ InvalidateRegionsWorker::InvalidateRegions(RegionStoreManager &RM, // Scan the entire store and make the region clusters. for (RegionBindings::iterator RI = B.begin(), RE = B.end(); RI != RE; ++RI) { - AddToCluster(RI.getKey().getRegion()); - if (const MemRegion *R = RI.getData().getAsRegion()) - AddToCluster(R); + AddToCluster(RI.getKey()); + if (const MemRegion *R = RI.getData().getAsRegion()) { + // Generate a cluster, but don't add the region to the cluster + // if there aren't any bindings. + getCluster(R->getBaseRegion()); + } } // Add the cluster for I .. E to a worklist. @@ -581,94 +589,92 @@ InvalidateRegionsWorker::InvalidateRegions(RegionStoreManager &RM, AddToWorkList(*I); while (!WL.empty()) { - RegionCluster *C = WL.back(); + const MemRegion *baseR; + RegionCluster *C; + llvm::tie(baseR, C) = WL.back(); WL.pop_back(); for (RegionCluster::iterator I = C->begin(), E = C->end(); I != E; ++I) { - const MemRegion *R = *I; + BindingKey K = *I; // Get the old binding. Is it a region? If so, add it to the worklist. - if (Optional V = RM.getDirectBinding(B, R)) { - if (const MemRegion *RV = V->getAsRegion()) - AddToWorkList(RV); + if (const SVal *V = RM.Lookup(B, K)) { + if (const MemRegion *R = V->getAsRegion()) + AddToWorkList(R); // A symbol? Mark it touched by the invalidation. if (IS) if (SymbolRef Sym = V->getAsSymbol()) IS->insert(Sym); } - - // Symbolic region? Mark that symbol touched by the invalidation. - if (IS) - if (const SymbolicRegion *SR = dyn_cast(R)) - IS->insert(SR->getSymbol()); - - // BlockDataRegion? If so, invalidate captured variables that are passed - // by reference. - if (const BlockDataRegion *BR = dyn_cast(R)) { - for (BlockDataRegion::referenced_vars_iterator - I = BR->referenced_vars_begin(), E = BR->referenced_vars_end() ; - I != E; ++I) { - const VarRegion *VR = *I; - if (VR->getDecl()->getAttr()) - AddToWorkList(VR); - } - continue; - } - - // Handle the region itself. - if (isa(R) || isa(R)) { - // Invalidate the region by setting its default value to - // conjured symbol. The type of the symbol is irrelavant. - DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(R, Ex, Ctx.IntTy, - Count); - B = RM.Add(B, R, BindingKey::Default, V); - continue; - } - if (!R->isBoundable()) - continue; - - const TypedRegion *TR = cast(R); - QualType T = TR->getValueType(Ctx); + B = RM.Remove(B, K); + } - // Invalidate the binding. - if (const RecordType *RT = T->getAsStructureType()) { - const RecordDecl *RD = RT->getDecl()->getDefinition(Ctx); - // No record definition. There is nothing we can do. - if (!RD) { - B = RM.Remove(B, R); - continue; - } - - // Invalidate the region by setting its default value to - // conjured symbol. The type of the symbol is irrelavant. - DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(R, Ex, Ctx.IntTy, - Count); - B = RM.Add(B, R, BindingKey::Default, V); - continue; - } - if (const ArrayType *AT = Ctx.getAsArrayType(T)) { - // Set the default value of the array to conjured symbol. - DefinedOrUnknownSVal V = - ValMgr.getConjuredSymbolVal(R, Ex, AT->getElementType(), Count); - B = RM.Add(B, R, BindingKey::Default, V); - continue; + // Now inspect the base region. + + if (IS) { + // Symbolic region? Mark that symbol touched by the invalidation. + if (const SymbolicRegion *SR = dyn_cast(baseR)) + IS->insert(SR->getSymbol()); + } + + // BlockDataRegion? If so, invalidate captured variables that are passed + // by reference. + if (const BlockDataRegion *BR = dyn_cast(baseR)) { + for (BlockDataRegion::referenced_vars_iterator + I = BR->referenced_vars_begin(), E = BR->referenced_vars_end() ; + I != E; ++I) { + const VarRegion *VR = *I; + if (VR->getDecl()->getAttr()) + AddToWorkList(VR); } + continue; + } + + if (isa(baseR) || isa(baseR)) { + // Invalidate the region by setting its default value to + // conjured symbol. The type of the symbol is irrelavant. + DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(baseR, Ex, Ctx.IntTy, + Count); + B = RM.Add(B, baseR, BindingKey::Default, V); + continue; + } + + if (!baseR->isBoundable()) + continue; - // For fields and elements that aren't themselves structs or arrays, - // just remove the binding. Base regions will get default values from - // which the fields and elements will get lazily symbolicated. - if (isa(R) || isa(R)) { - B = RM.Remove(B, R, BindingKey::Direct); + const TypedRegion *TR = cast(baseR); + QualType T = TR->getValueType(Ctx); + + // Invalidate the binding. + if (const RecordType *RT = T->getAsStructureType()) { + const RecordDecl *RD = RT->getDecl()->getDefinition(Ctx); + // No record definition. There is nothing we can do. + if (!RD) { + B = RM.Remove(B, baseR); continue; } - - - DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(R, Ex, T, Count); - assert(SymbolManager::canSymbolicate(T) || V.isUnknown()); - B = RM.Add(B, R, BindingKey::Direct, V); + + // Invalidate the region by setting its default value to + // conjured symbol. The type of the symbol is irrelavant. + DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(baseR, Ex, Ctx.IntTy, + Count); + B = RM.Add(B, baseR, BindingKey::Default, V); + continue; + } + + if (const ArrayType *AT = Ctx.getAsArrayType(T)) { + // Set the default value of the array to conjured symbol. + DefinedOrUnknownSVal V = + ValMgr.getConjuredSymbolVal(baseR, Ex, AT->getElementType(), Count); + B = RM.Add(B, baseR, BindingKey::Default, V); + continue; } + + DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(baseR, Ex, T, Count); + assert(SymbolManager::canSymbolicate(T) || V.isUnknown()); + B = RM.Add(B, baseR, BindingKey::Direct, V); } // Create a new state with the updated bindings. diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m index b31cd4cdd1..88c73bc22e 100644 --- a/test/Analysis/misc-ps.m +++ b/test/Analysis/misc-ps.m @@ -866,3 +866,16 @@ void bar_rev95119() { baz_rev95119((double)value); } +//===----------------------------------------------------------------------===// +// Handle loading a symbolic pointer from a symbolic region that was +// invalidated by a call to an unknown function. +//===----------------------------------------------------------------------===// + +void bar_rev95192(int **x); +void foo_rev95192(int **x) { + *x = 0; + bar_rev95192(x); + // Not a null dereference. + **x = 1; // no-warning +} +