[cfe-commits] r82920 - in /cfe/trunk: include/clang/Analysis/PathSensitive/ValueManager.h lib/Analysis/BasicStore.cpp lib/Analysis/CFRefCount.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/RegionStore.cpp lib/Analysis/ValueManager.cpp test/Analysis/misc-ps-region-store.m
Ted Kremenek
kremenek at apple.com
Sun Sep 27 13:45:22 PDT 2009
Author: kremenek
Date: Sun Sep 27 15:45:21 2009
New Revision: 82920
URL: http://llvm.org/viewvc/llvm-project?rev=82920&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h
cfe/trunk/lib/Analysis/BasicStore.cpp
cfe/trunk/lib/Analysis/CFRefCount.cpp
cfe/trunk/lib/Analysis/GRExprEngine.cpp
cfe/trunk/lib/Analysis/RegionStore.cpp
cfe/trunk/lib/Analysis/ValueManager.cpp
cfe/trunk/test/Analysis/misc-ps-region-store.m
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h Sun Sep 27 15:45:21 2009
@@ -104,8 +104,10 @@
return UnknownVal();
}
- DefinedOrUnknownSVal getConjuredSymbolVal(const Expr *E, unsigned Count);
- DefinedOrUnknownSVal getConjuredSymbolVal(const Expr *E, QualType T,
+ DefinedOrUnknownSVal getConjuredSymbolVal(const void *SymbolTag,
+ const Expr *E, unsigned Count);
+ DefinedOrUnknownSVal getConjuredSymbolVal(const void *SymbolTag,
+ const Expr *E, QualType T,
unsigned Count);
DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(SymbolRef parentSymbol,
Modified: cfe/trunk/lib/Analysis/BasicStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BasicStore.cpp?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BasicStore.cpp (original)
+++ cfe/trunk/lib/Analysis/BasicStore.cpp Sun Sep 27 15:45:21 2009
@@ -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);
}
Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Sun Sep 27 15:45:21 2009
@@ -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);
}
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Sun Sep 27 15:45:21 2009
@@ -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);
Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
+++ cfe/trunk/lib/Analysis/RegionStore.cpp Sun Sep 27 15:45:21 2009
@@ -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));
-
- RegionBindings B = GetRegionBindings(state->getStore());
- RegionDefaultBindings DVM = state->get<RegionDefaultValue>();
- RegionDefaultBindings::Factory &DVMFactory =
- state->get_context<RegionDefaultValue>();
-
- RemoveSubRegionBindings(B, DVM, DVMFactory, R, *SubRegions.get());
- state = state->makeWithStore(B.getRoot())->set<RegionDefaultValue>(DVM);
- }
-
- 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;
+ // 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;
- // 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);
- }
+ // 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;
+ }
- 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);
+ 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);
}
- 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);
}
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/Analysis/ValueManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ValueManager.cpp?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ValueManager.cpp (original)
+++ cfe/trunk/lib/Analysis/ValueManager.cpp Sun Sep 27 15:45:21 2009
@@ -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));
Modified: cfe/trunk/test/Analysis/misc-ps-region-store.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.m?rev=82920&r1=82919&r2=82920&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-region-store.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-region-store.m Sun Sep 27 15:45:21 2009
@@ -237,3 +237,15 @@
x += *b++; // no-warning
}
+// <rdar://problem/6914474> - Check that 'x' is invalidated because its
+// address is passed in as a value to a struct.
+struct doodad_6914474 { int *v; };
+extern void prod_6914474(struct doodad_6914474 *d);
+int rdar_6914474(void) {
+ int x;
+ struct doodad_6914474 d;
+ d.v = &x;
+ prod_6914474(&d);
+ return x; // no-warning
+}
+
More information about the cfe-commits
mailing list