r270154 - [analyzer] Fix for PR23790 : constrain return value of strcmp() rather than returning a concrete value.

Anton Yartsev via cfe-commits cfe-commits at lists.llvm.org
Thu May 19 16:03:50 PDT 2016


Author: ayartsev
Date: Thu May 19 18:03:49 2016
New Revision: 270154

URL: http://llvm.org/viewvc/llvm-project?rev=270154&view=rev
Log:
[analyzer] Fix for PR23790 : constrain return value of strcmp() rather than returning a concrete value.

The function strcmp() can return any value, not just {-1,0,1} : "The strcmp(const char *s1, const char *s2) function returns an integer greater than, equal to, or less than zero, accordingly as the string pointed to by s1 is greater than, equal to, or less than the string pointed to by s2." [C11 7.24.4.2p3]
https://llvm.org/bugs/show_bug.cgi?id=23790
http://reviews.llvm.org/D16317

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
    cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=270154&r1=270153&r2=270154&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Thu May 19 18:03:49 2016
@@ -1837,6 +1837,8 @@ void CStringChecker::evalStrcmpCommon(Ch
   const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val);
   const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val);
   bool canComputeResult = false;
+  SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx,
+                                                C.blockCount());
 
   if (s1StrLiteral && s2StrLiteral) {
     StringRef s1StrRef = s1StrLiteral->getString();
@@ -1870,28 +1872,29 @@ void CStringChecker::evalStrcmpCommon(Ch
         s2StrRef = s2StrRef.substr(0, s2Term);
 
       // Use StringRef's comparison methods to compute the actual result.
-      int result;
+      int compareRes = ignoreCase ? s1StrRef.compare_lower(s2StrRef)
+                                  : s1StrRef.compare(s2StrRef);
 
-      if (ignoreCase) {
-        // Compare string 1 to string 2 the same way strcasecmp() does.
-        result = s1StrRef.compare_lower(s2StrRef);
-      } else {
-        // Compare string 1 to string 2 the same way strcmp() does.
-        result = s1StrRef.compare(s2StrRef);
+      // The strcmp function returns an integer greater than, equal to, or less
+      // than zero, [c11, p7.24.4.2].
+      if (compareRes == 0) {
+        resultVal = svalBuilder.makeIntVal(compareRes, CE->getType());
+      }
+      else {
+        DefinedSVal zeroVal = svalBuilder.makeIntVal(0, CE->getType());
+        // Constrain strcmp's result range based on the result of StringRef's
+        // comparison methods.
+        BinaryOperatorKind op = (compareRes == 1) ? BO_GT : BO_LT;
+        SVal compareWithZero =
+          svalBuilder.evalBinOp(state, op, resultVal, zeroVal,
+                                svalBuilder.getConditionType());
+        DefinedSVal compareWithZeroVal = compareWithZero.castAs<DefinedSVal>();
+        state = state->assume(compareWithZeroVal, true);
       }
-
-      // Build the SVal of the comparison and bind the return value.
-      SVal resultVal = svalBuilder.makeIntVal(result, CE->getType());
-      state = state->BindExpr(CE, LCtx, resultVal);
     }
   }
 
-  if (!canComputeResult) {
-    // Conjure a symbolic value. It's the best we can do.
-    SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx,
-                                                  C.blockCount());
-    state = state->BindExpr(CE, LCtx, resultVal);
-  }
+  state = state->BindExpr(CE, LCtx, resultVal);
 
   // Record this as a possible path.
   C.addTransition(state);

Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=270154&r1=270153&r2=270154&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Thu May 19 18:03:49 2016
@@ -680,6 +680,18 @@ void strncat_empty() {
 #define strcmp BUILTIN(strcmp)
 int strcmp(const char * s1, const char * s2);
 
+void strcmp_check_modelling() {
+  char *x = "aa";
+  char *y = "a";
+  clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) <= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strcmp(x, y) > 1); // expected-warning{{UNKNOWN}}
+
+  clang_analyzer_eval(strcmp(y, x) < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(y, x) >= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strcmp(y, x) < -1); // expected-warning{{UNKNOWN}}
+}
+
 void strcmp_constant0() {
   clang_analyzer_eval(strcmp("123", "123") == 0); // expected-warning{{TRUE}}
 }
@@ -703,13 +715,13 @@ void strcmp_0() {
 void strcmp_1() {
   char *x = "234";
   char *y = "123";
-  clang_analyzer_eval(strcmp(x, y) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_2() {
   char *x = "123";
   char *y = "234";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_null_0() {
@@ -727,25 +739,25 @@ void strcmp_null_1() {
 void strcmp_diff_length_0() {
   char *x = "12345";
   char *y = "234";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_diff_length_1() {
   char *x = "123";
   char *y = "23456";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_diff_length_2() {
   char *x = "12345";
   char *y = "123";
-  clang_analyzer_eval(strcmp(x, y) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_diff_length_3() {
   char *x = "123";
   char *y = "12345";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_embedded_null () {
@@ -777,6 +789,18 @@ void strcmp_union_function_pointer_cast(
 #define strncmp BUILTIN(strncmp)
 int strncmp(const char *s1, const char *s2, size_t n);
 
+void strncmp_check_modelling() {
+  char *x = "aa";
+  char *y = "a";
+  clang_analyzer_eval(strncmp(x, y, 2) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 2) <= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strncmp(x, y, 2) > 1); // expected-warning{{UNKNOWN}}
+
+  clang_analyzer_eval(strncmp(y, x, 2) < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(y, x, 2) >= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strncmp(y, x, 2) < -1); // expected-warning{{UNKNOWN}}
+}
+
 void strncmp_constant0() {
   clang_analyzer_eval(strncmp("123", "123", 3) == 0); // expected-warning{{TRUE}}
 }
@@ -800,13 +824,13 @@ void strncmp_0() {
 void strncmp_1() {
   char *x = "234";
   char *y = "123";
-  clang_analyzer_eval(strncmp(x, y, 3) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) > 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_2() {
   char *x = "123";
   char *y = "234";
-  clang_analyzer_eval(strncmp(x, y, 3) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_null_0() {
@@ -824,25 +848,25 @@ void strncmp_null_1() {
 void strncmp_diff_length_0() {
   char *x = "12345";
   char *y = "234";
-  clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_1() {
   char *x = "123";
   char *y = "23456";
-  clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_2() {
   char *x = "12345";
   char *y = "123";
-  clang_analyzer_eval(strncmp(x, y, 5) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) > 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_3() {
   char *x = "123";
   char *y = "12345";
-  clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_4() {
@@ -854,13 +878,13 @@ void strncmp_diff_length_4() {
 void strncmp_diff_length_5() {
   char *x = "012";
   char *y = "12345";
-  clang_analyzer_eval(strncmp(x, y, 3) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_6() {
   char *x = "234";
   char *y = "12345";
-  clang_analyzer_eval(strncmp(x, y, 3) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) > 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_embedded_null () {
@@ -874,6 +898,18 @@ void strncmp_embedded_null () {
 #define strcasecmp BUILTIN(strcasecmp)
 int strcasecmp(const char *s1, const char *s2);
 
+void strcasecmp_check_modelling() {
+  char *x = "aa";
+  char *y = "a";
+  clang_analyzer_eval(strcasecmp(x, y) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) <= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strcasecmp(x, y) > 1); // expected-warning{{UNKNOWN}}
+
+  clang_analyzer_eval(strcasecmp(y, x) < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(y, x) >= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strcasecmp(y, x) < -1); // expected-warning{{UNKNOWN}}
+}
+
 void strcasecmp_constant0() {
   clang_analyzer_eval(strcasecmp("abc", "Abc") == 0); // expected-warning{{TRUE}}
 }
@@ -897,13 +933,13 @@ void strcasecmp_0() {
 void strcasecmp_1() {
   char *x = "Bcd";
   char *y = "abc";
-  clang_analyzer_eval(strcasecmp(x, y) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) > 0); // expected-warning{{TRUE}}
 }
 
 void strcasecmp_2() {
   char *x = "abc";
   char *y = "Bcd";
-  clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcasecmp_null_0() {
@@ -921,25 +957,25 @@ void strcasecmp_null_1() {
 void strcasecmp_diff_length_0() {
   char *x = "abcde";
   char *y = "aBd";
-  clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcasecmp_diff_length_1() {
   char *x = "abc";
   char *y = "aBdef";
-  clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcasecmp_diff_length_2() {
   char *x = "aBcDe";
   char *y = "abc";
-  clang_analyzer_eval(strcasecmp(x, y) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) > 0); // expected-warning{{TRUE}}
 }
 
 void strcasecmp_diff_length_3() {
   char *x = "aBc";
   char *y = "abcde";
-  clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcasecmp_embedded_null () {
@@ -953,6 +989,18 @@ void strcasecmp_embedded_null () {
 #define strncasecmp BUILTIN(strncasecmp)
 int strncasecmp(const char *s1, const char *s2, size_t n);
 
+void strncasecmp_check_modelling() {
+  char *x = "aa";
+  char *y = "a";
+  clang_analyzer_eval(strncasecmp(x, y, 2) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 2) <= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strncasecmp(x, y, 2) > 1); // expected-warning{{UNKNOWN}}
+
+  clang_analyzer_eval(strncasecmp(y, x, 2) < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(y, x, 2) >= 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(strncasecmp(y, x, 2) < -1); // expected-warning{{UNKNOWN}}
+}
+
 void strncasecmp_constant0() {
   clang_analyzer_eval(strncasecmp("abc", "Abc", 3) == 0); // expected-warning{{TRUE}}
 }
@@ -976,13 +1024,13 @@ void strncasecmp_0() {
 void strncasecmp_1() {
   char *x = "Bcd";
   char *y = "abc";
-  clang_analyzer_eval(strncasecmp(x, y, 3) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 3) > 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_2() {
   char *x = "abc";
   char *y = "Bcd";
-  clang_analyzer_eval(strncasecmp(x, y, 3) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 3) < 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_null_0() {
@@ -1000,25 +1048,25 @@ void strncasecmp_null_1() {
 void strncasecmp_diff_length_0() {
   char *x = "abcde";
   char *y = "aBd";
-  clang_analyzer_eval(strncasecmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_diff_length_1() {
   char *x = "abc";
   char *y = "aBdef";
-  clang_analyzer_eval(strncasecmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_diff_length_2() {
   char *x = "aBcDe";
   char *y = "abc";
-  clang_analyzer_eval(strncasecmp(x, y, 5) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 5) > 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_diff_length_3() {
   char *x = "aBc";
   char *y = "abcde";
-  clang_analyzer_eval(strncasecmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_diff_length_4() {
@@ -1030,13 +1078,13 @@ void strncasecmp_diff_length_4() {
 void strncasecmp_diff_length_5() {
   char *x = "abcde";
   char *y = "aBd";
-  clang_analyzer_eval(strncasecmp(x, y, 3) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 3) < 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_diff_length_6() {
   char *x = "aBDe";
   char *y = "abc";
-  clang_analyzer_eval(strncasecmp(x, y, 3) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncasecmp(x, y, 3) > 0); // expected-warning{{TRUE}}
 }
 
 void strncasecmp_embedded_null () {




More information about the cfe-commits mailing list