r175703 - [analyzer] Tighten up safety in the use of lazy bindings.
Jordan Rose
jordan_rose at apple.com
Wed Feb 20 17:34:51 PST 2013
Author: jrose
Date: Wed Feb 20 19:34:51 2013
New Revision: 175703
URL: http://llvm.org/viewvc/llvm-project?rev=175703&view=rev
Log:
[analyzer] Tighten up safety in the use of lazy bindings.
- When deciding if we can reuse a lazy binding, make sure to check if there
are additional bindings in the sub-region.
- When reading from a lazy binding, don't accidentally strip off casts or
base object regions. This slows down lazy binding reading a bit but is
necessary for type sanity when treating one class as another.
A bit of minor refactoring allowed these two checks to be unified in a nice
early-return-using helper function.
<rdar://problem/13239840>
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/test/Analysis/array-struct-region.c
cfe/trunk/test/Analysis/derived-to-base.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=175703&r1=175702&r2=175703&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Feb 20 19:34:51 2013
@@ -477,7 +477,7 @@ public: // Part of public interface to c
QualType Ty,
const MemRegion *superR);
- SVal getLazyBinding(const MemRegion *LazyBindingRegion,
+ SVal getLazyBinding(const SubRegion *LazyBindingRegion,
RegionBindingsRef LazyBinding);
/// Get bindings for the values in a struct and return a CompoundVal, used
@@ -500,9 +500,9 @@ public: // Part of public interface to c
///
/// If there is no lazy binding for \p R, the returned value will have a null
/// \c second. Note that a null pointer can represents a valid Store.
- std::pair<Store, const MemRegion *>
- getLazyBinding(RegionBindingsConstRef B, const MemRegion *R,
- const MemRegion *originalRegion);
+ std::pair<Store, const SubRegion *>
+ findLazyBinding(RegionBindingsConstRef B, const SubRegion *R,
+ const SubRegion *originalRegion);
/// Returns the cached set of interesting SVals contained within a lazy
/// binding.
@@ -1255,72 +1255,88 @@ SVal RegionStoreManager::getBinding(Regi
return svalBuilder.getRegionValueSymbolVal(R);
}
-std::pair<Store, const MemRegion *>
-RegionStoreManager::getLazyBinding(RegionBindingsConstRef B,
- const MemRegion *R,
- const MemRegion *originalRegion) {
- typedef std::pair<Store, const MemRegion *> StoreRegionPair;
+/// Checks to see if store \p B has a lazy binding for region \p R.
+///
+/// If \p AllowSubregionBindings is \c false, a lazy binding will be rejected
+/// if there are additional bindings within \p R.
+///
+/// Note that unlike RegionStoreManager::findLazyBinding, this will not search
+/// for lazy bindings for super-regions of \p R.
+static Optional<nonloc::LazyCompoundVal>
+getExistingLazyBinding(SValBuilder &SVB, RegionBindingsConstRef B,
+ const SubRegion *R, bool AllowSubregionBindings) {
+ Optional<SVal> V = B.getDefaultBinding(R);
+ if (!V)
+ return Optional<nonloc::LazyCompoundVal>();
+
+ Optional<nonloc::LazyCompoundVal> LCV = V->getAs<nonloc::LazyCompoundVal>();
+ if (!LCV)
+ return Optional<nonloc::LazyCompoundVal>();
+
+ // If the LCV is for a subregion, the types won't match, and we shouldn't
+ // reuse the binding. Unfortuately we can only check this if the destination
+ // region is a TypedValueRegion.
+ if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(R)) {
+ QualType RegionTy = TVR->getValueType();
+ QualType SourceRegionTy = LCV->getRegion()->getValueType();
+ if (!SVB.getContext().hasSameUnqualifiedType(RegionTy, SourceRegionTy))
+ return Optional<nonloc::LazyCompoundVal>();
+ }
+
+ if (!AllowSubregionBindings) {
+ // If there are any other bindings within this region, we shouldn't reuse
+ // the top-level binding.
+ SmallVector<BindingKey, 16> Keys;
+ collectSubRegionKeys(Keys, SVB, *B.lookup(R->getBaseRegion()), R,
+ /*IncludeAllDefaultBindings=*/true);
+ if (Keys.size() > 1)
+ return Optional<nonloc::LazyCompoundVal>();
+ }
+
+ return *LCV;
+}
+
+std::pair<Store, const SubRegion *>
+RegionStoreManager::findLazyBinding(RegionBindingsConstRef B,
+ const SubRegion *R,
+ const SubRegion *originalRegion) {
if (originalRegion != R) {
- if (Optional<SVal> OV = B.getDefaultBinding(R)) {
- if (Optional<nonloc::LazyCompoundVal> V =
- OV->getAs<nonloc::LazyCompoundVal>())
- return std::make_pair(V->getStore(), V->getRegion());
- }
+ if (Optional<nonloc::LazyCompoundVal> V =
+ getExistingLazyBinding(svalBuilder, B, R, true))
+ return std::make_pair(V->getStore(), V->getRegion());
}
-
+
+ typedef std::pair<Store, const SubRegion *> StoreRegionPair;
+ StoreRegionPair Result = StoreRegionPair();
+
if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
- StoreRegionPair X = getLazyBinding(B, ER->getSuperRegion(), originalRegion);
+ Result = findLazyBinding(B, cast<SubRegion>(ER->getSuperRegion()),
+ originalRegion);
+
+ if (Result.second)
+ Result.second = MRMgr.getElementRegionWithSuper(ER, Result.second);
+
+ } else if (const FieldRegion *FR = dyn_cast<FieldRegion>(R)) {
+ Result = findLazyBinding(B, cast<SubRegion>(FR->getSuperRegion()),
+ originalRegion);
+
+ if (Result.second)
+ Result.second = MRMgr.getFieldRegionWithSuper(FR, Result.second);
- if (X.second)
- return std::make_pair(X.first,
- MRMgr.getElementRegionWithSuper(ER, X.second));
- }
- else if (const FieldRegion *FR = dyn_cast<FieldRegion>(R)) {
- StoreRegionPair X = getLazyBinding(B, FR->getSuperRegion(), originalRegion);
-
- if (X.second) {
- return std::make_pair(X.first,
- MRMgr.getFieldRegionWithSuper(FR, X.second));
- }
-
} else if (const CXXBaseObjectRegion *BaseReg =
dyn_cast<CXXBaseObjectRegion>(R)) {
// C++ base object region is another kind of region that we should blast
// through to look for lazy compound value. It is like a field region.
- StoreRegionPair Result = getLazyBinding(B, BaseReg->getSuperRegion(),
- originalRegion);
+ Result = findLazyBinding(B, cast<SubRegion>(BaseReg->getSuperRegion()),
+ originalRegion);
- if (Result.second) {
- // Make sure the types match up.
- const CXXRecordDecl *LazyD = 0;
- if (const TypedValueRegion *LazyR =
- dyn_cast<TypedValueRegion>(Result.second)) {
- LazyD = LazyR->getValueType()->getAsCXXRecordDecl();
- if (LazyD)
- LazyD = LazyD->getCanonicalDecl();
- }
-
- typedef SmallVector<const CXXBaseObjectRegion *, 4> BaseListTy;
- BaseListTy BaseRegions;
- do {
- BaseRegions.push_back(BaseReg);
- BaseReg = dyn_cast<CXXBaseObjectRegion>(BaseReg->getSuperRegion());
- } while (BaseReg && LazyD != BaseReg->getDecl()->getCanonicalDecl());
-
- // Layer the base regions on the result in least-to-most derived order.
- for (BaseListTy::const_reverse_iterator I = BaseRegions.rbegin(),
- E = BaseRegions.rend();
- I != E; ++I) {
- Result.second = MRMgr.getCXXBaseObjectRegionWithSuper(*I,
- Result.second);
- }
- }
-
- return Result;
+ if (Result.second)
+ Result.second = MRMgr.getCXXBaseObjectRegionWithSuper(BaseReg,
+ Result.second);
}
- return StoreRegionPair();
+ return Result;
}
SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
@@ -1440,7 +1456,7 @@ RegionStoreManager::getBindingForDerived
return Optional<SVal>();
}
-SVal RegionStoreManager::getLazyBinding(const MemRegion *LazyBindingRegion,
+SVal RegionStoreManager::getLazyBinding(const SubRegion *LazyBindingRegion,
RegionBindingsRef LazyBinding) {
SVal Result;
if (const ElementRegion *ER = dyn_cast<ElementRegion>(LazyBindingRegion))
@@ -1480,8 +1496,8 @@ RegionStoreManager::getBindingForFieldOr
// Lazy binding?
Store lazyBindingStore = NULL;
- const MemRegion *lazyBindingRegion = NULL;
- llvm::tie(lazyBindingStore, lazyBindingRegion) = getLazyBinding(B, R, R);
+ const SubRegion *lazyBindingRegion = NULL;
+ llvm::tie(lazyBindingStore, lazyBindingRegion) = findLazyBinding(B, R, R);
if (lazyBindingRegion)
return getLazyBinding(lazyBindingRegion,
getRegionBindings(lazyBindingStore));
@@ -1665,17 +1681,9 @@ RegionStoreManager::getInterestingValues
NonLoc RegionStoreManager::createLazyBinding(RegionBindingsConstRef B,
const TypedValueRegion *R) {
- // If we already have a lazy binding, and it's for the whole structure,
- // don't create a new lazy binding.
- if (Optional<SVal> V = B.getDefaultBinding(R)) {
- if (Optional<nonloc::LazyCompoundVal> LCV =
- V->getAs<nonloc::LazyCompoundVal>()) {
- QualType RegionTy = R->getValueType();
- QualType SourceRegionTy = LCV->getRegion()->getValueType();
- if (Ctx.hasSameUnqualifiedType(RegionTy, SourceRegionTy))
- return *LCV;
- }
- }
+ if (Optional<nonloc::LazyCompoundVal> V =
+ getExistingLazyBinding(svalBuilder, B, R, false))
+ return *V;
return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R);
}
Modified: cfe/trunk/test/Analysis/array-struct-region.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.c?rev=175703&r1=175702&r2=175703&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/array-struct-region.c (original)
+++ cfe/trunk/test/Analysis/array-struct-region.c Wed Feb 20 19:34:51 2013
@@ -285,6 +285,13 @@ void testArrayStructCopy() {
clang_analyzer_eval(s3.data[0] == 'a'); // expected-warning{{TRUE}}
clang_analyzer_eval(s3.data[1] == 'b'); // expected-warning{{TRUE}}
clang_analyzer_eval(s3.data[2] == 'c'); // expected-warning{{TRUE}}
+
+ s3.data[0] = 'z';
+ ShortString s4 = s3;
+
+ clang_analyzer_eval(s4.data[0] == 'z'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(s4.data[1] == 'b'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(s4.data[2] == 'c'); // expected-warning{{TRUE}}
}
void testArrayStructCopyNested() {
Modified: cfe/trunk/test/Analysis/derived-to-base.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/derived-to-base.cpp?rev=175703&r1=175702&r2=175703&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/derived-to-base.cpp (original)
+++ cfe/trunk/test/Analysis/derived-to-base.cpp Wed Feb 20 19:34:51 2013
@@ -303,4 +303,33 @@ namespace LazyBindings {
}
#endif
}
+
+#if CONSTRUCTORS
+ namespace Nested {
+ struct NonTrivialCopy {
+ int padding;
+ NonTrivialCopy() {}
+ NonTrivialCopy(const NonTrivialCopy &) {}
+ };
+
+ struct FullyDerived : private NonTrivialCopy, public Derived {
+ int z;
+ };
+
+ struct Wrapper {
+ FullyDerived d;
+ int zz;
+
+ Wrapper(const FullyDerived &d) : d(d), zz(0) {}
+ };
+
+ void test5() {
+ Wrapper w((FullyDerived()));
+ w.d.x = 1;
+
+ Wrapper w2(w);
+ clang_analyzer_eval(getX(w2.d) == 1); // expected-warning{{TRUE}}
+ }
+ }
+#endif
}
More information about the cfe-commits
mailing list