From: Jordan Rose <jordan_rose@apple.com>
Date: Mon, 19 Aug 2013 16:27:34 +0000 (+0000)
Subject: [analyzer] Assume that strings are no longer than SIZE_MAX/4.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a728e927c6e58f26b2c8615a8baa761d2f157e4b;p=clang

[analyzer] Assume that strings are no longer than SIZE_MAX/4.

This keeps the analyzer from making silly assumptions, like thinking
strlen(foo)+1 could wrap around to 0. This fixes PR16558.

Patch by Karthik Bhat!

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

diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 932f6316b5..ba1d9b9ff6 100644
--- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -661,7 +661,7 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C,
     if (Recorded)
       return *Recorded;
   }
-  
+
   // Otherwise, get a new symbol and update the state.
   SValBuilder &svalBuilder = C.getSValBuilder();
   QualType sizeTy = svalBuilder.getContext().getSizeType();
@@ -669,8 +669,21 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C,
                                                     MR, Ex, sizeTy,
                                                     C.blockCount());
 
-  if (!hypothetical)
+  if (!hypothetical) {
+    if (Optional<NonLoc> strLn = strLength.getAs<NonLoc>()) {
+      // In case of unbounded calls strlen etc bound the range to SIZE_MAX/4
+      BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
+      const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy);
+      llvm::APSInt fourInt = APSIntType(maxValInt).getValue(4);
+      const llvm::APSInt *maxLengthInt = BVF.evalAPSInt(BO_Div, maxValInt,
+                                                        fourInt);
+      NonLoc maxLength = svalBuilder.makeIntVal(*maxLengthInt);
+      SVal evalLength = svalBuilder.evalBinOpNN(state, BO_LE, *strLn,
+                                                maxLength, sizeTy);
+      state = state->assume(evalLength.castAs<DefinedOrUnknownSVal>(), true);
+    }
     state = state->set<CStringLength>(MR, strLength);
+  }
 
   return strLength;
 }
diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index 83946c83a8..2e5213e374 100644
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -1216,6 +1216,38 @@ void testReallocEscaped(void **memory) {
   }
 }
 
+// PR16558
+void *smallocNoWarn(size_t size) {
+  if (size == 0) {
+    return malloc(1); // this branch is never called
+  } 
+  else {
+    return malloc(size);
+  }
+}
+
+char *dupstrNoWarn(const char *s) {
+  const int len = strlen(s);
+  char *p = (char*) smallocNoWarn(len + 1);
+  strcpy(p, s); // no-warning
+  return p;
+}
+
+void *smallocWarn(size_t size) {
+  if (size == 2) {
+    return malloc(1);
+  }
+  else {
+    return malloc(size);
+  }
+}
+
+char *dupstrWarn(const char *s) {
+  const int len = strlen(s);
+  char *p = (char*) smallocWarn(len + 1);
+  strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
+  return p;
+}
 
 // ----------------------------------------------------------------------------
 // False negatives.
diff --git a/test/Analysis/string.c b/test/Analysis/string.c
index 6cf52f7a55..9fd3efb5c2 100644
--- a/test/Analysis/string.c
+++ b/test/Analysis/string.c
@@ -430,11 +430,12 @@ void strcat_unknown_src_length(char *src, int offset) {
 // length for the "before" strlen, we won't be able to set one for "after".
 
 void strcat_too_big(char *dst, char *src) {
+  // We assume this can never actually happen, so we don't get a warning.
 	if (strlen(dst) != (((size_t)0) - 2))
 		return;
 	if (strlen(src) != 2)
 		return;
-	strcat(dst, src); // expected-warning{{This expression will create a string whose length is too big to be represented as a size_t}}
+	strcat(dst, src);
 }
 
 
@@ -653,11 +654,12 @@ void strncat_unknown_limit(float limit) {
 }
 
 void strncat_too_big(char *dst, char *src) {
+  // We assume this will never actually happen, so we don't get a warning.
   if (strlen(dst) != (((size_t)0) - 2))
     return;
   if (strlen(src) != 2)
     return;
-  strncat(dst, src, 2); // expected-warning{{This expression will create a string whose length is too big to be represented as a size_t}}
+  strncat(dst, src, 2);
 }
 
 void strncat_zero(char *src) {
diff --git a/www/analyzer/open_projects.html b/www/analyzer/open_projects.html
index a5f5538662..4a888adbb6 100644
--- a/www/analyzer/open_projects.html
+++ b/www/analyzer/open_projects.html
@@ -174,11 +174,6 @@ mailing list</a> to notify other members of the community.</p>
     <i>(Difficulty: Easy)</i></p>
     </li>
 
-    <li>Teach CStringChecker that strings are never longer than, say, <code>SIZE_MAX/4</code> characters.</li>
-    <p>Though most of CStringChecker's functionality is disabled (due to poor diagnostics for error edge cases), it's still used to model certain operations like <code>strlen</code>, which should give the same result each time it's called on a string. However, assuming that the string length is an arbitrary symbolic value can give strange results -- for example, <code>strlen(str)+1</code> could wrap around to 0. (This is the root cause of <a href="http://llvm.org/bugs/show_bug.cgi?id=16558">PR16558</a>.) In practice, strings are never that long, so picking some large upper bound and recording that in the state would make plenty of sense, and would fix these false positives.
-    <i>(Difficulty: Easy)</i></p>
-    </li>
-
     <li>Implement iterators invalidation checker.
     <p><i>(Difficulty: Easy)</i></p>
     </li>