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

[Ted Kremenek]

Author: kremenek
Date: Mon Apr  2 19:03:34 2012
New Revision: 153913

URL: http://llvm.org/viewvc/llvm-project?rev=153913&view=rev
Log:
Fix another false positive in RegionStore involving doing loads from symbolic offsets.  We still don't
properly reason about such accesses, but we shouldn't emit bogus "uninitialized value" warnings
either.  Fixes <rdar://problem/11127008>.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=153913&r1=153912&r2=153913&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Mon Apr  2 19:03:34 2012
@@ -1194,14 +1194,23 @@
 
   // At this point we have already checked in either getBindingForElement or
   // getBindingForField if 'R' has a direct binding.
-
   RegionBindings B = GetRegionBindings(store);
+  
+  // Record whether or not we see a symbolic index.  That can completely
+  // be out of scope of our lookup.
+  bool hasSymbolicIndex = false;
 
   while (superR) {
     if (const Optional<SVal> &D =
         getBindingForDerivedDefaultValue(B, superR, R, Ty))
       return *D;
 
+    if (const ElementRegion *ER = dyn_cast<ElementRegion>(superR)) {
+      NonLoc index = ER->getIndex();
+      if (!index.isConstant())
+        hasSymbolicIndex = true;
+    }
+    
     // If our super region is a field or element itself, walk up the region
     // hierarchy to see if there is a default value installed in an ancestor.
     if (const SubRegion *SR = dyn_cast<SubRegion>(superR)) {
@@ -1220,7 +1229,7 @@
     return getLazyBinding(lazyBindingRegion, lazyBindingStore);
 
   if (R->hasStackNonParametersStorage()) {
-    if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
+    if (isa<ElementRegion>(R)) {
       // Currently we don't reason specially about Clang-style vectors.  Check
       // if superR is a vector and if so return Unknown.
       if (const TypedValueRegion *typedSuperR = 
@@ -1228,13 +1237,15 @@
         if (typedSuperR->getValueType()->isVectorType())
           return UnknownVal();
       }
-      
-      // FIXME: We also need to take ElementRegions with symbolic indexes into
-      // account.
-      if (!ER->getIndex().isConstant())
-        return UnknownVal();
     }
 
+    // FIXME: We also need to take ElementRegions with symbolic indexes into
+    // account.  This case handles both directly accessing an ElementRegion
+    // with a symbolic offset, but also fields within an element with
+    // a symbolic offset.
+    if (hasSymbolicIndex)
+      return UnknownVal();
+    
     return UndefinedVal();
   }
 

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=153913&r1=153912&r2=153913&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-region-store.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-region-store.m Mon Apr  2 19:03:34 2012
@@ -1325,3 +1325,19 @@
   *dst = '\0';
 }
 
+// Test handling symbolic elements with field accesses.
+// <rdar://problem/11127008>
+typedef struct {
+    unsigned value;
+} RDar11127008;
+
+signed rdar_11127008_index();
+
+static unsigned rdar_11127008(void) {
+    RDar11127008 values[] = {{.value = 0}, {.value = 1}};
+    signed index = rdar_11127008_index();
+    if (index < 0) return 0;
+    if (index >= 2) return 0;
+    return values[index].value;
+}
+