[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 48f0ab962f967d71920482e821f0bc7607375983..0fb5af6d07e1a1010a137bf77309f941a43b6ad0 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -33,7 +33,7 @@ class LazyCompoundValData;
 class ProgramState;
 class BasicValueFactory;
 class MemRegion;
-class TypedRegion;
+class TypedValueRegion;
 class MemRegionManager;
 class ProgramStateManager;
 class SValBuilder;
@@ -379,7 +379,7 @@ public:
     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 06218664f7a942ddf463d6da69610ab9640713c9..61518e61162f6222ce438fdfd55713ed7ec738b3 100644 (file)
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -475,9 +475,9 @@ public: // Part of public interface to class.
   /// 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 @@ SVal RegionStoreManager::getBindingForLazySymbol(const TypedValueRegion *R) {
   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 04f24fa1d3e35c550d4fbd61f6a15f4f58dde283..5906068c8961f0a55dd27e8591ba776b2fd8899f 100644 (file)
--- a/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/lib/StaticAnalyzer/Core/SVals.cpp
@@ -144,7 +144,7 @@ const void *nonloc::LazyCompoundVal::getStore() const {
   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 c4d9aff95fbc1b7745f68267d6fd09af03128491..dfb4cf13a9b95eb13303030f6500d3cbb4e31439 100644 (file)
--- a/test/Analysis/array-struct-region.c
+++ b/test/Analysis/array-struct-region.c
@@ -267,6 +267,50 @@ static void radar13116945(struct point centerCoordinate) {
   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
 // --------------------