[analyzer] CStringChecker should not rely on the analyzer generating UndefOrUnknown...

We are now often generating expressions even if the solver is not known to be able to simplify it. This is another cleanup of the existing code, where the rest of the analyzer and checkers should not base their logic on knowing ahead of the time what the solver can reason about.

In this case, CStringChecker is performing a check for overflow of 'left+right' operation. The overflow can be checked with either 'maxVal-left' or 'maxVal-right'. Previously, the decision was based on whether the expresion evaluated to undef or not. With this patch, we check if one of the arguments is a constant, in which case we know that 'maxVal-const' is easily simplified. (Another option is to use canReasonAbout() method of the solver here, however, it's currently is protected.)

This patch also contains 2 small bug fixes:
 - swap the order of operators inside SValBuilder::makeGenericVal.
 - handle a case when AddeVal is unknown in GenericTaintChecker::getPointedToSymbol.

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

diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index ff9d8689f63425b2b574d6de022e4f963b4b9699..49d8bf5388be53fad22525b364ecef4264061da0 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -532,10 +532,11 @@ const ProgramState *CStringChecker::checkAdditionOverflow(CheckerContext &C,
   const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy);
   NonLoc maxVal = svalBuilder.makeIntVal(maxValInt);
 
-  SVal maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, right,
-                                               sizeTy);
-
-  if (maxMinusRight.isUnknownOrUndef()) {
+  SVal maxMinusRight;
+  if (isa<nonloc::ConcreteInt>(right)) {
+    maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, right,
+                                                 sizeTy);
+  } else {
     // Try switching the operands. (The order of these two assignments is
     // important!)
     maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, left, 

diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index c8e54efb6732ca4a53535b72080201d174cc1171..76405a2db21d29b75c64bef49738a4a0e7f9caef 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -77,6 +77,11 @@ SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
                                                   bool IssueWarning) const {
   const ProgramState *State = C.getState();
   SVal AddrVal = State->getSVal(Arg->IgnoreParenCasts());
+
+  // TODO: Taint is not going to propagate?
+  if (AddrVal.isUnknownOrUndef())
+    return 0;
+
   Loc *AddrLoc = dyn_cast<Loc>(&AddrVal);
 
   if (!AddrLoc && !IssueWarning)

diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp
index cc2a8cb4d611bdceff0f688dcfba5332893df5f1..5eabbbeaaaf730e23f02b34d57040ef9a319d6de 100644 (file)
--- a/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -183,7 +183,7 @@ SVal SValBuilder::makeGenericVal(const ProgramState *State,
 
     if (const nonloc::ConcreteInt *lInt = dyn_cast<nonloc::ConcreteInt>(&LHS)) {
       symRHS = RHS.getAsSymExpr();
-      return makeNonLoc(symRHS, Op, lInt->getValue(), ResultTy);
+      return makeNonLoc(lInt->getValue(), Op, symRHS, ResultTy);
     }
 
     symLHS = LHS.getAsSymExpr();

diff --git a/test/Analysis/string.c b/test/Analysis/string.c
index a71e1f008817345f608ac65a6258e1f6399bfd8a..89283befad213d4f605b632785ef63727b6d6f6b 100644 (file)
--- a/test/Analysis/string.c
+++ b/test/Analysis/string.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.unix.CString,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
 // RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,experimental.unix.CString,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
 // RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,experimental.unix.CString,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,experimental.unix.CString,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=experimental.security.taint,core,experimental.unix.CString,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
 
 //===----------------------------------------------------------------------===
 // Declarations
@@ -26,6 +26,7 @@
 
 #define NULL 0
 typedef typeof(sizeof(int)) size_t;
+int scanf(const char *restrict format, ...);
 
 //===----------------------------------------------------------------------===
 // strlen()
@@ -436,6 +437,13 @@ void strcat_symbolic_src_length(char *src) {
                (void)*(char*)0; // no-warning
 }
 
+void strcat_symbolic_dst_length_taint(char *dst) {
+  scanf("%s", dst); // Taint data.
+  strcat(dst, "1234");
+  if (strlen(dst) < 4)
+    (void)*(char*)0; // no-warning
+}
+
 void strcat_unknown_src_length(char *src, int offset) {
        char dst[8] = "1234";
        strcat(dst, &src[offset]);

diff --git a/test/Analysis/taint-generic.c b/test/Analysis/taint-generic.c
index 9c99f908a85eb89f988066db356ea5ad926d2762..431fad4fe7b161bd10f2b5aff9dc22a67ce89607 100644 (file)
--- a/test/Analysis/taint-generic.c
+++ b/test/Analysis/taint-generic.c
@@ -39,8 +39,7 @@ void bufferScanfAssignment(int x) {
 
 void scanfArg() {
   int t;
-  scanf("%d", t); // expected-warning {{Pointer argument is expected}} \
-                  // expected-warning {{conversion specifies type 'int *' but the argument has type 'int'}}
+  scanf("%d", t); // expected-warning {{conversion specifies type 'int *' but the argument has type 'int'}}
 }
 
 void bufferGetchar(int x) {