Fixing review comments

-separating test cases for the tainteddiv and the dividezero checkers
-simplifying taint test execution lit test invocations

Author: dkrupp

clang/docs/analyzer/checkers.rst

@@ -1295,25 +1295,25 @@ optin.taint.TaintedDiv (C, C++, ObjC)
 This checker warns when the denominator in a division
 operation is a tainted (potentially attacker controlled) value.
 If the attacker can set the denominator to 0, a runtime error can
-be triggered. The checker warns if the analyzer cannot prove
-that the denominator is not 0 and it is a tainted value.
-This warning is more pessimistic than the :ref:`core-DivideZero` checker
+be triggered. The checker warns when the denominator is a  tainted
+value  and the analyzer cannot prove that it is not 0. This warning
+is more pessimistic than the :ref:`core-DivideZero` checker
 which warns only when it can prove that the denominator is 0.
 
 .. code-block:: c
 
   int vulnerable(int n) {
     size_t size = 0;
     scanf("%zu", &size);
-    return n/size; // warn: Division by a tainted value, possibly zero
+    return n / size; // warn: Division by a tainted value, possibly zero
   }
 
   int not_vulnerable(int n) {
     size_t size = 0;
     scanf("%zu", &size);
     if (!size)
       return 0;
-    return n/size; // no warning
+    return n / size; // no warning
   }
 
 .. _security-checkers:

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -1704,8 +1704,8 @@ def TaintedAllocChecker: Checker<"TaintedAlloc">,
   Documentation<HasDocumentation>;
 
 def TaintedDivChecker: Checker<"TaintedDiv">,
-  HelpText<"Check for divisions, where the denominator "
-           "might be 0 as it is a tainted (attacker controlled) value.">,
+  HelpText<"Check for divisions where the denominator is tainted "
+           "(attacker controlled) and might be 0.">,
   Dependencies<[TaintPropagationChecker]>,
   Documentation<HasDocumentation>;
 

clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp

@@ -33,7 +33,7 @@ class DivZeroChecker : public Checker<check::PreStmt<BinaryOperator>> {
                       llvm::ArrayRef<SymbolRef> TaintedSyms) const;
 
 public:
-  /// This checker class implements multiple user facing checker
+  /// This checker class implements several user facing checkers
   enum CheckKind { CK_DivideZero, CK_TaintedDivChecker, CK_NumCheckKinds };
   bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];

clang/test/Analysis/divzero-tainted-div-difference.c

@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast \
+// RUN:   -Wno-incompatible-library-redeclaration -verify=normaldiv %s \
+// RUN:   -analyzer-checker=optin.taint.GenericTaint \
+// RUN:   -analyzer-checker=core
+
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast \
+// RUN:   -Wno-incompatible-library-redeclaration -verify=tainteddiv %s \
+// RUN:   -analyzer-checker=optin.taint.GenericTaint \
+// RUN:   -analyzer-checker=optin.taint.TaintedDiv
+
+int getchar(void);
+
+
+//If we are sure that we divide by zero
+//we emit a divide by zero warning
+int testDivZero(void) {
+  int x = getchar(); // taint source
+  if (!x)
+    return 5 / x; // normaldiv-warning{{Division by zero}}
+  return 8;
+}
+
+// The attacker provided value might be 0
+int testDivZero2(void) {
+  int x = getchar(); // taint source
+  return 5 / x; // tainteddiv-warning{{Division by a tainted value}}
+}
+
+int testDivZero3(void) {
+  int x = getchar(); // taint source
+  if (!x)
+    return 0;
+  return 5 / x; // no warning
+}

clang/test/Analysis/taint-diagnostic-visitor.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=optin.taint,core,alpha.security.ArrayBoundV2,optin.taint.TaintedAlloc,optin.taint.TaintedDiv -analyzer-output=text -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=optin.taint,core,alpha.security.ArrayBoundV2 -analyzer-output=text -verify %s
 
 // This file is for testing enhanced diagnostics produced by the GenericTaintChecker
 

clang/test/Analysis/taint-generic.c

@@ -19,11 +19,9 @@
 // RUN:   -analyzer-config \
 // RUN:     optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: not %clang_analyze_cc1 -Wno-pointer-to-int-cast \
-// RUN:   -Wno-incompatible-library-redeclaration -verify %s \
+// RUN: not %clang_analyze_cc1 \
+// RUN:   -verify %s \
 // RUN:   -analyzer-checker=optin.taint.GenericTaint  \
-// RUN:   -analyzer-checker=optin.taint.TaintedDiv \
-// RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config \
 // RUN:     optin.taint.TaintPropagation:Config=justguessit \
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FILE
@@ -33,10 +31,9 @@
 // CHECK-INVALID-FILE-SAME:        that expects a valid filename instead of
 // CHECK-INVALID-FILE-SAME:        'justguessit'
 
-// RUN: not %clang_analyze_cc1 -Wno-incompatible-library-redeclaration \
+// RUN: not %clang_analyze_cc1 \
 // RUN:   -verify %s \
 // RUN:   -analyzer-checker=optin.taint.GenericTaint  \
-// RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config \
 // RUN:     optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config-ill-formed.yaml \
 // RUN:   2>&1 | FileCheck -DMSG=%errc_EINVAL %s -check-prefix=CHECK-ILL-FORMED
@@ -45,10 +42,9 @@
 // CHECK-ILL-FORMED-SAME:        'optin.taint.TaintPropagation:Config',
 // CHECK-ILL-FORMED-SAME:        that expects a valid yaml file: [[MSG]]
 
-// RUN: not %clang_analyze_cc1 -Wno-incompatible-library-redeclaration \
+// RUN: not %clang_analyze_cc1 \
 // RUN:   -verify %s \
 // RUN:   -analyzer-checker=optin.taint.GenericTaint \
-// RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config \
 // RUN:     optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config-invalid-arg.yaml \
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-ARG
@@ -419,16 +415,6 @@ int testTaintedDivFP(void) {
   return 5/x; // x cannot be 0, so no tainted warning either
 }
 
-
-//If we are sure that we divide by zero
-//we emit a divide by zero warning
-int testDivZero(void) {
-  int x = getchar(); // taint source
-  if (!x)
-    return 5 / x; // expected-warning{{Division by zero}}
-  return 8;
-}
-
 // Zero-sized VLAs.
 void testTaintedVLASize(void) {
   int x;