[analyzer] Make VLA checker taint aware.

Also, slightly modify the diagnostic message in ArrayBound and DivZero (still use 'taint', which might not mean much to the user, but plan on changing it later).

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

diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 041e74e764fe7bcd6db1be0d55c2db5bffad8db9..2c7dcd47a9e7f72031cdbf7eb1d7f22cbefe3854 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -28,7 +28,7 @@ class ArrayBoundCheckerV2 :
     public Checker<check::Location> {
   mutable llvm::OwningPtr<BuiltinBug> BT;
       
-  enum OOB_Kind { OOB_Precedes, OOB_Excedes };
+  enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted };
   
   void reportOOB(CheckerContext &C, const ProgramState *errorState,
                  OOB_Kind kind) const;
@@ -157,7 +157,7 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
     // If we are under constrained and the index variables are tainted, report.
     if (state_exceedsUpperBound && state_withinUpperBound) {
       if (state->isTainted(rawOffset.getByteOffset()))
-        reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
+        reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted);
         return;
     }
   
@@ -193,9 +193,18 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext,
 
   llvm::SmallString<256> buf;
   llvm::raw_svector_ostream os(buf);
-  os << "Out of bound memory access "
-     << (kind == OOB_Precedes ? "(accessed memory precedes memory block)"
-                              : "(access exceeds upper limit of memory block)");
+  os << "Out of bound memory access ";
+  switch (kind) {
+  case OOB_Precedes:
+    os << "(accessed memory precedes memory block)";
+    break;
+  case OOB_Excedes:
+    os << "(access exceeds upper limit of memory block)";
+    break;
+  case OOB_Tainted:
+    os << "(index is tainted)";
+    break;
+  }
 
   checkerContext.EmitReport(new BugReport(*BT, os.str(), errorNode));
 }

diff --git a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
index b9ed384e0aa8199a99dff4c0f5cecc09c63c7266..9f2f5151bff827605eb78d976ae10a1ad24fed1b 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
@@ -37,10 +37,10 @@ void DivZeroChecker::reportBug(const char *Msg,
                                CheckerContext &C) const {
   if (ExplodedNode *N = C.generateSink(StateZero)) {
     if (!BT)
-      BT.reset(new BuiltinBug(Msg));
+      BT.reset(new BuiltinBug("Division by zero"));
 
     BugReport *R =
-      new BugReport(*BT, BT->getDescription(), N);
+      new BugReport(*BT, Msg, N);
 
     R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
                                  bugreporter::GetDenomExpr(N)));

diff --git a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 0308bf5c11b6365dd846c1febb0ce27062f01d22..74d4f24348d15c298750f668944bbc855b5bb29d 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -26,14 +26,52 @@ using namespace ento;
 
 namespace {
 class VLASizeChecker : public Checker< check::PreStmt<DeclStmt> > {
-  mutable llvm::OwningPtr<BugType> BT_zero;
-  mutable llvm::OwningPtr<BugType> BT_undef;
-  
+  mutable llvm::OwningPtr<BugType> BT;
+  enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted };
+
+  void reportBug(VLASize_Kind Kind,
+                 const Expr *SizeE,
+                 const ProgramState *State,
+                 CheckerContext &C) const;
 public:
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
 };
 } // end anonymous namespace
 
+void VLASizeChecker::reportBug(VLASize_Kind Kind,
+                               const Expr *SizeE,
+                               const ProgramState *State,
+                               CheckerContext &C) const {
+  // Generate an error node.
+  ExplodedNode *N = C.generateSink(State);
+  if (!N)
+    return;
+
+  if (!BT)
+    BT.reset(new BuiltinBug("Dangerous variable-length array (VLA) declaration"));
+
+  llvm::SmallString<256> buf;
+  llvm::raw_svector_ostream os(buf);
+  os << "Declared variable-length array (VLA) ";
+  switch (Kind) {
+  case VLA_Garbage:
+    os << "uses a garbage value as its size";
+    break;
+  case VLA_Zero:
+    os << "has zero size";
+    break;
+  case VLA_Tainted:
+    os << "has tainted size";
+    break;
+  }
+
+  BugReport *report = new BugReport(*BT, os.str(), N);
+  report->addRange(SizeE->getSourceRange());
+  report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SizeE));
+  C.EmitReport(report);
+  return;
+}
+
 void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   if (!DS->isSingleDecl())
     return;
@@ -53,20 +91,7 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   SVal sizeV = state->getSVal(SE, C.getLocationContext());
 
   if (sizeV.isUndef()) {
-    // Generate an error node.
-    ExplodedNode *N = C.generateSink();
-    if (!N)
-      return;
-    
-    if (!BT_undef)
-      BT_undef.reset(new BuiltinBug("Declared variable-length array (VLA) "
-                                    "uses a garbage value as its size"));
-
-    BugReport *report =
-      new BugReport(*BT_undef, BT_undef->getName(), N);
-    report->addRange(SE->getSourceRange());
-    report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SE));
-    C.EmitReport(report);
+    reportBug(VLA_Garbage, SE, state, C);
     return;
   }
 
@@ -75,6 +100,12 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   if (sizeV.isUnknown())
     return;
   
+  // Check if the size is tainted.
+  if (state->isTainted(sizeV)) {
+    reportBug(VLA_Tainted, SE, 0, C);
+    return;
+  }
+
   // Check if the size is zero.
   DefinedSVal sizeD = cast<DefinedSVal>(sizeV);
 
@@ -82,16 +113,7 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   llvm::tie(stateNotZero, stateZero) = state->assume(sizeD);
 
   if (stateZero && !stateNotZero) {
-    ExplodedNode *N = C.generateSink(stateZero);
-    if (!BT_zero)
-      BT_zero.reset(new BuiltinBug("Declared variable-length array (VLA) has "
-                                   "zero size"));
-
-    BugReport *report =
-      new BugReport(*BT_zero, BT_zero->getName(), N);
-    report->addRange(SE->getSourceRange());
-    report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SE));
-    C.EmitReport(report);
+    reportBug(VLA_Zero, SE, stateZero, C);
     return;
   }
  

diff --git a/test/Analysis/taint-generic.c b/test/Analysis/taint-generic.c
index be20f2c24f600dfcfbc85a329f3b96e3548a84dc..d52dcda5a1f5290ecbc33cd02716c64ddcdd2d3e 100644 (file)
--- a/test/Analysis/taint-generic.c
+++ b/test/Analysis/taint-generic.c
@@ -70,7 +70,7 @@ void scanfArg() {
 
 void bufferGetchar(int x) {
   int m = getchar();
-  Buffer[m] = 1;  //expected-warning {{Out of bound memory access }}
+  Buffer[m] = 1;  //expected-warning {{Out of bound memory access (index is tainted)}}
 }
 
 void testUncontrolledFormatString(char **p) {
@@ -176,3 +176,10 @@ int testDivByZero() {
   scanf("%d", &x);
   return 5/x; // expected-warning {{Division by a tainted value, possibly zero}}
 }
+
+// Zero-sized VLAs.
+void testTaintedVLASize() {
+  int x;
+  scanf("%d", &x);
+  int vla[x]; // expected-warning{{Declared variable-length array (VLA) has tainted size}}
+}