r175228 - [analyzer] Refactor RegionStore's sub-region bindings traversal.
Jordan Rose
jordan_rose at apple.com
Thu Feb 14 16:32:04 PST 2013
Author: jrose
Date: Thu Feb 14 18:32:03 2013
New Revision: 175228
URL: http://llvm.org/viewvc/llvm-project?rev=175228&view=rev
Log:
[analyzer] Refactor RegionStore's sub-region bindings traversal.
This is going to be used in the next commit.
While I'm here, tighten up assumptions about symbolic offset
BindingKeys, and make offset calculation explicitly handle all
MemRegion kinds.
No functionality change.
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=175228&r1=175227&r2=175228&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Thu Feb 14 18:32:03 2013
@@ -1080,16 +1080,37 @@ RegionOffset MemRegion::getAsOffset() co
while (1) {
switch (R->getKind()) {
- default:
- return RegionOffset(R, RegionOffset::Symbolic);
+ case GenericMemSpaceRegionKind:
+ case StackLocalsSpaceRegionKind:
+ case StackArgumentsSpaceRegionKind:
+ case HeapSpaceRegionKind:
+ case UnknownSpaceRegionKind:
+ case StaticGlobalSpaceRegionKind:
+ case GlobalInternalSpaceRegionKind:
+ case GlobalSystemSpaceRegionKind:
+ case GlobalImmutableSpaceRegionKind:
+ // Stores can bind directly to a region space to set a default value.
+ assert(Offset == 0 && !SymbolicOffsetBase);
+ goto Finish;
+
+ case FunctionTextRegionKind:
+ case BlockTextRegionKind:
+ case BlockDataRegionKind:
+ // These will never have bindings, but may end up having values requested
+ // if the user does some strange casting.
+ if (Offset != 0)
+ SymbolicOffsetBase = R;
+ goto Finish;
case SymbolicRegionKind:
case AllocaRegionKind:
case CompoundLiteralRegionKind:
case CXXThisRegionKind:
case StringRegionKind:
+ case ObjCStringRegionKind:
case VarRegionKind:
case CXXTempObjectRegionKind:
+ // Usual base regions.
goto Finish;
case ObjCIvarRegionKind:
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=175228&r1=175227&r2=175228&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Feb 14 18:32:03 2013
@@ -46,11 +46,15 @@ private:
llvm::PointerIntPair<const MemRegion *, 2> P;
uint64_t Data;
- explicit BindingKey(const MemRegion *r, const MemRegion *Base, Kind k)
+ /// Create a key for a binding to region \p r, which has a symbolic offset
+ /// from region \p Base.
+ explicit BindingKey(const SubRegion *r, const SubRegion *Base, Kind k)
: P(r, k | Symbolic), Data(reinterpret_cast<uintptr_t>(Base)) {
assert(r && Base && "Must have known regions.");
assert(getConcreteOffsetRegion() == Base && "Failed to store base region");
}
+
+ /// Create a key for a binding at \p offset from base region \p r.
explicit BindingKey(const MemRegion *r, uint64_t offset, Kind k)
: P(r, k), Data(offset) {
assert(r && "Must have known regions.");
@@ -68,9 +72,9 @@ public:
return Data;
}
- const MemRegion *getConcreteOffsetRegion() const {
+ const SubRegion *getConcreteOffsetRegion() const {
assert(hasSymbolicOffset());
- return reinterpret_cast<const MemRegion *>(static_cast<uintptr_t>(Data));
+ return reinterpret_cast<const SubRegion *>(static_cast<uintptr_t>(Data));
}
const MemRegion *getBaseRegion() const {
@@ -106,7 +110,7 @@ public:
BindingKey BindingKey::Make(const MemRegion *R, Kind k) {
const RegionOffset &RO = R->getAsOffset();
if (RO.hasSymbolicOffset())
- return BindingKey(R, RO.getRegion(), k);
+ return BindingKey(cast<SubRegion>(R), cast<SubRegion>(RO.getRegion()), k);
return BindingKey(RO.getRegion(), RO.getOffset(), k);
}
@@ -709,61 +713,45 @@ static bool isCompatibleWithFields(Bindi
Fields.begin() - Delta);
}
-RegionBindingsRef
-RegionStoreManager::removeSubRegionBindings(RegionBindingsConstRef B,
- const SubRegion *R) {
- BindingKey SRKey = BindingKey::Make(R, BindingKey::Default);
- const MemRegion *ClusterHead = SRKey.getBaseRegion();
- if (R == ClusterHead) {
- // We can remove an entire cluster's bindings all in one go.
- return B.remove(R);
- }
-
+static void collectSubRegionKeys(SmallVectorImpl<BindingKey> &Keys,
+ SValBuilder &SVB,
+ const ClusterBindings &Cluster,
+ const SubRegion *Top, BindingKey TopKey) {
FieldVector FieldsInSymbolicSubregions;
- bool HasSymbolicOffset = SRKey.hasSymbolicOffset();
- if (HasSymbolicOffset) {
- getSymbolicOffsetFields(SRKey, FieldsInSymbolicSubregions);
- R = cast<SubRegion>(SRKey.getConcreteOffsetRegion());
- SRKey = BindingKey::Make(R, BindingKey::Default);
+ if (TopKey.hasSymbolicOffset()) {
+ getSymbolicOffsetFields(TopKey, FieldsInSymbolicSubregions);
+ Top = cast<SubRegion>(TopKey.getConcreteOffsetRegion());
+ TopKey = BindingKey::Make(Top, BindingKey::Default);
}
// This assumes the region being invalidated is char-aligned. This isn't
// true for bitfields, but since bitfields have no subregions they shouldn't
// be using this function anyway.
uint64_t Length = UINT64_MAX;
-
- SVal Extent = R->getExtent(svalBuilder);
+ SVal Extent = Top->getExtent(SVB);
if (nonloc::ConcreteInt *ExtentCI = dyn_cast<nonloc::ConcreteInt>(&Extent)) {
const llvm::APSInt &ExtentInt = ExtentCI->getValue();
assert(ExtentInt.isNonNegative() || ExtentInt.isUnsigned());
// Extents are in bytes but region offsets are in bits. Be careful!
- Length = ExtentInt.getLimitedValue() * Ctx.getCharWidth();
+ Length = ExtentInt.getLimitedValue() * SVB.getContext().getCharWidth();
}
- const ClusterBindings *Cluster = B.lookup(ClusterHead);
- if (!Cluster)
- return B;
-
- ClusterBindingsRef Result(*Cluster, CBFactory);
-
- // It is safe to iterate over the bindings as they are being changed
- // because they are in an ImmutableMap.
- for (ClusterBindings::iterator I = Cluster->begin(), E = Cluster->end();
+ for (ClusterBindings::iterator I = Cluster.begin(), E = Cluster.end();
I != E; ++I) {
BindingKey NextKey = I.getKey();
- if (NextKey.getRegion() == SRKey.getRegion()) {
+ if (NextKey.getRegion() == TopKey.getRegion()) {
// FIXME: This doesn't catch the case where we're really invalidating a
// region with a symbolic offset. Example:
// R: points[i].y
// Next: points[0].x
- if (NextKey.getOffset() > SRKey.getOffset() &&
- NextKey.getOffset() - SRKey.getOffset() < Length) {
+ if (NextKey.getOffset() > TopKey.getOffset() &&
+ NextKey.getOffset() - TopKey.getOffset() < Length) {
// Case 1: The next binding is inside the region we're invalidating.
// Remove it.
- Result = Result.remove(NextKey);
+ Keys.push_back(NextKey);
- } else if (NextKey.getOffset() == SRKey.getOffset()) {
+ } else if (NextKey.getOffset() == TopKey.getOffset()) {
// Case 2: The next binding is at the same offset as the region we're
// invalidating. In this case, we need to leave default bindings alone,
// since they may be providing a default value for a regions beyond what
@@ -771,33 +759,60 @@ RegionStoreManager::removeSubRegionBindi
// FIXME: This is probably incorrect; consider invalidating an outer
// struct whose first field is bound to a LazyCompoundVal.
if (NextKey.isDirect())
- Result = Result.remove(NextKey);
+ Keys.push_back(NextKey);
}
-
+
} else if (NextKey.hasSymbolicOffset()) {
const MemRegion *Base = NextKey.getConcreteOffsetRegion();
- if (R->isSubRegionOf(Base)) {
+ if (Top->isSubRegionOf(Base)) {
// Case 3: The next key is symbolic and we just changed something within
// its concrete region. We don't know if the binding is still valid, so
// we'll be conservative and remove it.
if (NextKey.isDirect())
if (isCompatibleWithFields(NextKey, FieldsInSymbolicSubregions))
- Result = Result.remove(NextKey);
+ Keys.push_back(NextKey);
} else if (const SubRegion *BaseSR = dyn_cast<SubRegion>(Base)) {
// Case 4: The next key is symbolic, but we changed a known
// super-region. In this case the binding is certainly no longer valid.
- if (R == Base || BaseSR->isSubRegionOf(R))
+ if (Top == Base || BaseSR->isSubRegionOf(Top))
if (isCompatibleWithFields(NextKey, FieldsInSymbolicSubregions))
- Result = Result.remove(NextKey);
+ Keys.push_back(NextKey);
}
}
}
+}
+
+RegionBindingsRef
+RegionStoreManager::removeSubRegionBindings(RegionBindingsConstRef B,
+ const SubRegion *Top) {
+ BindingKey TopKey = BindingKey::Make(Top, BindingKey::Default);
+ const MemRegion *ClusterHead = TopKey.getBaseRegion();
+ if (Top == ClusterHead) {
+ // We can remove an entire cluster's bindings all in one go.
+ return B.remove(Top);
+ }
+
+ const ClusterBindings *Cluster = B.lookup(ClusterHead);
+ if (!Cluster)
+ return B;
+
+ SmallVector<BindingKey, 32> Keys;
+ collectSubRegionKeys(Keys, svalBuilder, *Cluster, Top, TopKey);
+
+ ClusterBindingsRef Result(*Cluster, CBFactory);
+ for (SmallVectorImpl<BindingKey>::const_iterator I = Keys.begin(),
+ E = Keys.end();
+ I != E; ++I)
+ Result = Result.remove(*I);
// If we're invalidating a region with a symbolic offset, we need to make sure
// we don't treat the base region as uninitialized anymore.
// FIXME: This isn't very precise; see the example in the loop.
- if (HasSymbolicOffset)
- Result = Result.add(SRKey, UnknownVal());
+ if (TopKey.hasSymbolicOffset()) {
+ const SubRegion *Concrete = TopKey.getConcreteOffsetRegion();
+ Result = Result.add(BindingKey::Make(Concrete, BindingKey::Default),
+ UnknownVal());
+ }
if (Result.isEmpty())
return B.remove(ClusterHead);
More information about the cfe-commits
mailing list