[analyzer] If a lazy binding is undefined, pretend that it's unknown instead.

This is a hack to work around the fact that we don't track extents for our
default bindings:

  CGPoint p;
  p.x = 0.0;
  p.y = 0.0;
  rectParam.origin = p;
  use(rectParam.size); // warning: uninitialized value in rectParam.size.width

In this case, the default binding for 'p' gets copied into 'rectParam',
because the 'origin' field is at offset 0 within CGRect. From then on,
rectParam's old default binding (in this case a symbol) is lost.

This patch silences the warning by pretending that lazy bindings are never
made from uninitialized memory, but not only is that not true, the original
default binding is still getting overwritten (see FIXME test cases).
The long-term solution is tracked in <rdar://problem/12701038>

PR14765 and <rdar://problem/12875012>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174031 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index a37223a528d57b2f09eb74f8911890b12c3420b8..06218664f7a942ddf463d6da69610ab9640713c9 100644 (file)
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1384,9 +1384,31 @@ RegionStoreManager::getBindingForDerivedDefaultValue(RegionBindingsConstRef B,
 
 SVal RegionStoreManager::getLazyBinding(const MemRegion *LazyBindingRegion,
                                         RegionBindingsRef LazyBinding) {
+  SVal Result;
   if (const ElementRegion *ER = dyn_cast<ElementRegion>(LazyBindingRegion))
-    return getBindingForElement(LazyBinding, ER);
-  return getBindingForField(LazyBinding, cast<FieldRegion>(LazyBindingRegion));
+    Result = getBindingForElement(LazyBinding, ER);
+  else
+    Result = getBindingForField(LazyBinding,
+                                cast<FieldRegion>(LazyBindingRegion));
+
+  // This is a hack to deal with RegionStore's inability to distinguish a
+  // default value for /part/ of an aggregate from a default value for the
+  // /entire/ aggregate. The most common case of this is when struct Outer
+  // has as its first member a struct Inner, which is copied in from a stack
+  // variable. In this case, even if the Outer's default value is symbolic, 0,
+  // or unknown, it gets overridden by the Inner's default value of undefined.
+  //
+  // This is a general problem -- if the Inner is zero-initialized, the Outer
+  // will now look zero-initialized. The proper way to solve this is with a
+  // new version of RegionStore that tracks the extent of a binding as well
+  // as the offset.
+  //
+  // This hack only takes care of the undefined case because that can very
+  // quickly result in a warning.
+  if (Result.isUndef())
+    Result = UnknownVal();
+
+  return Result;
 }
                                         
 SVal

diff --git a/test/Analysis/uninit-vals.m b/test/Analysis/uninit-vals.m
index 1cd57590df054420fe3327c54ba5c77c34adddfc..57e83e3f9754ae06157e66e5a0e8fa41911e2677 100644 (file)
--- a/test/Analysis/uninit-vals.m
+++ b/test/Analysis/uninit-vals.m
@@ -1,7 +1,13 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -verify %s
 
 typedef unsigned int NSUInteger;
+typedef __typeof__(sizeof(int)) size_t;
+
+void *malloc(size_t);
+void *calloc(size_t nmemb, size_t size);
+void free(void *);
+
+void clang_analyzer_eval(int);
 
 @interface A
 - (NSUInteger)foo;
@@ -32,3 +38,54 @@ void PR10163 (void) {
   float x[2] = {0};
   test_PR10163(x[1]); // no-warning  
 }
+
+
+typedef struct {
+  float x;
+  float y;
+} Point;
+typedef struct {
+  Point origin;
+  int size;
+} Circle;
+
+Point makePoint(float x, float y) {
+  Point result;
+  result.x = x;
+  result.y = y;
+  return result;
+}
+
+void PR14765_test() {
+  Circle *testObj = calloc(sizeof(Circle), 1);
+
+  clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}}
+
+  testObj->origin = makePoint(0.0, 0.0);
+  if (testObj->size > 0) { ; } // warning occurs here
+
+  // FIXME: Assigning to 'testObj->origin' kills the default binding for the
+  // whole region, meaning that we've forgotten that testObj->size should also
+  // default to 0. Tracked by <rdar://problem/12701038>.
+  // This should be TRUE.
+  clang_analyzer_eval(testObj->size == 0); // expected-warning{{UNKNOWN}}
+
+  free(testObj);
+}
+
+void PR14765_incorrectBehavior(Circle *testObj) {
+  int oldSize = testObj->size;
+
+  clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}}
+
+  testObj->origin = makePoint(0.0, 0.0);
+
+  // FIXME: Assigning to 'testObj->origin' kills the default binding for the
+  // whole region, meaning that we've forgotten that testObj->size should also
+  // default to 0. Tracked by <rdar://problem/12701038>.
+  // This should be TRUE.
+  clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{UNKNOWN}}
+
+  free(testObj);
+}
+