Augment RegionStore::BindStruct() to bind symbolicated struct values. This fixes a false path issue reported in <rdar://problem/8243408> and also spurs another cause where the idempotent operations checker fires.

llvm-svn: 109710

clang/lib/Checker/RegionStore.cpp

@@ -289,7 +289,7 @@ public: // Part of public interface to class.
   Store BindArray(Store store, const TypedRegion* R, SVal V);
 
   /// KillStruct - Set the entire struct to unknown.
-  Store KillStruct(Store store, const TypedRegion* R);
+  Store KillStruct(Store store, const TypedRegion* R, SVal DefaultVal);
 
   Store Remove(Store store, Loc LV);
 
@@ -1560,10 +1560,11 @@ Store RegionStoreManager::BindStruct(Store store, const TypedRegion* R,
   if (const nonloc::LazyCompoundVal *LCV=dyn_cast<nonloc::LazyCompoundVal>(&V))
     return CopyLazyBindings(*LCV, store, R);
 
-  // We may get non-CompoundVal accidentally due to imprecise cast logic.
-  // Ignore them and kill the field values.
+  // We may get non-CompoundVal accidentally due to imprecise cast logic or
+  // that we are binding symbolic struct value. Kill the field values, and if
+  // the value is symbolic go and bind it as a "default" binding.
   if (V.isUnknown() || !isa<nonloc::CompoundVal>(V))
-    return KillStruct(store, R);
+    return KillStruct(store, R, isa<nonloc::SymbolVal>(V) ? V : UnknownVal());
 
   nonloc::CompoundVal& CV = cast<nonloc::CompoundVal>(V);
   nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();
@@ -1596,14 +1597,15 @@ Store RegionStoreManager::BindStruct(Store store, const TypedRegion* R,
   return store;
 }
 
-Store RegionStoreManager::KillStruct(Store store, const TypedRegion* R) {
+Store RegionStoreManager::KillStruct(Store store, const TypedRegion* R,
+                                     SVal DefaultVal) {
   RegionBindings B = GetRegionBindings(store);
   llvm::OwningPtr<RegionStoreSubRegionMap>
     SubRegions(getRegionStoreSubRegionMap(store));
   RemoveSubRegionBindings(B, R, *SubRegions);
 
   // Set the default value of the struct region to "unknown".
-  return Add(B, R, BindingKey::Default, UnknownVal()).getRoot();
+  return Add(B, R, BindingKey::Default, DefaultVal).getRoot();
 }
 
 Store RegionStoreManager::CopyLazyBindings(nonloc::LazyCompoundVal V,

clang/test/Analysis/misc-ps-region-store.m

@@ -1041,3 +1041,19 @@ void pr_7450() {
   pr_7450_aux(p + 8);
 }
 
+// <rdar://problem/8243408> - Symbolicate struct values returned by value.
+struct s_rdar_8243408 { int x; };
+extern struct s_rdar_8243408 rdar_8243408_aux(void);
+void rdar_8243408(void) {
+  struct s_rdar_8243408 a = { 1 }, *b = 0;
+  while (a.x && !b)
+    a = rdar_8243408_aux();
+
+  // Previously there was a false error here with 'b' being null.
+  (void) (a.x && b->x); // no-warning
+
+  // Introduce a null deref to ensure we are checking this path.
+  int *p = 0;
+  *p = 0xDEADBEEF; // expected-warning{{Dereference of null pointer}}
+}
+

clang/test/Analysis/uninit-vals-ps-region.m

@@ -42,7 +42,7 @@ void test_uninit_pos_3() {
 void test_uninit_neg() {
   struct TestUninit v1 = { 0, 0 };
   struct TestUninit v2 = test_uninit_aux();
-  test_unit_aux2(v2.x + v1.y); // no-warning
+  test_unit_aux2(v2.x + v1.y); // expected-warning{{The right operand to '+' is always 0}}
 }
 
 extern void test_uninit_struct_arg_aux(struct TestUninit arg);