[cfe-commits] r123368 - in /cfe/trunk: lib/StaticAnalyzer/RegionStore.cpp test/Analysis/misc-ps-region-store.m

[Ted Kremenek]

Author: kremenek
Date: Thu Jan 13 00:58:15 2011
New Revision: 123368

URL: http://llvm.org/viewvc/llvm-project?rev=123368&view=rev
Log:
Fix a corner case in RegionStore where we assign
a struct value to a symbolic index into array.
RegionStore can't actually reason about this,
so we were getting bogus warnings about loading
uninitialized values from the array.  The solution
is invalidate the entire array when we cannot
represent the binding explicitly.

Fixes <rdar://problem/8848957>

Modified:
    cfe/trunk/lib/StaticAnalyzer/RegionStore.cpp
    cfe/trunk/test/Analysis/misc-ps-region-store.m

Modified: cfe/trunk/lib/StaticAnalyzer/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/RegionStore.cpp?rev=123368&r1=123367&r2=123368&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/RegionStore.cpp Thu Jan 13 00:58:15 2011
@@ -1572,13 +1572,38 @@
 
 Store RegionStoreManager::KillStruct(Store store, const TypedRegion* R,
                                      SVal DefaultVal) {
+  
+  BindingKey key = BindingKey::Make(R, BindingKey::Default);
+  
+  // The BindingKey may be "invalid" if we cannot handle the region binding
+  // explicitly.  One example is something like array[index], where index
+  // is a symbolic value.  In such cases, we want to invalidate the entire
+  // array, as the index assignment could have been to any element.  In
+  // the case of nested symbolic indices, we need to march up the region
+  // hierarchy untile we reach a region whose binding we can reason about.
+  const SubRegion *subReg = R;
+  
+  while (!key.isValid()) {
+    if (const SubRegion *tmp = dyn_cast<SubRegion>(subReg->getSuperRegion())) {
+      subReg = tmp;
+      key = BindingKey::Make(tmp, BindingKey::Default);
+    }
+    else
+      break;
+  }                                 
+
+  // Remove the old bindings, using 'subReg' as the root of all regions
+  // we will invalidate.
   RegionBindings B = GetRegionBindings(store);
   llvm::OwningPtr<RegionStoreSubRegionMap>
     SubRegions(getRegionStoreSubRegionMap(store));
-  RemoveSubRegionBindings(B, R, *SubRegions);
+  RemoveSubRegionBindings(B, subReg, *SubRegions);
 
   // Set the default value of the struct region to "unknown".
-  return addBinding(B, R, BindingKey::Default, DefaultVal).getRoot();
+  if (!key.isValid())
+    return B.getRoot();
+  
+  return addBinding(B, key, DefaultVal).getRoot();
 }
 
 Store RegionStoreManager::CopyLazyBindings(nonloc::LazyCompoundVal V,

Modified: cfe/trunk/test/Analysis/misc-ps-region-store.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.m?rev=123368&r1=123367&r2=123368&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-region-store.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-region-store.m Thu Jan 13 00:58:15 2011
@@ -1203,3 +1203,17 @@
     rdar_8642434_funcA(y); // expected-warning{{Null pointer passed as an argument to a 'nonnull' parameter}}
 }
 
+// <rdar://problem/8848957> - Handle loads and stores from a symbolic index
+// into array without warning about an uninitialized value being returned.
+// While RegionStore can't fully reason about this example, it shouldn't
+// warn here either.
+typedef struct s_test_rdar8848957 {
+  int x, y, z;
+} s_test_rdar8848957;
+
+s_test_rdar8848957 foo_rdar8848957();
+int rdar8848957(int index) {
+  s_test_rdar8848957 vals[10];
+  vals[index] = foo_rdar8848957();
+  return vals[index].x; // no-warning
+}