[analyzer] Check for negative values used as the size of a C variable-length array.

Patch by Daniel Fahlgren!

llvm-svn: 215456

3 files changed, 110 insertions, 15 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 198a6285c90..cceffef82b3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -30,7 +30,7 @@ using namespace ento;
 namespace {
 class VLASizeChecker : public Checker< check::PreStmt<DeclStmt> > {
   mutable std::unique_ptr<BugType> BT;
-  enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted };
+  enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
 
   void reportBug(VLASize_Kind Kind,
                  const Expr *SizeE,
@@ -67,6 +67,9 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind,
   case VLA_Tainted:
     os << "has tainted size";
     break;
+  case VLA_Negative:
+    os << "has negative size";
+    break;
   }
 
   BugReport *report = new BugReport(*BT, os.str(), N);
@@ -128,8 +131,27 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   // declared. We do this by multiplying the array length by the element size,
   // then matching that with the array region's extent symbol.
 
-  // Convert the array length to size_t.
+  // Check if the size is negative.
   SValBuilder &svalBuilder = C.getSValBuilder();
+
+  QualType Ty = SE->getType();
+  DefinedOrUnknownSVal Zero = svalBuilder.makeZeroVal(Ty);
+
+  SVal LessThanZeroVal = svalBuilder.evalBinOp(state, BO_LT, sizeD, Zero, Ty);
+  if (Optional<DefinedSVal> LessThanZeroDVal =
+        LessThanZeroVal.getAs<DefinedSVal>()) {
+    ConstraintManager &CM = C.getConstraintManager();
+    ProgramStateRef StatePos, StateNeg;
+
+    std::tie(StateNeg, StatePos) = CM.assumeDual(state, *LessThanZeroDVal);
+    if (StateNeg && !StatePos) {
+      reportBug(VLA_Negative, SE, state, C);
+      return;
+    }
+    state = StatePos;
+  }
+
+  // Convert the array length to size_t.
   QualType SizeTy = Ctx.getSizeType();
   NonLoc ArrayLength =
       svalBuilder.evalCast(sizeD, SizeTy, SE->getType()).castAs<NonLoc>();
diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index 6da9604467d..ad7393b82d3 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -118,19 +118,6 @@ __m128i vec128i(long long __q1, long long __q0) {
   return __extension__ (__m128i)(__v2di){ __q0, __q1 };
 }
 
-// Zero-sized VLAs.
-void check_zero_sized_VLA(int x) {
-  if (x)
-    return;
-
-  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has zero size}}
-}
-
-void check_uninit_sized_VLA() {
-  int x;
-  int vla[x]; // expected-warning{{Declared variable-length array (VLA) uses a garbage value as its size}}
-}
-
 // sizeof(void)
 // - Tests a regression reported in PR 3211: http://llvm.org/bugs/show_bug.cgi?id=3211
 void handle_sizeof_void(unsigned flag) {
diff --git a/clang/test/Analysis/vla.c b/clang/test/Analysis/vla.c
new file mode 100644
index 00000000000..f94bea96e8f
--- /dev/null
+++ b/clang/test/Analysis/vla.c
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+
+// Zero-sized VLAs.
+void check_zero_sized_VLA(int x) {
+  if (x)
+    return;
+
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has zero size}}
+}
+
+void check_uninit_sized_VLA() {
+  int x;
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) uses a garbage value as its size}}
+}
+
+// Negative VLAs.
+static void vla_allocate_signed(int x) {
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has negative size}}
+}
+
+static void vla_allocate_unsigned(unsigned int x) {
+  int vla[x]; // no-warning
+}
+
+void check_negative_sized_VLA_1() {
+  vla_allocate_signed(-1);
+}
+
+void check_negative_sized_VLA_2() {
+  vla_allocate_unsigned(-1);
+}
+
+void check_negative_sized_VLA_3() {
+  int x = -1;
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has negative size}}
+}
+
+void check_negative_sized_VLA_4() {
+  unsigned int x = -1;
+  int vla[x]; // no-warning
+}
+
+void check_negative_sized_VLA_5() {
+  signed char x = -1;
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has negative size}}
+}
+
+void check_negative_sized_VLA_6() {
+  unsigned char x = -1;
+  int vla[x]; // no-warning
+}
+
+void check_negative_sized_VLA_7() {
+  signed char x = -1;
+  int vla[x + 2]; // no-warning
+}
+
+void check_negative_sized_VLA_8() {
+  signed char x = 1;
+  int vla[x - 2]; // expected-warning{{Declared variable-length array (VLA) has negative size}}
+}
+
+void check_negative_sized_VLA_9() {
+  int x = 1;
+  int vla[x]; // no-warning
+}
+
+static void check_negative_sized_VLA_10_sub(int x)
+{
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has negative size}}
+}
+
+void check_negative_sized_VLA_10(int x) {
+  if (x < 0)
+    check_negative_sized_VLA_10_sub(x);
+}
+
+static void check_negative_sized_VLA_11_sub(int x)
+{
+  int vla[x]; // no-warning
+}
+
+void check_negative_sized_VLA_11(int x) {
+  if (x > 0)
+    check_negative_sized_VLA_11_sub(x);
+}