Fix:
<rdar://problem/6914474> checker doesn't realize that variable might
have been assigned if a pointer to that variable was passed to another
function via a structure
The problem here was the RegionStoreManager::InvalidateRegion didn't
invalidate the bindings of invalidated regions. This required a
rewrite of this method using a worklist.
As part of this fix, changed ValueManager::getConjuredSymbolVal() to
require a 'void*' SymbolTag argument. This tag is used to
differentiate two different symbols created at the same location.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@82920 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Analysis/BasicStore.cpp b/lib/Analysis/BasicStore.cpp
index 017399f..e2a19cf 100644
--- a/lib/Analysis/BasicStore.cpp
+++ b/lib/Analysis/BasicStore.cpp
@@ -639,7 +639,7 @@
return state;
QualType T = cast<TypedRegion>(R)->getValueType(R->getContext());
- SVal V = ValMgr.getConjuredSymbolVal(E, T, Count);
+ SVal V = ValMgr.getConjuredSymbolVal(R, E, T, Count);
return Bind(state, loc::MemRegionVal(R), V);
}
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 970646f..75bd25c 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -2906,7 +2906,7 @@
if (Loc::IsLocType(T) || (T->isIntegerType() && T->isScalarType())) {
unsigned Count = Builder.getCurrentBlockCount();
ValueManager &ValMgr = Eng.getValueManager();
- SVal X = ValMgr.getConjuredSymbolVal(Ex, T, Count);
+ SVal X = ValMgr.getConjuredSymbolVal(NULL, Ex, T, Count);
state = state->BindExpr(Ex, X, false);
}
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 735949c..dc39d8b 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -2203,7 +2203,7 @@
// UnknownVal.
if (InitVal.isUnknown() ||
!getConstraintManager().canReasonAbout(InitVal)) {
- InitVal = ValMgr.getConjuredSymbolVal(InitEx, Count);
+ InitVal = ValMgr.getConjuredSymbolVal(NULL, InitEx, Count);
}
state = state->bindDecl(VD, LC, InitVal);
@@ -2608,7 +2608,8 @@
// Conjure a new symbol if necessary to recover precision.
if (Result.isUnknown() || !getConstraintManager().canReasonAbout(Result)){
DefinedOrUnknownSVal SymVal =
- ValMgr.getConjuredSymbolVal(Ex, Builder->getCurrentBlockCount());
+ ValMgr.getConjuredSymbolVal(NULL, Ex,
+ Builder->getCurrentBlockCount());
Result = SymVal;
// If the value is a location, ++/-- should always preserve
@@ -2812,7 +2813,7 @@
&& (Loc::IsLocType(T) ||
(T->isScalarType() && T->isIntegerType()))) {
unsigned Count = Builder->getCurrentBlockCount();
- RightV = ValMgr.getConjuredSymbolVal(B->getRHS(), Count);
+ RightV = ValMgr.getConjuredSymbolVal(NULL, B->getRHS(), Count);
}
// Simulate the effects of a "store": bind the value of the RHS
@@ -2936,7 +2937,7 @@
// The symbolic value is actually for the type of the left-hand side
// expression, not the computation type, as this is the value the
// LValue on the LHS will bind to.
- LHSVal = ValMgr.getConjuredSymbolVal(B->getRHS(), LTy, Count);
+ LHSVal = ValMgr.getConjuredSymbolVal(NULL, B->getRHS(), LTy, Count);
// However, we need to convert the symbol to the computation type.
llvm::tie(state, Result) = SVator.EvalCast(LHSVal, state, CTy, LTy);
diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index 75907da..6b7799c 100644
--- a/lib/Analysis/RegionStore.cpp
+++ b/lib/Analysis/RegionStore.cpp
@@ -436,69 +436,98 @@
DVM = DVMFactory.Remove(DVM, R);
}
-
const GRState *RegionStoreManager::InvalidateRegion(const GRState *state,
const MemRegion *R,
- const Expr *E,
+ const Expr *Ex,
unsigned Count) {
ASTContext& Ctx = StateMgr.getContext();
// Strip away casts.
R = R->getBaseRegion();
- // Remove the bindings to subregions.
- {
- // Get the mapping of regions -> subregions.
- llvm::OwningPtr<RegionStoreSubRegionMap>
- SubRegions(getRegionStoreSubRegionMap(state));
+ // Get the mapping of regions -> subregions.
+ llvm::OwningPtr<RegionStoreSubRegionMap>
+ SubRegions(getRegionStoreSubRegionMap(state));
+
+ RegionBindings B = GetRegionBindings(state->getStore());
+ RegionDefaultBindings DVM = state->get<RegionDefaultValue>();
+ RegionDefaultBindings::Factory &DVMFactory =
+ state->get_context<RegionDefaultValue>();
+
+ llvm::DenseMap<const MemRegion *, unsigned> Visited;
+ llvm::SmallVector<const MemRegion *, 10> WorkList;
+ WorkList.push_back(R);
+
+ while (!WorkList.empty()) {
+ R = WorkList.back();
+ WorkList.pop_back();
+
+ // Have we visited this region before?
+ unsigned &visited = Visited[R];
+ if (visited)
+ continue;
+ visited = 1;
- RegionBindings B = GetRegionBindings(state->getStore());
- RegionDefaultBindings DVM = state->get<RegionDefaultValue>();
- RegionDefaultBindings::Factory &DVMFactory =
- state->get_context<RegionDefaultValue>();
+ // Add subregions to work list.
+ RegionStoreSubRegionMap::iterator I, E;
+ for (llvm::tie(I, E) = SubRegions->begin_end(R); I!=E; ++I)
+ WorkList.push_back(*I);
+
+ // Handle region.
+ if (isa<AllocaRegion>(R) || isa<SymbolicRegion>(R) ||
+ isa<ObjCObjectRegion>(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);
+ DVM = DVMFactory.Add(DVM, R, V);
+ continue;
+ }
- RemoveSubRegionBindings(B, DVM, DVMFactory, R, *SubRegions.get());
- state = state->makeWithStore(B.getRoot())->set<RegionDefaultValue>(DVM);
+ if (!R->isBoundable())
+ continue;
+
+ const TypedRegion *TR = cast<TypedRegion>(R);
+ QualType T = TR->getValueType(Ctx);
+
+ if (const RecordType *RT = T->getAsStructureType()) {
+ // FIXME: handle structs with default region value.
+ const RecordDecl *RD = RT->getDecl()->getDefinition(Ctx);
+
+ // No record definition. There is nothing we can do.
+ if (!RD)
+ 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);
+ DVM = DVMFactory.Add(DVM, R, 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);
+ DVM = DVMFactory.Add(DVM, R, V);
+ continue;
+ }
+
+ // Get the old binding. Is it a region? If so, add it to the worklist.
+ if (const SVal *OldV = B.lookup(R)) {
+ if (const MemRegion *RV = OldV->getAsRegion())
+ WorkList.push_back(RV);
+ }
+
+ // Invalidate the binding.
+ DefinedOrUnknownSVal V = ValMgr.getConjuredSymbolVal(R, Ex, T, Count);
+ assert(SymbolManager::canSymbolicate(T) || V.isUnknown());
+ B = RBFactory.Add(B, R, V);
}
- if (!R->isBoundable())
- return state;
-
- if (isa<AllocaRegion>(R) || isa<SymbolicRegion>(R) ||
- isa<ObjCObjectRegion>(R)) {
- // Invalidate the region by setting its default value to
- // conjured symbol. The type of the symbol is irrelavant.
- SVal V = ValMgr.getConjuredSymbolVal(E, Ctx.IntTy, Count);
- return setDefaultValue(state, R, V);
- }
-
- const TypedRegion *TR = cast<TypedRegion>(R);
- QualType T = TR->getValueType(Ctx);
-
- if (const RecordType *RT = T->getAsStructureType()) {
- // FIXME: handle structs with default region value.
- const RecordDecl *RD = RT->getDecl()->getDefinition(Ctx);
-
- // No record definition. There is nothing we can do.
- if (!RD)
- return state;
-
- // Invalidate the region by setting its default value to
- // conjured symbol. The type of the symbol is irrelavant.
- SVal V = ValMgr.getConjuredSymbolVal(E, Ctx.IntTy, Count);
- return setDefaultValue(state, R, V);
- }
-
- if (const ArrayType *AT = Ctx.getAsArrayType(T)) {
- // Set the default value of the array to conjured symbol.
- SVal V = ValMgr.getConjuredSymbolVal(E, AT->getElementType(),
- Count);
- return setDefaultValue(state, TR, V);
- }
-
- SVal V = ValMgr.getConjuredSymbolVal(E, T, Count);
- assert(SymbolManager::canSymbolicate(T) || V.isUnknown());
- return Bind(state, ValMgr.makeLoc(TR), V);
+ // Create a new state with the updated bindings.
+ return state->makeWithStore(B.getRoot())->set<RegionDefaultValue>(DVM);
}
//===----------------------------------------------------------------------===//
diff --git a/lib/Analysis/ValueManager.cpp b/lib/Analysis/ValueManager.cpp
index 9c3dbdd..fe670e7 100644
--- a/lib/Analysis/ValueManager.cpp
+++ b/lib/Analysis/ValueManager.cpp
@@ -88,13 +88,15 @@
return nonloc::SymbolVal(sym);
}
-DefinedOrUnknownSVal ValueManager::getConjuredSymbolVal(const Expr *E, unsigned Count) {
+DefinedOrUnknownSVal ValueManager::getConjuredSymbolVal(const void *SymbolTag,
+ const Expr *E,
+ unsigned Count) {
QualType T = E->getType();
if (!SymbolManager::canSymbolicate(T))
return UnknownVal();
- SymbolRef sym = SymMgr.getConjuredSymbol(E, Count);
+ SymbolRef sym = SymMgr.getConjuredSymbol(E, Count, SymbolTag);
if (Loc::IsLocType(T))
return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
@@ -102,14 +104,15 @@
return nonloc::SymbolVal(sym);
}
-DefinedOrUnknownSVal ValueManager::getConjuredSymbolVal(const Expr *E,
+DefinedOrUnknownSVal ValueManager::getConjuredSymbolVal(const void *SymbolTag,
+ const Expr *E,
QualType T,
unsigned Count) {
if (!SymbolManager::canSymbolicate(T))
return UnknownVal();
- SymbolRef sym = SymMgr.getConjuredSymbol(E, T, Count);
+ SymbolRef sym = SymMgr.getConjuredSymbol(E, T, Count, SymbolTag);
if (Loc::IsLocType(T))
return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));