Fix regression in -Wuninitialized involving VLAs. It turns out that we were modeling sizeof(VLAs)

incorrectly in the CFG, and also the static analyzer.  This patch regresses the analyzer a bit, but
that needs to be followed up with a better solution.

Fixes <rdar://problem/10008112>.

llvm-svn: 138372

5 files changed, 43 insertions, 59 deletions

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 65dabda7e76..e0905eda218 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2203,16 +2203,7 @@ CFGBlock *CFGBuilder::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E,
     for (const VariableArrayType *VA =FindVA(E->getArgumentType().getTypePtr());
          VA != 0; VA = FindVA(VA->getElementType().getTypePtr()))
       lastBlock = addStmt(VA->getSizeExpr());
-  } else {
-    // For sizeof(x), where 'x' is a VLA, we should include the computation
-    // of the lvalue of 'x'.
-    Expr *subEx = E->getArgumentExpr();
-    if (subEx->getType()->isVariableArrayType()) {
-      assert(subEx->isLValue());
-      lastBlock = addStmt(subEx);
-    }
   }
-
   return lastBlock;
 }
 
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 3e3238cdef8..a4e640c0e98 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -503,30 +503,9 @@ VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Ex,
     if (!T->isIncompleteType() && !T->isConstantSizeType()) {
       assert(T->isVariableArrayType() && "Unknown non-constant-sized type.");
       
-      // FIXME: Add support for VLA type arguments, not just VLA expressions.
+      // FIXME: Add support for VLA type arguments and VLA expressions.
       // When that happens, we should probably refactor VLASizeChecker's code.
-      if (Ex->isArgumentType()) {
-        Dst.Add(Pred);
-        return;
-      }
-      
-      // Get the size by getting the extent of the sub-expression.
-      // First, visit the sub-expression to find its region.
-      const Expr *Arg = Ex->getArgumentExpr();
-      const ProgramState *state = Pred->getState();
-      const MemRegion *MR = state->getSVal(Arg).getAsRegion();
-      
-      // If the subexpression can't be resolved to a region, we don't know
-      // anything about its size. Just leave the state as is and continue.
-      if (!MR) {
-        Dst.Add(Pred);
-        return;
-      }
-      
-      // The result is the extent of the VLA.
-      SVal Extent = cast<SubRegion>(MR)->getExtent(svalBuilder);
-      MakeNode(Dst, Ex, Pred, state->BindExpr(Ex, Extent));
-      
+      Dst.Add(Pred);
       return;
     }
     else if (T->getAs<ObjCObjectType>()) {
diff --git a/clang/test/Analysis/outofbound-notwork.c b/clang/test/Analysis/outofbound-notwork.c
new file mode 100644
index 00000000000..45e713b6880
--- /dev/null
+++ b/clang/test/Analysis/outofbound-notwork.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -Wno-array-bounds -analyze -analyzer-checker=core,experimental.unix,experimental.security.ArrayBound -analyzer-store=region -verify %s
+// XFAIL: *
+
+// Once we better handle modeling of sizes of VLAs, we can pull this back
+// into outofbound.c.
+
+void sizeof_vla(int a) {
+  if (a == 5) {
+    char x[a];
+    int y[sizeof(x)];
+    y[4] = 4; // no-warning
+    y[5] = 5; // expected-warning{{out-of-bound}}
+  }
+}
+
+void sizeof_vla_2(int a) {
+  if (a == 5) {
+    char x[a];
+    int y[sizeof(x) / sizeof(char)];
+    y[4] = 4; // no-warning
+    y[5] = 5; // expected-warning{{out-of-bound}}
+  }
+}
+
+void sizeof_vla_3(int a) {
+  if (a == 5) {
+    char x[a];
+    int y[sizeof(*&*&*&x)];
+    y[4] = 4; // no-warning
+    y[5] = 5; // expected-warning{{out-of-bound}}
+  }
+}
diff --git a/clang/test/Analysis/outofbound.c b/clang/test/Analysis/outofbound.c
index f5a24c90683..2e7a7d30d67 100644
--- a/clang/test/Analysis/outofbound.c
+++ b/clang/test/Analysis/outofbound.c
@@ -63,33 +63,6 @@ void vla(int a) {
   }
 }
 
-void sizeof_vla(int a) {
-  if (a == 5) {
-    char x[a];
-    int y[sizeof(x)];
-    y[4] = 4; // no-warning
-    y[5] = 5; // expected-warning{{out-of-bound}}
-  }
-}
-
-void sizeof_vla_2(int a) {
-  if (a == 5) {
-    char x[a];
-    int y[sizeof(x) / sizeof(char)];
-    y[4] = 4; // no-warning
-    y[5] = 5; // expected-warning{{out-of-bound}}
-  }
-}
-
-void sizeof_vla_3(int a) {
-  if (a == 5) {
-    char x[a];
-    int y[sizeof(*&*&*&x)];
-    y[4] = 4; // no-warning
-    y[5] = 5; // expected-warning{{out-of-bound}}
-  }
-}
-
 void alloca_region(int a) {
   if (a == 5) {
     char *x = __builtin_alloca(a);
diff --git a/clang/test/Sema/uninit-variables.c b/clang/test/Sema/uninit-variables.c
index 2c91ecc2713..f26e478bdfc 100644
--- a/clang/test/Sema/uninit-variables.c
+++ b/clang/test/Sema/uninit-variables.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wconditional-uninitialized -fsyntax-only -fblocks %s -verify
 
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
 int test1() {
   int x; // expected-note{{variable 'x' is declared here}} expected-note{{add initialization to silence this warning}}
   return x; // expected-warning{{variable 'x' is uninitialized when used here}}
@@ -372,3 +375,9 @@ void PR10379_f(int *len) {
     *len += new_len; // expected-warning {{variable 'new_len' may be uninitialized when used here}}
   }
 }
+
+// Test that sizeof(VLA) doesn't trigger a warning.
+void test_vla_sizeof(int x) {
+  double (*memory)[2][x] = malloc(sizeof(*memory)); // no-warning
+}
+