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

Jordan Rose jordan_rose at apple.com
Wed Jan 30 18:57:06 PST 2013


Author: jrose
Date: Wed Jan 30 20:57:06 2013
New Revision: 174031

URL: http://llvm.org/viewvc/llvm-project?rev=174031&view=rev
Log:
[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>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=174031&r1=174030&r2=174031&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Jan 30 20:57:06 2013
@@ -1384,9 +1384,31 @@ RegionStoreManager::getBindingForDerived
 
 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

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=174031&r1=174030&r2=174031&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/uninit-vals.m (original)
+++ cfe/trunk/test/Analysis/uninit-vals.m Wed Jan 30 20:57:06 2013
@@ -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);
+}
+





More information about the cfe-commits mailing list