r174211 - [analyzer] Reuse a LazyCompoundVal if its type matches the new region.

Jordan Rose jordan_rose at apple.com
Fri Feb 1 11:49:57 PST 2013


Author: jrose
Date: Fri Feb  1 13:49:57 2013
New Revision: 174211

URL: http://llvm.org/viewvc/llvm-project?rev=174211&view=rev
Log:
[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.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
    cfe/trunk/test/Analysis/array-struct-region.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=174211&r1=174210&r2=174211&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Fri Feb  1 13:49:57 2013
@@ -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 &&

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=174211&r1=174210&r2=174211&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri Feb  1 13:49:57 2013
@@ -475,9 +475,9 @@ public: // Part of public interface to c
   /// 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::getBindingForLa
   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,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp?rev=174211&r1=174210&r2=174211&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp Fri Feb  1 13:49:57 2013
@@ -144,7 +144,7 @@ const void *nonloc::LazyCompoundVal::get
   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();
 }
 

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=174211&r1=174210&r2=174211&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/array-struct-region.c (original)
+++ cfe/trunk/test/Analysis/array-struct-region.c Fri Feb  1 13:49:57 2013
@@ -267,6 +267,50 @@ static void radar13116945(struct point c
   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
 // --------------------





More information about the cfe-commits mailing list