r188680 - [analyzer] Assume that strings are no longer than SIZE_MAX/4.
Jordan Rose
jordan_rose at apple.com
Mon Aug 19 09:27:35 PDT 2013
Author: jrose
Date: Mon Aug 19 11:27:34 2013
New Revision: 188680
URL: http://llvm.org/viewvc/llvm-project?rev=188680&view=rev
Log:
[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!
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/malloc.c
cfe/trunk/test/Analysis/string.c
cfe/trunk/www/analyzer/open_projects.html
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=188680&r1=188679&r2=188680&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Mon Aug 19 11:27:34 2013
@@ -661,7 +661,7 @@ SVal CStringChecker::getCStringLengthFor
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::getCStringLengthFor
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;
}
Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=188680&r1=188679&r2=188680&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Mon Aug 19 11:27:34 2013
@@ -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.
Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=188680&r1=188679&r2=188680&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Mon Aug 19 11:27:34 2013
@@ -430,11 +430,12 @@ void strcat_unknown_src_length(char *src
// 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) {
Modified: cfe/trunk/www/analyzer/open_projects.html
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/open_projects.html?rev=188680&r1=188679&r2=188680&view=diff
==============================================================================
--- cfe/trunk/www/analyzer/open_projects.html (original)
+++ cfe/trunk/www/analyzer/open_projects.html Mon Aug 19 11:27:34 2013
@@ -174,11 +174,6 @@ mailing list</a> to notify other members
<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>
More information about the cfe-commits
mailing list