r321290 - [analyzer] Fix zero-initialization of stack VLAs under ObjC ARC.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 21 10:43:02 PST 2017


Author: dergachev
Date: Thu Dec 21 10:43:02 2017
New Revision: 321290

URL: http://llvm.org/viewvc/llvm-project?rev=321290&view=rev
Log:
[analyzer] Fix zero-initialization of stack VLAs under ObjC ARC.

Using ARC, strong, weak, and autoreleasing stack variables are implicitly
initialized with nil. This includes variable-length arrays of Objective-C object
pointers. However, in the analyzer we don't zero-initialize them. We used to,
but it accidentally regressed after r289618.

Under ARC, the array variable's initializer within DeclStmt is an
ImplicitValueInitExpr. Environment doesn't maintain any bindings for this
expression kind - instead it always knows that it's a known constant
(0 in our case), so it just returns the known value by calling
SValBuilder::makeZeroVal() (see EnvironmentManager::getSVal().
Commit r289618 had introduced reasonable behavior of SValBuilder::makeZeroVal()
for the arrays, which produces a zero-length compoundVal{}. When such value
is bound to arrays, in RegionStoreManager::bindArray() "remaining" items in the
array are default-initialized with zero, as in
RegionStoreManager::setImplicitDefaultValue(). The similar mechanism works when
an array is initialized by an initializer list that is too short, eg.
  int a[3] = { 1, 2 };
would result in a[2] initialized with 0. However, in case of variable-length
arrays it didn't know if any more items need to be added,
because, well, the length is variable.

Add the default binding anyway, regardless of how many actually need
to be added. We don't really care how many, because the default binding covers
the whole array anyway.

Differential Revision: https://reviews.llvm.org/D41478
rdar://problem/35477763

Added:
    cfe/trunk/test/Analysis/arc-zero-init.m
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=321290&r1=321289&r2=321290&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Dec 21 10:43:02 2017
@@ -2132,9 +2132,10 @@ RegionStoreManager::bindArray(RegionBind
       NewB = bind(NewB, loc::MemRegionVal(ER), *VI);
   }
 
-  // If the init list is shorter than the array length, set the
-  // array default value.
-  if (Size.hasValue() && i < Size.getValue())
+  // If the init list is shorter than the array length (or the array has
+  // variable length), set the array default value. Values that are already set
+  // are not overwritten.
+  if (!Size.hasValue() || i < Size.getValue())
     NewB = setImplicitDefaultValue(NewB, R, ElementTy);
 
   return NewB;

Added: cfe/trunk/test/Analysis/arc-zero-init.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/arc-zero-init.m?rev=321290&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/arc-zero-init.m (added)
+++ cfe/trunk/test/Analysis/arc-zero-init.m Thu Dec 21 10:43:02 2017
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -fobjc-arc %s
+
+#if __has_feature(objc_arc)
+// expected-no-diagnostics
+#endif
+
+ at interface SomeClass
+ at end
+
+void simpleStrongPointerValue() {
+  SomeClass *x;
+  if (x) {}
+#if !__has_feature(objc_arc)
+// expected-warning at -2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void simpleArray() {
+  SomeClass *vlaArray[5];
+
+  if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+// expected-warning at -2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void variableLengthArray() {
+   int count = 1;
+   SomeClass * vlaArray[count];
+
+   if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+  // expected-warning at -2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void variableLengthArrayWithExplicitStrongAttribute() {
+   int count = 1;
+   __attribute__((objc_ownership(strong))) SomeClass * vlaArray[count];
+
+   if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+  // expected-warning at -2{{Branch condition evaluates to a garbage value}}
+#endif
+}




More information about the cfe-commits mailing list