[cfe-commits] r159866 - in /cfe/trunk: lib/StaticAnalyzer/Core/RegionStore.cpp test/Analysis/array-struct-region.c

[Jordan Rose]

Author: jrose
Date: Fri Jul  6 16:59:56 2012
New Revision: 159866

URL: http://llvm.org/viewvc/llvm-project?rev=159866&view=rev
Log:
[analyzer] Be careful about LazyCompoundVals, which may be for the first field.

We use LazyCompoundVals to avoid copying the contents of structs and arrays
around in the store, and when we need to pass a struct around that already
has a LazyCompoundVal we just use the original one. However, it's possible
that the first field of a struct may have a LazyCompoundVal of its own, and
we currently can't distinguish a LazyCompoundVal for the first element of a
struct from a LazyCompoundVal for the entire struct. In this case we should
just drop the optimization and make a new LazyCompoundVal that encompasses
the old one.

PR13264 / <rdar://problem/11802440>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/array-struct-region.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=159866&r1=159865&r2=159866&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri Jul  6 16:59:56 2012
@@ -1447,16 +1447,27 @@
   return svalBuilder.getRegionValueSymbolVal(R);
 }
 
+static bool mayHaveLazyBinding(QualType Ty) {
+  return Ty->isArrayType() || Ty->isStructureOrClassType();
+}
+
 SVal RegionStoreManager::getBindingForStruct(Store store, 
                                         const TypedValueRegion* R) {
-  assert(R->getValueType()->isStructureOrClassType());
-  
-  // If we already have a lazy binding, don't create a new one.
-  RegionBindings B = GetRegionBindings(store);
-  BindingKey K = BindingKey::Make(R, BindingKey::Default);
-  if (const nonloc::LazyCompoundVal *V =
-      dyn_cast_or_null<nonloc::LazyCompoundVal>(lookup(B, K))) {
-    return *V;
+  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)) {
+    RegionBindings B = GetRegionBindings(store);
+    BindingKey K = BindingKey::Make(R, BindingKey::Default);
+    if (const nonloc::LazyCompoundVal *V =
+          dyn_cast_or_null<nonloc::LazyCompoundVal>(lookup(B, K))) {
+      return *V;
+    }
   }
 
   return svalBuilder.makeLazyCompoundVal(StoreRef(store, *this), R);
@@ -1464,14 +1475,19 @@
 
 SVal RegionStoreManager::getBindingForArray(Store store,
                                        const TypedValueRegion * R) {
-  assert(Ctx.getAsConstantArrayType(R->getValueType()));
+  const ConstantArrayType *Ty = Ctx.getAsConstantArrayType(R->getValueType());
+  assert(Ty && "Only constant array types can have compound bindings.");
   
-  // If we already have a lazy binding, don't create a new one.
-  RegionBindings B = GetRegionBindings(store);
-  BindingKey K = BindingKey::Make(R, BindingKey::Default);
-  if (const nonloc::LazyCompoundVal *V =
-      dyn_cast_or_null<nonloc::LazyCompoundVal>(lookup(B, K))) {
-    return *V;
+  // 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())) {
+    RegionBindings B = GetRegionBindings(store);
+    BindingKey K = BindingKey::Make(R, BindingKey::Default);
+    if (const nonloc::LazyCompoundVal *V =
+        dyn_cast_or_null<nonloc::LazyCompoundVal>(lookup(B, K))) {
+      return *V;
+    }
   }
 
   return svalBuilder.makeLazyCompoundVal(StoreRef(store, *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=159866&r1=159865&r2=159866&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/array-struct-region.c (original)
+++ cfe/trunk/test/Analysis/array-struct-region.c Fri Jul  6 16:59:56 2012
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=basic -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=range -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=basic -analyzer-ipa=all -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=range -analyzer-ipa=all -verify %s
 
 void clang_analyzer_eval(int);
 
@@ -57,3 +57,38 @@
   clang_analyzer_eval(p->y == 5); // expected-warning{{TRUE}}
 }
 
+
+// PR13264 / <rdar://problem/11802440>
+struct point { int x; int y; };
+struct circle { struct point o; int r; };
+struct circle get_circle() {
+  struct circle result;
+  result.r = 5;
+  result.o = (struct point){0, 0};
+  return result;
+}
+
+void struct_in_struct() {
+  struct circle c;
+  c = get_circle();
+  // This used to think c.r was undefined because c.o is a LazyCompoundVal.
+  clang_analyzer_eval(c.r == 5); // expected-warning{{TRUE}}
+}
+
+// We also test with floats because we don't model floats right now,
+// and the original bug report used a float.
+struct circle_f { struct point o; float r; };
+struct circle_f get_circle_f() {
+  struct circle_f result;
+  result.r = 5.0;
+  result.o = (struct point){0, 0};
+  return result;
+}
+
+float struct_in_struct_f() {
+  struct circle_f c;
+  c = get_circle_f();
+
+  return c.r; // no-warning
+}
+