[cfe-commits] r146343 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CStringChecker.cpp lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp lib/StaticAnalyzer/Core/SValBuilder.cpp test/Analysis/string.c test/Analysis/taint-generic.c

Anna Zaks ganna at apple.com
Sun Dec 11 12:43:40 CST 2011


Author: zaks
Date: Sun Dec 11 12:43:40 2011
New Revision: 146343

URL: http://llvm.org/viewvc/llvm-project?rev=146343&view=rev
Log:
[analyzer] CStringChecker should not rely on the analyzer generating UndefOrUnknown value when it cannot reason about the expression.

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.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
    cfe/trunk/test/Analysis/string.c
    cfe/trunk/test/Analysis/taint-generic.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=146343&r1=146342&r2=146343&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Sun Dec 11 12:43:40 2011
@@ -532,10 +532,11 @@
   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, 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=146343&r1=146342&r2=146343&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun Dec 11 12:43:40 2011
@@ -77,6 +77,11 @@
                                                   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)

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=146343&r1=146342&r2=146343&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Sun Dec 11 12:43:40 2011
@@ -183,7 +183,7 @@
 
     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();

Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=146343&r1=146342&r2=146343&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Sun Dec 11 12:43:40 2011
@@ -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)*(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]);

Modified: cfe/trunk/test/Analysis/taint-generic.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-generic.c?rev=146343&r1=146342&r2=146343&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-generic.c (original)
+++ cfe/trunk/test/Analysis/taint-generic.c Sun Dec 11 12:43:40 2011
@@ -39,8 +39,7 @@
 
 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) {




More information about the cfe-commits mailing list