[analyzer] Reuse a LazyCompoundVal if its type matches the new region.
This allows us to keep from chaining LazyCompoundVals in cases like this:
CGRect r = CGRectMake(0, 0, 640, 480);
CGRect r2 = r;
CGRect r3 = r2;
Previously we only made this optimization if the struct did not begin with
an aggregate member, to make sure that we weren't picking up an LCV for
the first field of the struct. But since LazyCompoundVals are typed, we can
make that inference directly by comparing types.
This is a pure optimization; the test changes are to guard against possible
future regressions.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174211 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 48f0ab9..0fb5af6 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -33,7 +33,7 @@
class ProgramState;
class BasicValueFactory;
class MemRegion;
-class TypedRegion;
+class TypedValueRegion;
class MemRegionManager;
class ProgramStateManager;
class SValBuilder;
@@ -379,7 +379,7 @@
return static_cast<const LazyCompoundValData*>(Data);
}
const void *getStore() const;
- const TypedRegion *getRegion() const;
+ const TypedValueRegion *getRegion() const;
static bool classof(const SVal *V) {
return V->getBaseKind() == NonLocKind &&
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index 0621866..61518e6 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -475,9 +475,9 @@
/// struct s x, y;
/// x = y;
/// y's value is retrieved by this method.
- SVal getBindingForStruct(RegionBindingsConstRef B, const TypedValueRegion* R);
-
- SVal getBindingForArray(RegionBindingsConstRef B, const TypedValueRegion* R);
+ SVal getBindingForStruct(RegionBindingsConstRef B, const TypedValueRegion *R);
+ SVal getBindingForArray(RegionBindingsConstRef B, const TypedValueRegion *R);
+ NonLoc createLazyBinding(RegionBindingsConstRef B, const TypedValueRegion *R);
/// Used to lazily generate derived symbols for bindings that are defined
/// implicitly by default bindings in a super region.
@@ -1551,48 +1551,39 @@
return svalBuilder.getRegionValueSymbolVal(R);
}
-static bool mayHaveLazyBinding(QualType Ty) {
- return Ty->isArrayType() || Ty->isStructureOrClassType();
+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)) {
+ const nonloc::LazyCompoundVal *LCV =
+ dyn_cast<nonloc::LazyCompoundVal>(V.getPointer());
+ if (LCV) {
+ QualType RegionTy = R->getValueType();
+ QualType SourceRegionTy = LCV->getRegion()->getValueType();
+ if (RegionTy.getCanonicalType() == SourceRegionTy.getCanonicalType())
+ return *LCV;
+ }
+ }
+
+ return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R);
}
SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B,
- const TypedValueRegion* R) {
+ const TypedValueRegion *R) {
const RecordDecl *RD = R->getValueType()->castAs<RecordType>()->getDecl();
if (RD->field_empty())
return UnknownVal();
- // If we already have a lazy binding, don't create a new one,
- // unless the first field might have a lazy binding of its own.
- // (Right now we can't tell the difference.)
- QualType FirstFieldType = RD->field_begin()->getType();
- if (!mayHaveLazyBinding(FirstFieldType)) {
- BindingKey K = BindingKey::Make(R, BindingKey::Default);
- if (const nonloc::LazyCompoundVal *V =
- dyn_cast_or_null<nonloc::LazyCompoundVal>(B.lookup(K))) {
- return *V;
- }
- }
-
- return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R);
+ return createLazyBinding(B, R);
}
SVal RegionStoreManager::getBindingForArray(RegionBindingsConstRef B,
- const TypedValueRegion * R) {
- const ConstantArrayType *Ty = Ctx.getAsConstantArrayType(R->getValueType());
- assert(Ty && "Only constant array types can have compound bindings.");
+ const TypedValueRegion *R) {
+ assert(Ctx.getAsConstantArrayType(R->getValueType()) &&
+ "Only constant array types can have compound bindings.");
- // If we already have a lazy binding, don't create a new one,
- // unless the first element might have a lazy binding of its own.
- // (Right now we can't tell the difference.)
- if (!mayHaveLazyBinding(Ty->getElementType())) {
- BindingKey K = BindingKey::Make(R, BindingKey::Default);
- if (const nonloc::LazyCompoundVal *V =
- dyn_cast_or_null<nonloc::LazyCompoundVal>(B.lookup(K))) {
- return *V;
- }
- }
-
- return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R);
+ return createLazyBinding(B, R);
}
bool RegionStoreManager::includedInBindings(Store store,
diff --git a/lib/StaticAnalyzer/Core/SVals.cpp b/lib/StaticAnalyzer/Core/SVals.cpp
index 04f24fa..5906068 100644
--- a/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/lib/StaticAnalyzer/Core/SVals.cpp
@@ -144,7 +144,7 @@
return static_cast<const LazyCompoundValData*>(Data)->getStore();
}
-const TypedRegion *nonloc::LazyCompoundVal::getRegion() const {
+const TypedValueRegion *nonloc::LazyCompoundVal::getRegion() const {
return static_cast<const LazyCompoundValData*>(Data)->getRegion();
}
diff --git a/test/Analysis/array-struct-region.c b/test/Analysis/array-struct-region.c
index c4d9aff..dfb4cf1 100644
--- a/test/Analysis/array-struct-region.c
+++ b/test/Analysis/array-struct-region.c
@@ -267,6 +267,50 @@
test13116945(r.center); // no-warning
}
+
+typedef struct {
+ char data[4];
+} ShortString;
+
+typedef struct {
+ ShortString str;
+ int length;
+} ShortStringWrapper;
+
+void testArrayStructCopy() {
+ ShortString s = { "abc" };
+ ShortString s2 = s;
+ ShortString s3 = s2;
+
+ 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}}
+}
+
+void testArrayStructCopyNested() {
+ ShortString s = { "abc" };
+ ShortString s2 = s;
+
+ ShortStringWrapper w = { s2, 0 };
+
+ clang_analyzer_eval(w.str.data[0] == 'a'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w.str.data[1] == 'b'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w.str.data[2] == 'c'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w.length == 0); // expected-warning{{TRUE}}
+
+ ShortStringWrapper w2 = w;
+ clang_analyzer_eval(w2.str.data[0] == 'a'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w2.str.data[1] == 'b'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w2.str.data[2] == 'c'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w2.length == 0); // expected-warning{{TRUE}}
+
+ ShortStringWrapper w3 = w2;
+ clang_analyzer_eval(w3.str.data[0] == 'a'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w3.str.data[1] == 'b'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w3.str.data[2] == 'c'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(w3.length == 0); // expected-warning{{TRUE}}
+}
+
// --------------------
// False positives
// --------------------